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

Display model reports the wrong background color for text drawn in transparent mode #6467

Open
mwcampbell opened this issue Oct 15, 2016 · 7 comments
Labels

Comments

@mwcampbell
Copy link
Contributor

@mwcampbell mwcampbell commented Oct 15, 2016

When text is written with the GDI background mode set to transparent, the display model doesn't necessary report the correct background color for that text. In this case, the GDI hook should look at the actual background color before the text is drawn, rather than using the result of GetBkColor.

@michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Oct 16, 2016

Our displayModel currently only records chunks of text. If a fill or other non-text drawing action occurs, any existing text in its path is simply removed from the model. Thus the only background color info we have currently is from written text. I'm assuming you mean we should actually track other non-text drawing actions, including its background and foreground color, so that if a drawing action such as text has a transparent color we can inherit the background color from the existing chunk?

This would be a large project. What is the impact of this (taking into account GDI becoming less used anyway)?

Loading

@mwcampbell
Copy link
Contributor Author

@mwcampbell mwcampbell commented Oct 17, 2016

I don't think you need anything that complex. Before you call the original GDI TextOut or ExtTextOut function, you can just use GetPixel to get the color at the specified X/Y coordinates. That would be good enough for my purpose, at least.

This impacts my ability to make Dragon NaturallySpeaking fully accessible as part of the DictationBridge project. Specifically, Dragon has some custom list views where I can't yet track the highlighted item because NVDA thinks the background color of all the text in the list view is white.

Loading

@michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Oct 17, 2016

If the background contained some kind of pattern or image then it may
not be accurate. However, I agree that is better than just transparent.

I will look into it at some point, though if you need it quicker I'll
certainly review a submitted pull request.

Loading

@nvaccessAuto nvaccessAuto added this to the 2017.2 milestone Mar 14, 2017
jcsteh added a commit that referenced this issue Jun 7, 2017
…r transparent text drawing (#6844)"

This has been confirmed by several users as causing severe performance regressions in certain apps.
This reverts commit 29efeda.
Fixes #7251. Reverts PR #6844. Reopens issue #6467.
@jcsteh jcsteh removed this from the 2017.2 milestone Jun 7, 2017
@jcsteh
Copy link
Contributor

@jcsteh jcsteh commented Jun 7, 2017

This was reverted in 47337ef due to severe performance regressions; see #7251.

Loading

@jcsteh jcsteh reopened this Jun 7, 2017
@josephsl
Copy link
Collaborator

@josephsl josephsl commented Jul 24, 2017

Hi,

@michaelDCurran, attached is the path that Matt should have sent as an official PR, but looks like it didn't make it through. I'm attaching in in case you'd like to review it and let Jamie or someone commit this. Thanks.
displayModel-attempted-fix.txt

Loading

michaelDCurran added a commit that referenced this issue Jul 25, 2017
…arent text drawing (#6844)

* GDI hooks: In the TextOut and ExtTextOut functions, get the previous color at the provided X, Y coordinates before calling the original function, so we at least have a chance of knowing the correct background color if the text is being drawn in transparent mode. This is necessary to detect highlighted text in some parts of Dragon NaturallySpeaking.

Fixes #6467
@nvaccessAuto nvaccessAuto added this to the 2017.4 milestone Aug 29, 2017
@leonardder
Copy link
Collaborator

@leonardder leonardder commented Jan 23, 2019

This was reverted in 338fb3e

Loading

@leonardder leonardder reopened this Jan 23, 2019
@leonardder
Copy link
Collaborator

@leonardder leonardder commented Jan 24, 2019

Some quick research seems to reveal that JAWS suffers from exactly the same problem.

Loading

@feerrenrut feerrenrut removed this from the 2017.4 milestone Mar 3, 2020
feerrenrut added a commit that referenced this issue Jul 27, 2021
Resolves concerns about contributed PR: #9197
Workaround for #6467

# Summary of the issue:
From #6467 (comment)

> When text is written with the GDI background mode set to transparent, the display model doesn't necessary report the
> correct background color for that text. In this case, the GDI hook should look at the actual background color before the
> text is drawn, rather than using the result of GetBkColor.

# Description of fix:
There have been two prior attempts to fix this, both causing severe performance regressions.
- #6844: "Display model: Try to provide the correct background color for transparent text drawing (issue #6467)"
- #7440: "displayModel: second try at recording the actual background color for text when a transparant background color is set."

This implementation merely exposes information about the potential transparency of the text background.
Using the samples in https://github.com/nvaccess/testDisplayModel
to test with various GDI dependent GUI APIs shows that the GetBkColor result can be:
- Opaque and matches visual presentation(rawGDI)
- Transparent and matches visual presentation: (mfcTest, GDItest)
- Transparent and does not match visual presentation: (anecdotal)
- The exposure of the transparency (via the alpha value on the colors.RGB class) will allow appModules
or add-ons to customize NVDA's presentation as necessary.

For users, there is no change in behavior by default. An option is introduced to the advanced settings panel to allow reporting of transparency in colors. The intention here is that developers will use this to identify where
color reporting could be improved.

The prior approach used the colorref high-level byte of a colorref struct. However, MSDN requires this byte be zero.
This prior approach did not pass the colorref into any Windows API's, so it was safe. But to prevent this from being accidentally introduced in the future, a compatible alternative type is introduced and used instead.

In order to stay compatible with python code interpreting this color value, a single bit is set if the color is considered transparent.

# Testing:
- Built applications from https://github.com/nvaccess/testDisplayModel
- Tested with/without transparency reporting enabled (advanced panel)
- Ensure color reporting is enabled in document formatting panel of NVDA settings
- Use NVDA+Numpad 7 to switch to screen review
- Use Numpad 7, Numpad 8, Numpad 9 to read the dialog

Co-authored-by: Leonard de Ruijter <leonard@babbage.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants