Description: https://bugs.documentfoundation.org/show_bug.cgi?id=161709 changed cell highlighting a few times in different ways. Since the last changes the cell highlighting in Calc no longer is symmetrical which looks unexpected. Steps to Reproduce: Open calc and select cell Actual Results: Cell highlighting is off balance. Expected Results: Evenly / symmetrical cell highlighting. Reproducible: Always User Profile Reset: No Additional Info: Version: 25.2.0.0.alpha0+ (AARCH64) / LibreOffice Community Build ID: 715b5c430e14609b15d18a5a607f13b9476b3c2d CPU threads: 12; OS: macOS 14.6.1; UI render: Skia/Metal; VCL: osx Locale: en-US (en_US.UTF-8); UI: en-US Calc: threaded
Created attachment 196041 [details] 2024-08-27 cell highlighting off balance
Created attachment 196043 [details] Screenshot of current 25.2 master Hi steve, this is what I'm getting on kf5 (Plasma 5.27). You should be seeing the same thing. Are you running LO compiled from source? Or have you downloaded the latest nightly build?
Created attachment 196044 [details] Screenshot using dark mode Here's a screenshot with dark background for better visibility. Unfortunately I do now have access to a Mac to test it... maybe Heiko can confirm?
@Heiko, can you please confirm this on macOS?
I confirm the issue in question (using my system and LO in dark mode) for Version: 24.8.0.3 (AARCH64) / LibreOffice Community Build ID: 0bdf1299c94fe897b119f97f3c613e9dca6be583 CPU threads: 10; OS: macOS 14.6.1; UI render: Skia/Metal; VCL: osx Locale: de-DE (de_DE.UTF-8); UI: de-DE Calc: threaded Version: 25.2.0.0.alpha0+ (AARCH64) / LibreOffice Community Build ID: 715b5c430e14609b15d18a5a607f13b9476b3c2d CPU threads: 10; OS: macOS 14.6.1; UI render: Skia/Metal; VCL: osx Locale: de-DE (de_DE.UTF-8); UI: de-DE Calc: threaded
Any idea why this might be affecting only macOS?
Patrick: this might be something to do with HiDPI/Retina, any tips for Rafael? Rafael's earlier patches: https://gerrit.libreoffice.org/q/message:161709+branch:master
Created attachment 196064 [details] Snapshot of LibreOffice 24.8.0.3: line widths drawn correctly
Created attachment 196065 [details] Snapshot of master: top and left lines are too narrow and small sqaures drawn in upper-right and bottom-left corners
Created attachment 196066 [details] Snapshot of master without Rafael's patches: top and left lines are split into two lines Note: I reverted the following commits on master to get this snapshot: - Commit 3b4a8406c2e7f4f6a7ac43e192fdb8ef266d4b6d - Commit 81f2185c111b7d8154a2de128d490a0a9e822f19
So here is what I see: 1. In LibreOffice 24.8.0.3 (before Rafael's changes), the width of all the lines are equal and look OK (see snapshot in attachment #196064 [details]) 2. In master (after Rafael's changes), there are small squares drawn in the upper-right and bottom-left corners (see snapshot in attachment #196065 [details]) 3. I reverted Rafael's two patches in my master build. I expected to see the same drawn output as LibreOffice 24.8.0.3. But instead, the top and left lines are drawn as two separate lines (see snapshot in attachment #196066 [details]). So, I don't think that this bug is due to Rafael's 2 patches and I think that something else changed after LibreOffice 24.8.0.3. To me, it looks like the thick lines are really two thinner lines that are supposed to be drawn next to each other. So maybe in master, the two lines are being drawn on top of each other which renders as a single, thinner line.
(In reply to Patrick (volunteer) from comment #11) > So here is what I see: > > 1. In LibreOffice 24.8.0.3 (before Rafael's changes), the width of all the > lines are equal and look OK (see snapshot in attachment #196064 [details]) > > 2. In master (after Rafael's changes), there are small squares drawn in the > upper-right and bottom-left corners (see snapshot in attachment #196065 [details] > [details]) > > 3. I reverted Rafael's two patches in my master build. I expected to see the > same drawn output as LibreOffice 24.8.0.3. But instead, the top and left > lines are drawn as two separate lines (see snapshot in attachment #196066 [details] > [details]). > > So, I don't think that this bug is due to Rafael's 2 patches and I think > that something else changed after LibreOffice 24.8.0.3. > > To me, it looks like the thick lines are really two thinner lines that are > supposed to be drawn next to each other. So maybe in master, the two lines > are being drawn on top of each other which renders as a single, thinner line. Could the missing piece be 1d1f47e6ca6367bf8fb828dd09cce197a96f643c, which has not yet been backported to 24.8? I saw Rafael talking about it when considering what to backport next.
Created attachment 196067 [details] Snapshot of master without commit 1d1f47e6ca6367bf8fb828dd09cce197a96f643c
(In reply to Buovjaga from comment #12) > Could the missing piece be 1d1f47e6ca6367bf8fb828dd09cce197a96f643c, which > has not yet been backported to 24.8? I saw Rafael talking about it when > considering what to backport next. Commit 1d1f47e6ca6367bf8fb828dd09cce197a96f643c is definitely the cause. I reverted it in my master build and all of the lines have equal widths again (see snapshot in attachment #196067 [details]).
(In reply to Patrick (volunteer) from comment #14) > Commit 1d1f47e6ca6367bf8fb828dd09cce197a96f643c is definitely the cause. I > reverted it in my master build and all of the lines have equal widths again > (see snapshot in attachment #196067 [details]). FWIW, attachment 196067 [details] does not look exactly symmetric (regarding the cell grid lines).
(In reply to Patrick (volunteer) from comment #14) > Commit 1d1f47e6ca6367bf8fb828dd09cce197a96f643c is definitely the cause. I > reverted it in my master build and all of the lines have equal widths again > (see snapshot in attachment #196067 [details]). Update: none of Rafael's commits cause this bug. I found that while this bug occurs in my master build, it only occurs with Skia enabled so did some debugging in the Skia code and found that copying the Skia fix for tdf#148569 fixed this bug. I'll commit the following patch as soon as it passes unit tests: https://gerrit.libreoffice.org/c/core/+/172559
(In reply to Patrick (volunteer) from comment #16) > Update: none of Rafael's commits cause this bug. I found that while this bug > occurs in my master build, it only occurs with Skia enabled so did some > debugging in the Skia code and found that copying the Skia fix for > tdf#148569 fixed this bug. Thank you, Patrick for figuring this out. I was suspecting this had something to do with scaling. But I could not test locally on my machine. Anyways, I'm glad this got fixed.
(In reply to Rafael Lima from comment #17) > Thank you, Patrick for figuring this out. > > I was suspecting this had something to do with scaling. But I could not test > locally on my machine. > > Anyways, I'm glad this got fixed. Glad I could help. I think this is just something unique to Skia on macOS. So far, the pattern seems to be that calling SkPaint.paint.setAntiAlias(false) still allows some antialiased drawing to occur in the surface is scaled. Apparently, setting the stroke width and end cap style to squared is the extra settings needed with Skia on macOS.
Patrick Luby committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/33863317e216b1ecd843f5eb584e99998a76305f tdf#162646 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.
Nice work, Patrick! Copying my comment to a Gerrit patch: I have created a cherry pick for Patrick's recent Skia fix that affects the active cell indicator: https://gerrit.libreoffice.org/c/core/+/172504 It makes me confident that we could pick Rafael's two commits to 24-8 and quickly to 24-8-1 as well (including Patrick's patch to 24-8-1). Picks to 24-8: https://gerrit.libreoffice.org/c/core/+/172565 tdf#162006 Use contrast outline for the AutoFill/Cursor handle as well https://gerrit.libreoffice.org/c/core/+/172566 tdf#161709 Remove extra space between cursor and cell edges
Patrick Luby committed a patch related to this issue. It has been pushed to "libreoffice-24-8": https://git.libreoffice.org/core/commit/588a4acb4526e33f69f07aacfe790c620c4a513c tdf#162646 set extra drawing constraints when scaling It will be available in 24.8.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.
Patrick Luby committed a patch related to this issue. It has been pushed to "libreoffice-24-8-1": https://git.libreoffice.org/core/commit/6ce68d330c2c88a72a0af7190e89ded93d4a6e27 tdf#162646 set extra drawing constraints when scaling It will be available in 24.8.1. 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.
I just noticed after a git pull -r in my local master build that this bug is back on macOS. Have any changes been made recently in the Calc code that draws the border? If so, that may give me a clue as to why my fix no longer works.
(In reply to Patrick (volunteer) from comment #23) > Have any changes been made recently in the Calc code that draws the border? > If so, that may give me a clue as to why my fix no longer works. I have not made any further changes. Also, since my original patch [1] on July 18, I could not find any changes to the same portion of the code. [1] 1d1f47e6ca6367bf8fb828dd09cce197a96f643c Maybe try downloading the nightly build to test on your machine.
(In reply to Rafael Lima from comment #24) > I have not made any further changes. > > Also, since my original patch [1] on July 18, I could not find any changes > to the same portion of the code. > > [1] 1d1f47e6ca6367bf8fb828dd09cce197a96f643c > > Maybe try downloading the nightly build to test on your machine. OK. I'll do a git bisect when I have some time. Whatever changed should have happened in the last few days so it shouldn't be much hassle to find the cause. I'll post again when I have some more news and/or a new fix.
FTR I've just tested on Windows with Skia (latest build from Aug 31; your patch is applied) and it is working fine (at least on Skia/Win). So apparently it's something related to Skia/macOS. Unfortunately I can't test on macOS. Version: 25.2.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: 663ad4abfc295741790daa77f733fb33b6013be9 CPU threads: 4; OS: Windows 10 X86_64 (10.0 build 19045); UI render: Skia/Raster; VCL: win Locale: en-US (en_US); UI: en-US Calc: threaded
(In reply to Rafael Lima from comment #26) > FTR I've just tested on Windows with Skia (latest build from Aug 31; your > patch is applied) and it is working fine (at least on Skia/Win). So > apparently it's something related to Skia/macOS. Found the cause and I have uploaded the following patch: https://gerrit.libreoffice.org/c/core/+/172707 When I was working on the previous patch, the active cell border was being drawn in SkiaSalGraphicsImpl::privateDrawAlphaRect(). But even after I rolled my build back to the my previous commit, its now drawn in SkiaSalGraphicsImpl::performDrawPolyPolygon(). Not sure if where the drawing path might have changed so I just added a similar fix for macOS when drawing unantialiased polypolygons that contain only vertical and/or horizontal lines.
Note: I tested today's nightly LibreOffice 24.8 build and the original bug still occurs. So I have backported the patch to the libreoffice-24-8 and libreoffice-24-8-1 branches.
Created attachment 196156 [details] Snapshot of master with https://gerrit.libreoffice.org/c/core/+/172707 So this is what the active cell indicator looks like on macOS with my latest patch. Looks correctly positioned to me.
Patrick Luby committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/9147506aa6c4d5b783a367d89609ec1e8d7fe966 tdf#162646 don't move orthogonal polypolygons 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.
Patrick Luby committed a patch related to this issue. It has been pushed to "libreoffice-24-8": https://git.libreoffice.org/core/commit/2aeb9f13a6442f93f5ed858a461c2a8dfcc45afb tdf#162646 don't move orthogonal polypolygons when scaling It will be available in 24.8.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.
So this bug should now be fixed again in today's (01 September 2024) nightly builds. Can anyone test on a Mac Intel machine? I only have access to Mac Silicon.
Patrick Luby committed a patch related to this issue. It has been pushed to "libreoffice-24-8-1": https://git.libreoffice.org/core/commit/d4c445ff1ab3e89baa59fc5328ef28b686fde459 tdf#162646 don't move orthogonal polypolygons when scaling It will be available in 24.8.1. 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 196173 [details] Screenshot 2024-09-02 Screenshot 2024-09-02 on Apple Silicon Version: 25.2.0.0.alpha0+ (AARCH64) / LibreOffice Community Build ID: 77aa9c210ad79af443191860ee1a7233d2d007cf CPU threads: 12; OS: macOS 14.6.1; UI render: Skia/Metal; VCL: osx Locale: en-US (en_US.UTF-8); UI: en-US Calc: threaded This is much better. It is not perfect or maybe the remaining asymmetry is expected? There is a minimal gap between the whilte cell line and the active cell highlighting on the bottom and right side which does not exist on the left and top side. Is this some side effect of some intended "3D" or shadow appearance?
(In reply to steve from comment #34) > This is much better. It is not perfect or maybe the remaining asymmetry is > expected? There is a minimal gap between the whilte cell line and the active > cell highlighting on the bottom and right side which does not exist on the > left and top side. Is this some side effect of some intended "3D" or shadow > appearance? I switched the document background color to black and I can also see that the right vertical white line is shifted a pixel or so too much to the left just like in attchment #196173. When I disable all Skia options, I don't see any shift so looks like I need to tweak the macOS Skia code more. When I was debugging this previously, my document background was white so I never noticed this new "white inner border".
(In reply to Patrick (volunteer) from comment #35) > I switched the document background color to black and I can also see that > the right vertical white line is shifted a pixel or so too much to the left > just like in attchment #196173. FWIW, I just noticed that the bottom horizontal white line is also shift a pixel or so upward too much.
Found the cause: the inner white rectangle is drawn through a different drawing path than the outer border. The inner white rectangle is drawn in SkiaSalGraphicsImpl::drawPolyLine() with a "hairline" line width (i.e. zero line width). Apparently, a zero line width causes Skia when using a Retina display on macOS and with antialiasing disabled so I used the same fix that is used for unit tests to fix this bug: https://gerrit.libreoffice.org/c/core/+/172742 I'll commit this patch once it passes the automated unit tests.
Patrick Luby committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/bb1ce025ae6479bc3044197f25990d8c41ae09de tdf#162646 suppress drawing hairlines 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.
Created attachment 196187 [details] Snapshot with black document background with https://gerrit.libreoffice.org/c/core/+/172742 Here is a snapshot with my latest fix on Mac Silicon and a black document background so you can see the "inner white border". Looks good to me on Mac Silicon. Hopefully it looks the same on Mac Intel.
Patrick Luby committed a patch related to this issue. It has been pushed to "libreoffice-24-8": https://git.libreoffice.org/core/commit/7f7cfc333bca65a2290bb55354ba0b3abd53044c tdf#162646 suppress drawing hairlines when scaling It will be available in 24.8.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.
Thanks so much Patrick for digging into this and not giving up easily. I report a perfectly symmetric active cell highlighting in Version: 25.2.0.0.alpha0+ (AARCH64) / LibreOffice Community Build ID: bcadc9a6ec5be2541e259f0fa18022c0661c8df7 CPU threads: 12; OS: macOS 14.6.1; UI render: Skia/Metal; VCL: osx Locale: en-US (en_US.UTF-8); UI: en-US Calc: threaded I hope I don't forget to double check intel mac when I am at one, which I am currently not. There is still some inconsistency between light and dark mode, but as that is unrelated to the issue discussed in this bug I filed a follow up: https://bugs.documentfoundation.org/show_bug.cgi?id=162759
Patrick Luby committed a patch related to this issue. It has been pushed to "libreoffice-24-8-1": https://git.libreoffice.org/core/commit/9845f6bde0dc0c2e4a5ee64d69be6f4c30420db3 tdf#162646 suppress drawing hairlines when scaling It will be available in 24.8.1. 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 196237 [details] Screenshot 2024-09-04 intel mac Also verified on Version: 25.2.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: 677750f3f3d21c40e37cc4296673c931a0cb3779 CPU threads: 4; OS: macOS 14.6.1; UI render: Skia/Metal; VCL: osx Locale: en-US (en_US.UTF-8); UI: en-US Calc: threaded