Bug 160036 - Selection invisible in a11y High Contrast modes with SKIA/Raster, Skia/Vulkan unaffected
Summary: Selection invisible in a11y High Contrast modes with SKIA/Raster, Skia/Vulkan...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
24.2.0.0 alpha0+
Hardware: All Windows (All)
: highest major
Assignee: Patrick (volunteer)
URL:
Whiteboard: target:24.8.0 target:24.2.2.2
Keywords: accessibility, bibisected, bisected, regression
: 158231 159214 159751 160030 160044 160262 (view as bug list)
Depends on:
Blocks: High-Contrast Selection Skia
  Show dependency treegraph
 
Reported: 2024-03-05 02:49 UTC by Alistair Saywell
Modified: 2024-07-31 04:49 UTC (History)
12 users (show)

See Also:
Crash report or crash signature:


Attachments
Comparison screenshots of Calc and Writer from 24.2 and 7.6.3.2 (53.88 KB, image/jpeg)
2024-03-05 02:49 UTC, Alistair Saywell
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alistair Saywell 2024-03-05 02:49:30 UTC
Created attachment 192952 [details]
Comparison screenshots of Calc and Writer from 24.2 and 7.6.3.2

In 24.2.0.2 and also 24.1.2, with a new user profile, in Options select in View *Force Skia software rendering* and in Accessibility select *High Contrast mode*

Result: There is no highlight for selection and you cannot tell what has been selected. In Calc the blue highlight in the row and column headers is visible but the actual cell has no special selection border. In Writer the selected text looks identical to unselected text.

Compare to LibreOffice 7.6.3.2 where there is a heavy black outline for the selected Calc cell and in Writer the selected text has reversed colour. 

See attached image for comparison between the two versions.

Arisen in question https://ask.libreoffice.org/t/24-2-1-selected-text-cells-not-highlighted-in-windows-10-high-contrast-mode/102965

