Created attachment 201803 [details]
Screenshot of the issue

Self-confirming, as originally reported in the GNOME a11y matrix chat and I can reproduce.

Bibisected with linux-64-25.2 to:

    commit 1acd37a671b9d3633a7d31a0b60478815fbc685f
    Author: Armin Le Grand (Collabora)
    Date:   Fri Sep 13 11:42:27 2024 +0200

        CairoSDPR: Activate globally to check builds/tests

And the problem isn't reproducible with DISABLE_SYSTEM_DEPENDENT_PRIMITIVE_RENDERER=1, so it seems CairoSDPR-related.

CC'ing Armin Le Grand: Can you please take a look?

(In reply to Michael Weghorn from comment #3)
> Bibisected with linux-64-25.2 to:
> 
>     commit 1acd37a671b9d3633a7d31a0b60478815fbc685f

To mention it, there the text is actually white on white background, so it's completely invisible. (The light grey instead of white background seems to be another behavior change introduced later.)

Created attachment 201806 [details]
clips of attachment 201802 [details] in each of the Win11 standard HC themes

Odd (or maybe expected), if I place Win11 into one of its HC contrast themes, the presentation does flatten in reasonable ways, text fg does complement canvas bg with a high contrast rendering in all standard Win11 HC themes (Aquatic, Desert, Dusk, Night sky).

So, a11y HC response to os/DE HC theming seems correct here.

Version: 26.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: e38d4e8cf2b1a1b5c146acbe325d31f84da676b0
CPU threads: 28; OS: Windows 11 X86_64 (build 26100); UI render: Skia/Vulkan; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: CL threaded

Would note that for Windows builds the Tools -> Options -> Accessibility the High Contrast lb is set to "Automatic" by default, no need to "Enable" it. Is the issue there maybe on the gtk3 backend?

IIRC the determination of fg/bg contrast over low luminance bg came up a few times, like see also bug 156182 and bug 156685, where IsDark() vs. IsBright() got tweaked

https://gerrit.libreoffice.org/c/core/+/158024

Don't know if that is at play here when entering HC mode, or if there is something specific to the gtk3 vcl plugin. 

But seems the fg/bg contrast calculations are working on the Windows builds responding to HC theme settings.

(In reply to V Stuart Foote from comment #6)
> Would note that for Windows builds the Tools -> Options -> Accessibility the
> High Contrast lb is set to "Automatic" by default, no need to "Enable" it.

Explicitly enabling is just what I did to not have to tweak any desktop environment settings, but the exact way it gets enabled should be unrelated here.

> Is the issue there maybe on the gtk3 backend?

I only noticed later (see comment 3) that this is CairoSDPR-specific, which would explain why it's not seen on Windows. I can also reproduce with qt6 (which also uses Cairo), but using either DISABLE_SYSTEM_DEPENDENT_PRIMITIVE_RENDERER=1 or SAL_VCL_QT_USE_QFONT=1 to avoid the CairoSDPR code path makes it work fine there (text becomes black on light background).

(In reply to Michael Weghorn from comment #8)

So the CairoSDPR does not reimplement the tools/color luminance based contrast testing?

Something needed for testing os/DE HC theme flattening, maybe around BColorModifier_RGBLuminanceContrast?
 https://opengrok.libreoffice.org/xref/core/basegfx/source/color/bcolormodifier.cxx?a=true&r=c03d752c614b3b19ed89a510d7791bc46604a452&h=368#367

(In reply to V Stuart Foote from comment #9)
> So the CairoSDPR does not reimplement the tools/color luminance based
> contrast testing?
> 
> Something needed for testing os/DE HC theme flattening, maybe around
> BColorModifier_RGBLuminanceContrast?
>  https://opengrok.libreoffice.org/xref/core/basegfx/source/color/
> bcolormodifier.
> cxx?a=true&r=c03d752c614b3b19ed89a510d7791bc46604a452&h=368#367

Armin might know. (I'm not familiar with the CairoSDPR code, so can't say anything right now without spending time to dig deeper first.)

Taking a look. Does someone know where that text color is determined? Ideally it should be done at graphics data preparation to not have to do it in the renderer - and thus in ALL renderers. I would guess it is at render time. A code ptr would be appreciated, have no idea how and when that is actually done...

If it is done when rendering it might be a indirect effect that it works without CairoSDPR - the VCLPrimitiveRenderer does use OutDev/VCL as target, thus we often have hidden effects that in the VCL layers something is done even when rendering primitives.

IIANM for VCL we use tools/color IsDark() luminance test, and its negation for IsBright(), to assign the GetReadableFontColor() in the vcl output device outdev/wallpaper.cxx [1]

Luminance calculations for bug 161766 in addition to bug 156182 adjusted the thresholds.

But  not at all clear how the CairoSDPR paths would need to be tweaked to do similar IsDark() color checks for Automatic fg/bg Font contrast when in os/DE reported or user enabled HC assistive technology mode [2].

Note: I think this is mainly a Font fg/bg consideration, but there are other UI elements that can end up "disappearing" into a bg color. Recent Appearance theme work had required coloring for specific elements to receive increased or decreased luminance  of an element to make it visible, lacking granular color assignment in VCL.

=-ref-=
[1] https://opengrok.libreoffice.org/xref/core/vcl/source/outdev/wallpaper.cxx?r=e7143e7acbcc4abf7abd39390601246bc016e2dd&h=25#25

[2] https://opengrok.libreoffice.org/xref/core/vcl/source/window/settings.cxx?r=f0a8b5b81d6be08de6e0d504616a1f09830f7c38#234

Wondering: Why the need to restart LO when changing "Tools" -> "Options" -> "LibreOffice" -> "Accessibility", set "High Contrast"...

Text is on Color 'Automatic' -> as expected.
Using DISABLE_SYSTEM_DEPENDENT_PRIMITIVE_RENDERER makes the text appear black -> as reported.

Debugging where that is done...

(In reply to Armin Le Grand (collabora) from comment #14)
> Wondering: Why the need to restart LO when changing "Tools" -> "Options" ->
> "LibreOffice" -> "Accessibility", set "High Contrast"...
> 

Don't we "flatten" the UI elements in response to HC, and we also change the Icon theme to either Sifr flavor. That needs the restart, right?

The render with VCL primitive renderer uses VclPixelProcessor2D::processTextSimplePortionPrimitive2D which calls VclProcessor2D::RenderTextSimpleOrDecoratedPortionPrimitive2D and there in drawinglayer/source/processor2d/vclprocessor2d.cxx in line 451 it uses

    mpOutputDevice->SetTextColor(Color(aRGBFontColor));

and inside that the color is manipulated. OutputDevice::SetTextColor uses vcl::drawmode::GetTextColor and that when DrawModeFlags::SettingsText and not DrawModeFlags::SettingsForSelection uses rStyleSettings.GetWindowTextColor().

Thus this is not done based on using up-to now existing BG - wich is good, would be another read-access to target surface which is bad for many target systems. 

It is bad since it uses internal information from OutputDevice, resp. DrawModeFlags and StyleSettings. That is not available (by purpose - we want to get away from OutputDevice) in CairoSDPR.

The graphic data (aka primitives) for a primitive renderer are 'complete' and should no longer be interpreted by a renderer, but just 'rendererd'. I have to think about where and how to best apply that to primitives. I think there is already some stuff when creating those text primitives, have to check...

Looked at different possibilities where I could do that, but none is really good yet.
There is isDrawModeHighContrast at ObjectContact, but not accessible in ViewContactOfSdrRectObj::createViewIndependentPrimitive2DSequence where the text primitive gets created.
I could also add that information to drawinglayer::geometry::ViewInformation2D e.g. when repainting, but there are some usages e.g. in undo/redo where that info is not really filled, so might cause additional work.
Also possible to add at SdrTextObj::impDecomposeBlockTextPrimitive where the OutputDevice *is* available, but for some cases this might be a fake OutDev just to use the decompose.
And last OutDev *is* available in CairoSDPR incarnation, but as explained should not (only for construct/destruct bugfix) and thus should not be used.

Need to dig deeper...

I thought about it and there is not really a need to get that settings from the OutputDevice - that might have been cosy for the 'OutDevAge' but in principle it has to be fetched from the config and set at the OutDev. Looked for that and found in Window::Window:

    // adjust contrast mode initially
    bool bUseContrast = GetSettings().GetStyleSettings().GetHighContrastMode();
    GetOutDev()->SetDrawMode( bUseContrast
        ? sd::OUTPUT_DRAWMODE_CONTRAST
        : sd::OUTPUT_DRAWMODE_COLOR );

where

const DrawModeFlags OUTPUT_DRAWMODE_CONTRAST
        = DrawModeFlags::SettingsLine | DrawModeFlags::SettingsFill
        | DrawModeFlags::SettingsText | DrawModeFlags::SettingsGradient;

all flags are set anyways. Thus I can just evaluate what I need anytime, where and when I need it. Trying that next...

Found a good place in buildTextPortionPrimitive to do that, and that works as expected. But GetTextColor *also* decides based on DrawModeFlags::SettingsForSelection and thus I have to figure out what that one is doing at all and if it is necessary...

Luckily it seems as if DrawModeFlags::SettingsForSelection is only used in OverlayManager, and there specific for painting the selections.

NOTE: That is done setting that flag at the target OutDev - guess when this will no longer be working... If e.g. another renderer like CairoSDPR is used. Thus all that is unfortunately a hack and will also need to be replaced/fixed. It only works with VCLPixelPrimitiveRenderer because there the set flags from OverlayManager::ImpDrawMembers will be used.

What really needs to be done is not to patch Colors in the renderer like OutDev, but to use the correct colors where those controls are painted. I am sure having done it the way it is now was fast(er), but as we see this comes for a price - it has to be done correctly one day anyways - sigh...

For fixing this task I will go with the direct way, is doable.

@Testers: As said, there *should* be more errors due to that hacks used in OutputDevice, please have a look at TextSelections, these will probably use incorrect colors due to how it was done. Write bugs and inform me.

Armin Le Grand (collabora) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/2ebafbb84f4eb6ed55b813bbc24b6a4368236f25

tdf#167511 correctly apply Color in HighContrast mode to text

It will be available in 26.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.

(In reply to Armin Le Grand (collabora) from comment #20)
> @Testers: As said, there *should* be more errors due to that hacks used in
> OutputDevice, please have a look at TextSelections, these will probably use
> incorrect colors due to how it was done. Write bugs and inform me.

Thanks, Armin! I can confirm the text color is now black (correct) with High Contrast mode in editing mode now for CairoSDPR as well.

However, black is now also used (even with DISABLE_SYSTEM_DEPENDENT_PRIMITIVE_RENDERER=1) in actual presentation mode, where the actual red background is shown, and the actual font color (white) should be used, too, in my understanding.

Do you have an idea on how to avoid that?

I'd defer testing further cases until that's solved unless you think it makes sense to still do it now.

(In reply to Michael Weghorn from comment #22)
> Thanks, Armin! I can confirm the text color is now black (correct) with High
> Contrast mode in editing mode now for CairoSDPR as well.

Okay

> However, black is now also used (even with
> DISABLE_SYSTEM_DEPENDENT_PRIMITIVE_RENDERER=1) in actual presentation mode,
> where the actual red background is shown, and the actual font color (white)
> should be used, too, in my understanding.
> 
> Do you have an idea on how to avoid that?

Not really - what the change does is to react on HighContrast being set, so in principle the presentation *should* then be in high contrast too, right? The question is what is the error here...
Of course when it was like that (ignoring HighContrast for presentation mode) until now that's an argument to stay at that (what also means we have no way to have presnetation in HighContrast - but never had - but would maybe need due to accessibility?).
That's what I meant with it being an error by implementing it using hacks in the renderer - presentation mode is another renderer and it's not implemented there. I still think it should not be implemented in all renderers for all times, but where the draw commands or primitves get crafted - that way it will work in all renderers (including presentation mode and future ones like e.g. SkiaSDPR).

What to do?

For now I will try to find a way to not do the in principle right thing for slideshow mode - somehow (see, that will probably be another hack and we get deeper and deeper into the rabbit hole).

In principle we would have to remove those hacks in OutDev and move all that stuff to where the rendering is defined. Imagine the code to draw a button: It is hard-coded to some colors and these hacks make e.g. DrawRect in OutDev ignore that colors and use HighContrast ones. Instead the code to draw a Button should have a color scheme from where it picks the colors. Then just use schemes for normal and HighContrast and whatnot.

Problem is that there is never budget for things like this, I guess that's also the reason it is implemented as it is for now. Understandable, no one pays for cleanups which make it look as before ignoring the security and cleanup effect. But also ignoring that the code gets *again* more and more quirky and ssth like these modes need dozens of dozens of bugfixes - exactly for that reason. I would not be surprised when costs for that are a multiple at the end compared to just clean it up once. sigh...

> I'd defer testing further cases until that's solved unless you think it
> makes sense to still do it now.

Okay

(In reply to Armin Le Grand (collabora) from comment #23)
> Not really - what the change does is to react on HighContrast being set, so
> in principle the presentation *should* then be in high contrast too, right?
> The question is what is the error here...
> Of course when it was like that (ignoring HighContrast for presentation
> mode) until now that's an argument to stay at that (what also means we have
> no way to have presnetation in HighContrast - but never had - but would
> maybe need due to accessibility?).

Having a setting to also force high contrast in presentation mode might be worth a consideration, but I think using the "actual" colors at least by default makes sense. Similar to what happens for PDF export or printing, high contrast might be seen as a setting to help in the "one-user editing/reading stage", but not what affects the "final output" by default, which is what "everybody" sees. (Maybe using high contrast in the presenter console but not on the presentation screen might be another option, but that'd likely be a new feature and best handled independent of this bug report?)
(In the GNOME a11y matrix room discussion, there were also other voices that high contrast shouldn't affect the document at all, only the rest of the UI, which could be another configuration option if we want to introduce more flexibility/complexity.)


> In principle we would have to remove those hacks in OutDev and move all that
> stuff to where the rendering is defined. Imagine the code to draw a button:
> It is hard-coded to some colors and these hacks make e.g. DrawRect in OutDev
> ignore that colors and use HighContrast ones. Instead the code to draw a
> Button should have a color scheme from where it picks the colors. Then just
> use schemes for normal and HighContrast and whatnot.

I'm not very familiar with what exactly happens on the rendering stages, but this sounds plausible to me. (I think we have some concept of color schemes/StyleSettings already, but it's very possible that the actual colors are resolved "at the wrong stage" currently instead of being passed to where they should go.)

> Problem is that there is never budget for things like this, I guess that's
> also the reason it is implemented as it is for now. Understandable, no one
> pays for cleanups which make it look as before ignoring the security and
> cleanup effect. But also ignoring that the code gets *again* more and more
> quirky and ssth like these modes need dozens of dozens of bugfixes - exactly
> for that reason. I would not be surprised when costs for that are a multiple
> at the end compared to just clean it up once. sigh...

You can probably judge best what's the "best" approach/compromise here, taking the different factors that come into play into account ("cleanest" solution vs cost/effort). Thanks for looking into it!

This is a hard one. Found EditViewActive state in ViewInformation2D, this is (up to now) used to signal that an EditView gets painted. If I get all that right, HighContrast is - currently - only active in EditViews...?

At the end I need infos at the point where the text primitives get created, tis will always be true. For that I now added drawinglayer::geometry::ViewInformation2D to the StripPortionsHelper which gets forwarded as needed. At the end I have info at buildTextPortionPrimitive in editeng/source/editeng/StripPortionsHelper.cxx. Similar stuff may have to be done for other situations where Primitives get created. drawinglayer::geometry::ViewInformation2D is already a often used info container for primitive rendering, so that will be fine.
I decided for now to indeed use EditViewActive in that structure. If that collides somewhere in the future it will be easy to add something like 'SupportsHighContrast' or similar.

Armin Le Grand (collabora) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d6a3d308598d47d6658c51a95a561cc3c93a3393

tdf#167511 correctly apply Color in HighContrast mode to text II

It will be available in 26.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.

Looks good to me in presentation mode now in a quick test.

However, this scenario looks broken with both, gtk3 and qt6:

1) open attachment 201802 [details]
-> "Some title shown in black" (OK)
2) click into the title text box to enable editing
3) type to add some text at the end, e.g. just "a"
4) click outside of the box again

Now, the text becomes white again -> low contrast.
(Sometimes, I had to repeat steps 3 and 4 twice before the text turned white.)

Thanks, indeed happens. Also need to make the decompose of SdrTextPrimitive2D dependent of change of EditViewActive state, but only if HighContrast is active. Added that, see https://gerrit.libreoffice.org/c/core/+/188112.

Armin Le Grand (collabora) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6381d366672234d899eeff278870cfb1c5d100e9

tdf#167511 correctly apply Color in HighContrast mode to text III

It will be available in 26.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.

Created attachment 201939 [details]
Another sample with a different slide background

(In reply to Armin Le Grand (collabora) from comment #29)
> Also need to make the decompose of
> SdrTextPrimitive2D dependent of change of EditViewActive state, but only if
> HighContrast is active. Added that, see
> https://gerrit.libreoffice.org/c/core/+/188112.

Thanks.

(In reply to Michael Weghorn from comment #31)
> Created attachment 201939 [details]
> Another sample with a different slide background

This is a sample file where - other than for attachment 201802 [details] - the background apparently remains unchanged (maybe contrast is already considered high enough?). But the font is changed from white to black, resulting in bad contrast again with current master as of 2951e2ac0e80fc982cd1fe52a849f40e44da65dc.

Have added now https://gerrit.libreoffice.org/c/core/+/188527 which should solve the problem. If that works we may remove the former fixes again...

Ah - I guess I have to 1st revert those former fixes anyways, to go back to the original error. See the gerrit build mentioned one above as test build for now...

Armin Le Grand (collabora) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/355ffcbd8242e8e2c4df3daa6f2d5d61580cae12

Revert "tdf#167511 correctly apply Color in HighContrast mode to text III"

It will be available in 26.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.

Armin Le Grand (collabora) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/624690f479e402f9e5e9223801dde04f20418dd2

Revert "tdf#167511 correctly apply Color in HighContrast mode to text II"

It will be available in 26.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.

Armin Le Grand (collabora) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/56e887181606d574357bb5286a27b9131c60c85a

Revert "tdf#167511 correctly apply Color in HighContrast mode to text"

It will be available in 26.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.

(In reply to Armin Le Grand (collabora) from comment #33)
> Have added now https://gerrit.libreoffice.org/c/core/+/188527 which should
> solve the problem. If that works we may remove the former fixes again...

Thanks! Commented there with observations from a first test.

(In reply to Michael Weghorn from comment #24)
> (Maybe using high contrast in the presenter
> console but not on the presentation screen might be another option, but
> that'd likely be a new feature and best handled independent of this bug
> report?)

FWIW, when testing the latest Gerrit change (but unrelated to that change) I noticed that the "Next Slide" in presenter console already does apply the high contrast colors - while the current slide doesn't.

Armin Le Grand (collabora) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d569a6b335d27ac13487e5e66cd9880439beb03b

tdf#167511 Added use of DrawModeFlags to CairoSDPR

It will be available in 26.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.

(In reply to Michael Weghorn from comment #39)
> FWIW, when testing the latest Gerrit change (but unrelated to that change) I
> noticed that the "Next Slide" in presenter console already does apply the
> high contrast colors - while the current slide doesn't.

That maybe a hint that "Next Slide" is rendered using a renderer that supports HighContrast (now VCLPrimitiveRenderer & CairoSDPR after these changes) while 'current slide' (is this on the big screen or the presenter console - unclear?) does use the PresentationMode/Renderer...

(In reply to Armin Le Grand (collabora) from comment #41)
> while 'current slide' (is this on the big screen or the presenter
> console - unclear?) does use the PresentationMode/Renderer...

Currently both (and I think the big screen always should, but for the presenter console one, i.e. the one only the presenter sees it might be considered useful to respect high contrast, as it's already the case for "next slide" there)

(In reply to Michael Weghorn from comment #42)
> Currently both (and I think the big screen always should, but for the
> presenter console one, i.e. the one only the presenter sees it might be
> considered useful to respect high contrast, as it's already the case for
> "next slide" there)

But, as mentioned earlier: This is independent of/not directly related to this ticket.

Some info: I did https://gerrit.libreoffice.org/c/core/+/188704 which makes using DrawModeFlags system- and renderer-independent - I checked that it does not again break this, looks good.
For this - and maybe some more experimenting - would be good to have a 2nd look, please?

(In reply to Michael Weghorn from comment #42)
> Currently both (and I think the big screen always should, but for the
> presenter console one, i.e. the one only the presenter sees it might be
> considered useful to respect high contrast, as it's already the case for
> "next slide" there)

I agree.

About the big screen it's possible to argue in all directions - from 'why should anyone do presentations for listeners who need HC at all' up to 'If I set HC I want HC everywhere'...

(In reply to Armin Le Grand (collabora) from comment #44)
> Some info: I did https://gerrit.libreoffice.org/c/core/+/188704 which makes
> using DrawModeFlags system- and renderer-independent - I checked that it
> does not again break this, looks good.
> For this - and maybe some more experimenting - would be good to have a 2nd
> look, please?

Current git master (which includes that change) still works fine for me in a quick test.

(In reply to Armin Le Grand (collabora) from comment #45)
> About the big screen it's possible to argue in all directions - from 'why
> should anyone do presentations for listeners who need HC at all' up to 'If I
> set HC I want HC everywhere'...

True. I think having original colors by default makes sense. But if there's a need to make HC apply for the presentation screen(s) as well, it might maybe be a new option that can explicitly be enabled?

So I set this to fixed

Setting to VERIFIED, thanks again!