Check screen is black after activating screen curtain

[seanbudd]

Link to issue number:

Fixes #12708

Summary of the issue:

There is a security risk if the screen curtain fails silently. The Magnification API used by Screen Curtain does not officially support WOW64 applications like NVDA. As this is a risk with untested Windows versions, an external check would be helpful to minimise silent failures of the screen curtain.

Description of how this pull request fixes the issue:

Capture the screen and check it is fully black after initialising screen curtain.

wxWidgets uses winGDI methods, winGDI is a low level, stable API, as compared to the Magnification API:

"The Microsoft Windows graphics device interface (GDI) enables applications to use graphics and formatted text on both the video display and the printer. Windows-based applications do not access the graphics hardware directly. Instead, GDI interacts with device drivers on behalf of applications."

wxScreenDC captures all monitors.

Testing strategy:

Manual confirmation from a sighted developer is required.

1. Test the screen curtain activates properly
2. In the NVDA python console, change the black transformation matrix so it will fail to initialise the screen curtain properly.
3. Test that the screen curtain doesn't start and a warning is provided to the user.

Example for Win 10:

from visionEnhancementProviders import screenCurtain
screenCurtain.TRANSFORM_BLACK.transform[1][1] = 1  # retain the green channel

Example for Win 11 (preview):

from visionEnhancementProviders import screenCurtain
screenCurtain.TRANSFORM_BLACK.transform[3][3] = 0  # change the opacity to 0

Known issues with pull request:

None

Change log entries:

None

Code Review Checklist:

• Pull Request description is up to date.
• Unit tests.
• System (end to end) tests.
• Manual testing.
• User Documentation.
• Change log entry.
• Context sensitive help for GUI changes.
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers

[LeonarddeR]

have you considered using the screenBitmap module for this?

[seanbudd]

I've moved the implementation to use that instead, thanks for highlighting this as an option.

[seanbudd]

Having to iterate over this bitmap in python was considerably slower (took 4s on my machine). Using the wx method to calculate the histogram in native code takes 0.4s comparably.

[feerrenrut]

Using complete black as the screen curtain color makes it hard to differentiate from a failed buffer grab (initialized to zero). Perhaps, this would be safer to transform the screen to a slightly off-black, and test for that color.

[seanbudd]

Using complete black as the screen curtain color makes it hard to differentiate from a failed buffer grab (initialized to zero). Perhaps, this would be safer to transform the screen to a slightly off-black, and test for that color.

Should this be done in another PR? I imagine this may be contentious with problems like screen burn in and OLED power saving.

[seanbudd]

Using complete black as the screen curtain color makes it hard to differentiate from a failed buffer grab (initialized to zero). Perhaps, this would be safer to transform the screen to a slightly off-black, and test for that color.

@feerrenrut
Is it expected that a buffer grab fail would be a regular occurrence or just a random fail? I think the important part of this check is to flag if/when Windows changes the Magnification API causing the screen curtain to break.
We could even perform the check only for alpha builds. That is, if we are okay with just using this to fix and warn users when it breaks, as opposed to using it as a definitive future-proofed safety check.

[seanbudd]

@feerrenrut - at what stage is a buffer grab fail a concern?

Would a check like the following be an improvement?
Where setCanaryBits sets the bmp to some non-zero value canaryBits.
And areCanaryBitsSet checks if the bits are still set in the memory.

Suggested change

	bmp: wx.Bitmap = wx.Bitmap(screenSize[0], screenSize[1])
	mem = wx.MemoryDC(bmp)
	mem.Blit(0, 0, screenSize[0], screenSize[1], screen, 0, 0) # copy screen over to bmp
	del mem # Release bitmap
	img: wx.Image = bmp.ConvertToImage()
	bmp: wx.Bitmap = wx.Bitmap(screenSize[0], screenSize[1])
	setCanaryBits(bmp, canaryBits)
	mem = wx.MemoryDC(bmp)
	assert areCanaryBitsSet(mem, canaryBits)
	mem.Blit(0, 0, screenSize[0], screenSize[1], screen, 0, 0) # copy screen over to bmp
	assert not areCanaryBitsSet(mem, canaryBits)
	del mem # Release bitmap
	img: wx.Image = bmp.ConvertToImage()

[seanbudd]

Note: This doesn't handle if screen is initialised wrong - which is handled by wx and we have little control over.

[feerrenrut]

Is it expected that a buffer grab fail would be a regular occurrence or just a random fail? I think the important part of this check is to flag if/when Windows changes the Magnification API causing the screen curtain to break.

That's one failure we are aware of, but there are quite likely other ways that this could fail that we aren't aware of.

The feature should let users rely on it, so they don't need sighted assistance to confirm that screen curtain is working.

We could even perform the check only for alpha builds. That is, if we are okay with just using this to fix and warn users when it breaks, as opposed to using it as a definitive future-proofed safety check.

I don't think we could reliably guarantee this, even if we tested on all Windows versions (and updates) there may be other ways this could fail. Perhaps bad video drivers, or antivirus software.

Because this is privacy related, the feature should err on the side of caution.

[feerrenrut]

I think it should take multiple approaches:

• initialize the buffer with canaries and test they are no longer present.
• take a screen grab before and after enabling the screen curtain, confirm several spots are now all black
• use "off black" (at least while confirming screen curtain works). Eg set a transform that is "off black", screen grab, confirm, adjust transform to complete black.

To maintain performance it might be required to do this from C++ and asynchronously.

[SaschaCowley]

Closing in favour of #17894 17894