Version: 24.2.1.2 (X86_64) / LibreOffice Community
Build ID: db4def46b0453cc22e2d0305797cf981b68ef5ac
CPU threads: 8; OS: Windows 10.0 Build 22631; UI render: Skia/Raster; VCL: win
Locale: en-NZ (en_NZ); UI: en-GB
Calc: CL threaded
Comment 1 loic_1997 2024-03-05 14:34:26 UTC
*** Bug 160030 has been marked as a duplicate of this bug. ***
Comment 2 V Stuart Foote 2024-03-05 17:10:29 UTC
Confirmed. But only with Skia/Raster frame mode. The highlight/selection done in any of the 4 win10 HC modes (HC White as was attached, HC Black, HC#1 or HC#2) is not being exposed.

Skia/Vulkan rendering correctly treats the highlight or selection.

I did not check the HC modes on a win11 system, but would expect suspect Skia/Raster results there.


Version: 24.2.1.2 (X86_64) / LibreOffice Community
Build ID: db4def46b0453cc22e2d0305797cf981b68ef5ac
CPU threads: 8; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: CL threaded
Comment 3 V Stuart Foote 2024-03-05 17:15:33 UTC
(In reply to V Stuart Foote from comment #2)
> 
> Version: 24.2.1.2 (X86_64) / LibreOffice Community
> Build ID: db4def46b0453cc22e2d0305797cf981b68ef5ac
> CPU threads: 8; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL:
> win
> Locale: en-US (en_US); UI: en-US
> Calc: CL threaded

Should note there is no issue with the highlight / select behavior in Skia/Raster when os/DE has not been placed into HC mode assistive technology.
Comment 4 V Stuart Foote 2024-03-05 19:14:11 UTC
Just checked a recent master against 24.8 on Win11 (different system, different GPU).

Issue with Skia / Raster frame rendering with the win11 HC modes is present in master running on win11 as well.

Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 17fc445938dedb05125a6d6a5b4ce7f34ea95f59
CPU threads: 8; OS: Windows 10.0 Build 22621; UI render: Skia/Raster; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: threaded
Comment 5 V Stuart Foote 2024-03-05 19:15:43 UTC
Thus far looks to be Skia lib calls on windows only.
Comment 6 Stéphane Guillou (stragu) 2024-03-06 03:05:58 UTC
Bibisected with win64-24.2 repository to first bad build [7d56051a3adc746e523c45602895e3c4414a705d] which points to:

commit f8c15850dbfaa46605e1e353ae1f49e69184e8a1
author	Noel Grandin 	Sat Jul 08 20:28:29 2023 +0200
committer	Noel Grandin 	Mon Jul 10 12:44:38 2023 +0200
update skia to m116
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154223

Noel can you please have a look?
Comment 7 Patrick (volunteer) 2024-03-13 18:06:14 UTC
I can also reproduce this bug on macOS in my local master build:

Version: 24.8.0.0.alpha0+ (AARCH64) / LibreOffice Community
Build ID: 80d401dd9ed4bfb06ed91c81050e76fb32efd2b5
CPU threads: 8; OS: macOS 14.2.1; UI render: Skia/Raster; VCL: osx
Locale: en-CA (en_CA.UTF-8); UI: en-US
Calc: threaded

This bug does not occur when high contrast is disabled so I wonder what code is different when using the high contrast code.
Comment 8 Xisco Faulí 2024-03-14 16:16:11 UTC
Hi Noel,
I see some new commits in https://chromium.googlesource.com/skia/+log/refs/heads/chrome/m116 since 2ddcf18. Would it make sense to upgrade to the latest d2c2112 to see whether it fixes the issue ?
Comment 9 Patrick (volunteer) 2024-03-14 18:26:19 UTC
(In reply to Xisco Faulí from comment #8)
> Hi Noel,
> I see some new commits in
> https://chromium.googlesource.com/skia/+log/refs/heads/chrome/m116 since
> 2ddcf18. Would it make sense to upgrade to the latest d2c2112 to see whether
> it fixes the issue ?

I haven't had time to look at the code yet, but this bug looks like the XOR failures we saw when the unit tests started using Skia/Raster. IIRC, I found that the following "pixel function" strings in vcl/skia/SkiaHelper.cxx weren't doing anything even when I changed the strings:

        // Note that the colors are premultiplied, converting to 0-255 range
        // must also unpremultiply.
        const char* const diff = R"(
            vec4 main( vec4 src, vec4 dst )
            {
                return vec4(
                    float(int(src.r * src.a * 255.0) ^ int(dst.r * dst.a * 255.0)) / 255.0 / dst.a,
                    float(int(src.g * src.a * 255.0) ^ int(dst.g * dst.a * 255.0)) / 255.0 / dst.a,
                    float(int(src.b * src.a * 255.0) ^ int(dst.b * dst.a * 255.0)) / 255.0 / dst.a,
                    dst.a );
            }
        )";

What is interesting is the comment just below that:

        SkRuntimeEffect::Options opts;
        // Skia does not allow binary operators in the default ES2Strict mode, but that's only
        // because of OpenGL support. We don't use OpenGL, and it's safe for all modes that we do use.
        // https://groups.google.com/g/skia-discuss/c/EPLuQbg64Kc/m/2uDXFIGhAwAJ
        opts.maxVersionAllowed = SkSL::Version::k300;

So maybe the above runtime effect is no longer supported in m116?
Comment 10 Patrick (volunteer) 2024-03-14 21:42:14 UTC
I looked at the code and it is definitely doing XOR drawing of the selection polygon. The last case of XOR failing with Skia/Raster I fixed by drawing normally with polygonal clipping:

https://git.libreoffice.org/core/+/c43d11cbbabc871a3c71062239b0d9b6db99961e%5E%21

Below is the relevant portion of the stack when doing the XOR drawing. Maybe there is a way we could replace XOR drawing and, instead, draw normally with polygonal clipping without copying the buffer?:

    frame #4: 0x0000000117d597e8 libmergedlo.dylib`drawinglayer::processor2d::VclPixelProcessor2D::processInvertPrimitive2D(this=0x0000600002b1ca10, rCandidate=0x000060000a34a4e0) at vclpixelprocessor2d.cxx:911:21
    frame #5: 0x0000000117d576c8 libmergedlo.dylib`drawinglayer::processor2d::VclPixelProcessor2D::processBasePrimitive2D(this=0x0000600002b1ca10, rCandidate=0x000060000a34a4e0) at vclpixelprocessor2d.cxx:343:13
    frame #6: 0x0000000117d2f7e4 libmergedlo.dylib`drawinglayer::processor2d::BaseProcessor2D::process(this=0x0000600002b1ca10, rSource=0x000000016fdfba28) at baseprocessor2d.cxx:69:21
    frame #7: 0x000000011a68e0e4 libmergedlo.dylib`sdr::overlay::OverlayManager::ImpDrawMembers(this=0x00000003824f5310, rRange=0x000000016fdfbde0, rDestinationDevice=0x00000003824f6250) const at overlaymanager.cxx:88:41
    frame #8: 0x000000011a6856fc libmergedlo.dylib`sdr::overlay::OverlayManagerBuffered::ImpBufferTimerHandler(this=0x00000003824f5310, (null)=0x00000003824f5420) at overlaymanagerbuffered.cxx:289:33
Comment 11 Noel Grandin 2024-03-15 06:18:07 UTC
(In reply to Xisco Faulí from comment #8)
> Hi Noel,
> I see some new commits in
> https://chromium.googlesource.com/skia/+log/refs/heads/chrome/m116 since
> 2ddcf18. Would it make sense to upgrade to the latest d2c2112 to see whether
> it fixes the issue ?

It would, but upgradning skia is a painful process. We replace their build scripts (because they use some other build tool that we don't support), and their API sometimes changes or move around, which means we then need to adapt our code.

Not something I feel sufficiently enthusiastic to attempt.
Comment 12 Patrick (volunteer) 2024-03-15 10:29:08 UTC
(In reply to Noel Grandin from comment #11)
> It would, but upgradning skia is a painful process. We replace their build
> scripts (because they use some other build tool that we don't support), and
> their API sometimes changes or move around, which means we then need to
> adapt our code.

I wonder if something changed or moved around in m116 (and likely later) that is short-circuiting our invert and XOR strings. After all, the invert and XOR strings work in the GPU Skia implementations, but not in the Raster implementation.

When I get some time, I am thinking of stepping into the Skia code that handles our XOR string in a debugger and see if I can find anything in Skia that is failing when using Raster.

If I don't find anything with that, my fallback plan is to rewrite the XOR code in VclPixelProcessor2D::processInvertPrimitive2D() when using Skia/Raster to extract a BitmapEx from the OutputDevice, iterate through its pixels and XOR each pixel, and then draw the XOR'd BitmapEx onto the OutputDevice with clip set to the polypolygon selection shape.

Sound reasonable?
Comment 13 Xisco Faulí 2024-03-15 10:42:21 UTC
(In reply to Noel Grandin from comment #11)
> (In reply to Xisco Faulí from comment #8)
> > Hi Noel,
> > I see some new commits in
> > https://chromium.googlesource.com/skia/+log/refs/heads/chrome/m116 since
> > 2ddcf18. Would it make sense to upgrade to the latest d2c2112 to see whether
> > it fixes the issue ?
> 
> It would, but upgradning skia is a painful process. We replace their build
> scripts (because they use some other build tool that we don't support), and
> their API sometimes changes or move around, which means we then need to
> adapt our code.
> 
> Not something I feel sufficiently enthusiastic to attempt.

Hi Noel,
My question above was more about upgrading to a newer commit in the same branch, m116, rather than upgrading to a newer branch. I wouldn't expect any API change in the same branch.
Comment 14 Noel Grandin 2024-03-15 11:39:11 UTC
(In reply to Xisco Faulí from comment #13)
> My question above was more about upgrading to a newer commit in the same
> branch, m116, rather than upgrading to a newer branch. I wouldn't expect any
> API change in the same branch.

You are more than welcome to make that change
Comment 15 Commit Notification 2024-03-16 22:23:34 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/39663a323c3330c18b610fcdc9e9c75ddac770f1

tdf#160036 Enable SKSL when using Skia/Raster

It will be available in 24.8.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.
Comment 16 Patrick (volunteer) 2024-03-16 22:42:48 UTC
I have committed a fix this bug. The fix should be in tomorrow's (08 March 2024) nightly master builds:

https://dev-builds.libreoffice.org/daily/master/current.html

Note for macOS testers: the nightly master builds install in /Applications/LibreOfficeDev.app. These builds are not codesigned like regular LibreOffice releases so you will need to execute the following Terminal command after installation but before you launch /Applications/LibreOfficeDev:

xattr -d com.apple.quarantine /Applications/LibreOfficeDev.app
Comment 17 V Stuart Foote 2024-03-17 00:07:30 UTC
(In reply to Patrick Luby (volunteer) from comment #16)
Thanks Patrick! Will take it for a spin when it posts...
Comment 18 Patrick (volunteer) 2024-03-17 00:08:58 UTC
Note: I posted the wrong date. The fix will be in tomorrow's (17 March 2024) nightly master build.
Comment 19 Commit Notification 2024-03-17 06:09:13 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "libreoffice-24-2":

https://git.libreoffice.org/core/commit/0696fa0845feaab96a90dfdce96131998961b50a

tdf#160036 Enable SKSL when using Skia/Raster

It will be available in 24.2.3.

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.
Comment 20 Commit Notification 2024-03-17 11:26:49 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "libreoffice-24-2-2":

https://git.libreoffice.org/core/commit/0ffcf99d3e05cf6405d3aa5fbea281a6b5632c9a

tdf#160036 Enable SKSL when using Skia/Raster

It will be available in 24.2.2.

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.
Comment 21 V Stuart Foote 2024-03-18 16:21:23 UTC
Verified with TB77 2024-03-18 nightly, Win10 in HC Dark mode with Skia raster based rendering.


Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 26f7313f724921a9c6086e6c2a93fbe1c48635fa
CPU threads: 8; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: CL threaded
Comment 22 Alistair Saywell 2024-03-25 00:05:20 UTC
Works for me in nightly build Win-x86_64@tb77-TDF 2024-03-23 03:49:07 . Also fixes https://bugs.documentfoundation.org/show_bug.cgi?id=159751
Thanks 
Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: b38974391e8d4bf0d450abfaa86bbccbe1022995
CPU threads: 8; OS: Windows 10.0 Build 22631; UI render: Skia/Raster; VCL: win
Locale: en-NZ (en_NZ); UI: en-GB
Calc: CL threaded
Comment 23 loic_1997 2024-03-29 13:09:02 UTC
Thank you all very much for the quick fix. Just tested on the latest official version.
Comment 24 Buovjaga 2024-04-05 17:08:08 UTC
*** Bug 158231 has been marked as a duplicate of this bug. ***
Comment 25 Buovjaga 2024-04-05 17:08:17 UTC
*** Bug 160262 has been marked as a duplicate of this bug. ***
Comment 26 Buovjaga 2024-04-05 17:09:41 UTC
*** Bug 159751 has been marked as a duplicate of this bug. ***
Comment 27 Buovjaga 2024-04-05 17:10:05 UTC
*** Bug 159214 has been marked as a duplicate of this bug. ***
Comment 28 V Stuart Foote 2024-04-05 17:21:34 UTC
*** Bug 160044 has been marked as a duplicate of this bug. ***