Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check screen is black after activating screen curtain #12701

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions source/visionEnhancementProviders/screenCurtain.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,48 @@ def confirmInitWithUser(self) -> bool:
return res == wx.YES


def isScreenFullyBlack() -> bool:
"""
Uses wx to check that the screen is currently fully black by taking a screen capture and checking:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have you considered using the screenBitmap module for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

- there is only one colour used in the image
- the first pixel of the screen capture is black

Iterating over a large bitmap image in python is slow (~4s depending on machine, number of pixels).
seanbudd marked this conversation as resolved.
Show resolved Hide resolved
Due to no native support for calculating the number of black pixels, the screenBitmap module is avoided.
seanbudd marked this conversation as resolved.
Show resolved Hide resolved
wxWidgets has native support for calculating the usage of colours within an image.
Using this method to calculate a colour usage histogram in native code takes ~0.4s comparably.
"""
screen = wx.ScreenDC()
screenSize = screen.GetSize()
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()
Comment on lines +327 to +331
Copy link
Member Author

@seanbudd seanbudd Sep 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

# https://docs.wxwidgets.org/3.0/classwx_image.html#a7c9d557cd7ad577ed76e4337b1fd843a
hist = wx.ImageHistogram()
numberOfColours = img.ComputeHistogram(hist)
# Due to wxImageHistogram not being fully supported in wxPython, the histogram is not subscriptable,
# and thus the key value cannot be accessed for colours.
# https://github.com/wxWidgets/Phoenix/issues/1991
# This function could be improved in the future using the following logic:
# numberOfBlackPixelsKey = hist.MakeKey(255, 255, 255)
# numberOfBlackPixels = hist[numberOfBlackPixelsKey]
# numberOfDisplayPixels = screenSize[0] * screenSize[1]
# log.debug(f"""Screen Capture:
# - number of colours: {numberOfColours}
# - number of black pixels: {numberOfBlackPixels}
# - number of total pixels: {numberOfDisplayPixels}
# """)
# return numberOfColours == 1 and numberOfBlackPixels == numberOfDisplayPixels
firstPixelIsBlack = img.GetRed(0, 0) == 0 and img.GetBlue(0, 0) == 0 and img.GetGreen(0, 0) == 0
return numberOfColours == 1 and firstPixelIsBlack


class ScreenCurtainInitializationError(RuntimeError):
pass


class ScreenCurtainProvider(providerBase.VisionEnhancementProvider):
_settings = ScreenCurtainSettings()

Expand Down Expand Up @@ -332,7 +374,13 @@ def __init__(self):
try:
Magnification.MagSetFullscreenColorEffect(TRANSFORM_BLACK)
Magnification.MagShowSystemCursor(False)
if not isScreenFullyBlack():
raise ScreenCurtainInitializationError("Screen curtain not activated properly")
except Exception as e:
try:
Magnification.MagShowSystemCursor(True)
except Exception:
log.error(f"Could not restore cursor after screen curtain failure.", exc_info=True)
Magnification.MagUninitialize()
raise e
if self.getSettings().playToggleSounds:
Expand Down