Bug 148569 - Character highlighting: custom color color picker shows wrong colors with Skia raster on macOS
Summary: Character highlighting: custom color color picker shows wrong colors with Ski...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
7.4.0.0 alpha0+
Hardware: All macOS (All)
: medium normal
Assignee: Patrick (volunteer)
URL:
Whiteboard: target:25.2.0 target:24.8.0.0.beta2 t...
Keywords:
Depends on:
Blocks: Skia-macOS
  Show dependency treegraph
 
Reported: 2022-04-13 13:51 UTC by Telesto
Modified: 2024-06-27 14:50 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
Screenshot (443.18 KB, image/png)
2022-04-13 13:54 UTC, Telesto
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Telesto 2022-04-13 13:51:37 UTC
Description:
Character highlighting: custom color color picker doesn't look properly with skia raster on macOS

Steps to Reproduce:
1. Open Writer
2. Click expand button for character highlighting
3. More colors


Actual Results:
See screenshot

Expected Results:
Normal look


Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 7.4.0.0.alpha0+ / LibreOffice Community
Build ID: fbf739198aa7f02975d531521c6525073783c7f1
CPU threads: 8; OS: Mac OS X 12.2.1; UI render: Skia/Raster; VCL: osx
Locale: nl-NL (nl_NL.UTF-8); UI: en-US
Calc: threaded
Comment 1 Telesto 2022-04-13 13:54:07 UTC
Created attachment 179530 [details]
Screenshot
Comment 2 Aron Budea 2022-06-04 21:57:25 UTC
Confirmed using LO 7.4 daily build from 06-04 on macOS, with Skia/Raster.
Comment 3 Luboš Luňák 2022-06-16 09:38:03 UTC
What does 'doesn't look properly' mean?
Comment 4 Aron Budea 2022-06-16 09:41:51 UTC
(In reply to Luboš Luňák from comment #3)
> What does 'doesn't look properly' mean?
As can be seen in the screenshot, the colors are incorrect.
Comment 5 Aron Budea 2022-06-16 23:11:20 UTC
Happens with Skia/Metal as well. And only the large color area and the color band has wrong color, the small color preview in the bottom left corner and the actual color that is used in the end are both correct.
Comment 6 QA Administrators 2024-06-16 03:17:17 UTC Comment hidden (obsolete)
Comment 7 Telesto 2024-06-16 08:43:03 UTC
Still present with
Version: 24.8.0.0.alpha1+ (X86_64) / LibreOffice Community
Build ID: d2eab48f697a1e6097778158f623f11306ac7a3d
CPU threads: 8; OS: macOS 14.3; UI render: Skia/Raster; VCL: osx
Locale: nl-NL (nl_NL.UTF-8); UI: en-US
Calc: threaded

not present with non-skia rendering
Version: 24.8.0.0.alpha1+ (X86_64) / LibreOffice Community
Build ID: d2eab48f697a1e6097778158f623f11306ac7a3d
CPU threads: 8; OS: macOS 14.3; UI render: default; VCL: osx
Locale: nl-NL (nl_NL.UTF-8); UI: en-US
Calc: threaded
Comment 8 Telesto 2024-06-16 08:44:04 UTC
@Patrick
You might be interested in this one
Comment 9 Patrick (volunteer) 2024-06-16 13:27:03 UTC
(In reply to Telesto from comment #8)
> @Patrick
> You might be interested in this one

Like you, I see the same color distortion with both Skia/Metal and Skia/Raster but not with Skia disabled.

To me, it looks as if all of the colors have "whitened" or all of the colors are semi-transparent against a white background when using either Skia setting.

When I get some time, I will see if I can find the code that draws the colors in this dialog.
Comment 10 Patrick (volunteer) 2024-06-16 23:54:06 UTC
So I've narrowed down where the bug is occurring and it is in SkiaSalGraphicsImpl::drawPixel(). Still need to do more debugging.

One question: does this bug occur with Skia/Vulkan and/or Skia/Raster on Windows and/or Linux? Or is this just a Skia on macOS bug?
Comment 11 Patrick (volunteer) 2024-06-17 00:53:03 UTC
(In reply to Patrick Luby (volunteer) from comment #10)
> One question: does this bug occur with Skia/Vulkan and/or Skia/Raster on
> Windows and/or Linux? Or is this just a Skia on macOS bug?

I submitted a fix in the following patch. At least on macOS, this bug only occurs with a Retina display. If I force non-Retina resolution by setting my Terminal enviroment variables to SAL_FORCE_HIDPI_SCALING=1, the bug does not occur. So, I think that this bug only occurs when drawing a pixel with scaling:

https://gerrit.libreoffice.org/c/core/+/168973
Comment 12 Telesto 2024-06-17 09:01:47 UTC
@V Stuart Foote
From comment 10: One question: does this bug occur with Skia/Vulkan and/or Skia/Raster on Windows and/or Linux? Or is this just a Skia on macOS bug?

Would you mind to test this on Windows with a HiDPI screen?
Comment 13 Commit Notification 2024-06-17 12:52:33 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "master":

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

tdf#148569 set extra drawing constraints when scaling

It will be available in 25.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.
Comment 14 Patrick (volunteer) 2024-06-17 12:59:49 UTC
I have committed a fix this bug. The fix should be in tomorrow's (18 June 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 15 Commit Notification 2024-06-17 15:57:09 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/577f68bc29273c66466dbde5128fec66b01868b4

tdf#148569 set extra drawing constraints when scaling

It will be available in 24.8.0.0.beta2.

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 V Stuart Foote 2024-06-17 16:12:46 UTC
(In reply to Telesto from comment #12)
> @V Stuart Foote
> From comment 10: One question: does this bug occur with Skia/Vulkan and/or
> Skia/Raster on Windows and/or Linux? Or is this just a Skia on macOS bug?
> 
> Would you mind to test this on Windows with a HiDPI screen?

On Win10 with 4K monitor, neither Skia Vulkan nor Skia raster rendering causes that distortion in the 2D color chart for the 'Pick a color' dialog, colors are fully saturated and follow the Hue of the picker. No apparent issue as the graphic is scaled.

Version: 24.8.0.0.alpha1+ (X86_64) / LibreOffice Community
Build ID: 6d39b1a6068bbbd5ca4947f668f989dbfb73342d
CPU threads: 8; OS: Windows 10 X86_64 (10.0 build 19045); UI render: Skia/Raster; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: CL threaded
Comment 17 Commit Notification 2024-06-18 08:23:19 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "libreoffice-24-2":

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

tdf#148569 set extra drawing constraints when scaling

It will be available in 24.2.5.

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 18 Timur 2024-06-24 18:52:13 UTC
I tested DOCX with lot of pages and images that loads slowly in newer versions, due to multile perf regressions.
In 24.8+ this commit caused a fileopen slowndown from 4:50 to 5:40.
DOCX is private and I cannot open a ticket with it. 
Rather, I ask you to reconsider this commit.
Comment 19 Timur 2024-06-24 18:54:15 UTC
Note: test and slowdown are in Linux GTK3.
Comment 20 Timur 2024-06-24 19:00:17 UTC
For the moment, please revert 24.2.
Comment 21 Patrick (volunteer) 2024-06-24 19:11:46 UTC
(In reply to Timur from comment #20)
> For the moment, please revert 24.2.

How about I just limit the change to macOS only? The macOS Retina display of "1 pixel is backed by 4 pixels" appears to be unique to macOS so I think my fix is overkill for Windows and Linux.

I can get patches in Gerrit today back from master back to 24.2 later today.
Comment 22 Timur 2024-06-24 19:23:17 UTC
Thanks. Please do. It would be good to find large DOCX with lot of images in the existing tickets to retest, but I could not find it.
Comment 23 Patrick (volunteer) 2024-06-24 20:14:02 UTC
(In reply to Timur from comment #22)
> Thanks. Please do. It would be good to find large DOCX with lot of images in
> the existing tickets to retest, but I could not find it.

OK. I have submitted the following patch that limits the fix for this bug to only macOS with a Retina display. If you see anything that needs changing for Linux and/or Windows, please let me know and I can update the patch this evening:

https://gerrit.libreoffice.org/c/core/+/169475
Comment 24 Commit Notification 2024-06-25 15:05:44 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "master":

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

Related tdf#148569: do not apply macOS fix to non-macOS platforms

It will be available in 25.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.
Comment 25 Patrick (volunteer) 2024-06-25 15:18:15 UTC
So I have committed a patch that limits the fix for this bug to only macOS. Can anyone confirm that this bug does *not* occur on Linux and/or Windows in tomorrow's (26 June 2024) nightly master build?

On macOS, this bug only occurred with Retina (2x resolution) displays so I am not sure what the equivalent Linux and Windows display settings are. You may need to run LibreOfficeDev from the command line with the following environment variable set to emulate what I see on macOS:

export SAL_FORCE_HIDPI_SCALING=2
Comment 26 Commit Notification 2024-06-27 14:50:18 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "libreoffice-24-2":

https://git.libreoffice.org/core/commit/4982d78f887440cbb12aec512a757dd7d3a75f43

Related tdf#148569: do not apply macOS fix to non-macOS platforms

It will be available in 24.2.5.

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 27 Commit Notification 2024-06-27 14:50:21 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/16be4df75b23ec68c758e203a08af0f9486ebafb

Related tdf#148569: do not apply macOS fix to non-macOS platforms

It will be available in 24.8.0.0.beta2.

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.