Bug 164403 - Selection will not be shown by highlight if it is after page 490
Summary: Selection will not be shown by highlight if it is after page 490
Status: ASSIGNED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
25.2.0.0 alpha0+
Hardware: All All
: medium normal
Assignee: Armin Le Grand (allotropia)
URL:
Whiteboard: target:25.8.0
Keywords: bibisected, bisected, regression
Depends on:
Blocks: Selection CairoSDPR
  Show dependency treegraph
 
Reported: 2024-12-20 21:39 UTC by Adalbert Hanßen
Modified: 2025-01-29 10:35 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
Test case for this bug (65.78 KB, application/vnd.oasis.opendocument.text)
2024-12-20 21:39 UTC, Adalbert Hanßen
Details
Screenshot showing how even complex objects get 'truncated' at the end of page 490 (81.92 KB, image/png)
2025-01-10 15:32 UTC, Armin Le Grand (allotropia)
Details
screenshot (105.62 KB, image/png)
2025-01-22 19:23 UTC, BogdanB
Details
Example how 'my' local cairo version behaves at that 'magic' vertical boundary (398.45 KB, video/webm)
2025-01-28 14:51 UTC, Armin Le Grand (allotropia)
Details
video testing the bug (16.60 MB, video/mp4)
2025-01-28 18:27 UTC, BogdanB
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Adalbert Hanßen 2024-12-20 21:39:59 UTC
Created attachment 198196 [details]
Test case for this bug

This is a strange bug in

Version: 25.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 9595fc05608a1e7989c85d6eeb5127828f93c9df
CPU threads: 8; OS: Linux 6.8; UI render: default; VCL: gtk3
Locale: de-DE (de_DE.UTF-8); UI: en-US
Calc: threaded

and also in

Version: 25.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 2f5e75ed134cfd0224e03411ca9d7d81f319c91b
CPU threads: 8; OS: Linux 6.8; UI render: default; VCL: gtk3
Locale: de-DE (de_DE.UTF-8); UI: en-US
Calc: threaded

To reproduce this bug:
    1. Go to page 490 of this file.
    2. Double click on any word: it will be highlighted.
    3. Go to page 491 of this file.
    4. Double click on any word: it WILL NOT be highlighted - which is a bug.

This bug also appeared in other big documents: also on page 491 for the first time.
Therefore I created this simple document with 492 pages. The bug also appears here.

After double-clicking, the double-clicked word is indeed selected: Press Ctl-F and it will appear in the search box! You can also copy it or cut it, all these operations work. You can also change the language of selected text even if the selection is not shown by a highlight.

The bug is not restricted to selecting by double click, it also happens with triple click or with selecting by hovering over some test with the mouse and the left key depressed.
Comment 1 raal 2024-12-20 23:40:22 UTC
confirm with Version: 25.2.0.1.0+ (X86_64) / LibreOffice Community
Build ID: 468d47bdf44a772e989a0adbeb5bb097e3cb4f31
CPU threads: 4; OS: Linux 6.8; UI render: default; VCL: gtk3
Locale: cs-CZ (cs_CZ.UTF-8); UI: en-US
Calc: threaded

work in Version: 7.3.7.2 / LibreOffice Community
Build ID: 30(Build:2)
CPU threads: 4; OS: Linux 6.8; UI render: default; VCL: gtk3
Locale: cs-CZ (cs_CZ.UTF-8); UI: cs-CZ
Ubuntu package version: 1:7.3.7-0ubuntu0.22.04.7
Calc: threaded
Comment 2 raal 2024-12-20 23:59:31 UTC
This seems to have begun at the below commit in bibisect repository/OS linux-64-25.2.
Adding Cc: to Armin Le Grand ; Could you possibly take a look at this one?
Thanks
 7813d2b047401f6212870e4ea557fa2a059f342e is the first bad commit
commit 7813d2b047401f6212870e4ea557fa2a059f342e
Author: Jenkins Build User <tdf@maggie.tdf>
Date:   Tue Aug 27 13:21:18 2024 +0200

    source bf3cd8e50f886084500e3a8ff72ff4ffab05ddd7

172394: CairoSDPR: Make using SDPR dependent of ExperimentalMode | https://gerrit.libreoffice.org/c/core/+/172394
Comment 3 Armin Le Grand (allotropia) 2025-01-08 20:39:17 UTC
Using DISABLE_SYSTEM_DEPENDENT_PRIMITIVE_RENDERER=1 as EnvVar to disable CairoSDPR indeed shows that it works then but not with CiaroSDPR. No idea whats going on, have to dig into it.
Page 490 'smells' like integer overrun somewhere -> traditionally these were 32bit, this SW uses twips, page height is 297mm, 29700 100thmm. I found

    public static float twipsToHMM(float twips) {
        return (twips * 127 + 36) / 72;
    }

turning this around means ((29700 * 72) - 36) / 127 twips per page, that are 16837.* twips per page. For 490 pages we get to 8250380. Another hint for a numerical problem is that it works when going to double pages - then it should fail at 2*490 -> 980 pages i would guess. Indeed width/height in SdrPage are tools::Long nowadays, but often put in vars/handed over to functions that still use smaller-sized integers and get cut/turnaround.

Have to dig into it...
Comment 4 Armin Le Grand (allotropia) 2025-01-09 21:18:30 UTC
Tried some stuff (BTW the coordinates I calculated are pretty close to what I see in debugger):
- paint uses CairoPixelProcessor2D::processUnifiedTransparencePrimitive2D, checked that to be correct (would have popped up earlier when not)
- Cairo does not report any errors, cairo_status_t checked in processBasePrimitive2D is always okay. Also in temporary CairoPixelProcessor2D used to render the content
- I tried to add a 'cairo_identity_matrix(mpRT);' since it renders pixel-based, but made no difference (does work and not hort, though)
- Render goes to a temp cairo surface first. I tried to flush it using 'cairo_surface_flush(pContent);' but made no difference
- I switched off DoubleBuffering for the Overlay to force direct rendering to screen (in SdrPaintView::CreateOverlayManager) but same effect...?
- checked the created CairoPath, all looks good numerically (uses the mentioned high coordinates, but it's all double and not somehow 'cut-off')
- made the page one mm higher: Then it happens INSIDE page 489, thus NOT dependent on page itself. When moving the paragraphs together to see at which LINE it happens I see that the created selection is CUT OFF vertically, thus created not with the needed height, not the same height as one line above.
- the bottom coordinate is 8388656
Comment 5 BogdanB 2025-01-09 21:49:15 UTC
(In reply to Armin Le Grand (allotropia) from comment #4)
> - the bottom coordinate is 8388656

8388656 = 2 at power 23
Comment 6 Armin Le Grand (allotropia) 2025-01-10 15:05:20 UTC
Thanks BogdanB - yes, it smells like a numerical problem, but where? Hopefully not in Cairo itself, then we have a problem...
Comment 7 Armin Le Grand (allotropia) 2025-01-10 15:12:23 UTC
Added a rectangle shape at bottom of page 490. If you play around with it you can see it's bottom being clipped away at the end of the page, slightly below last line. Same with other shapes, you can interactively watch that already when constructing the shape moving around the mouse...
Not only the shape content but also the outline is 'clipped' in a way that all coordinates bigger than that magic 'boundary' get reset to that maximum. Since it works with non-CairoSDPR i start to fear that Cairo is doing that with cairo_path_t data ?!?
Comment 8 Armin Le Grand (allotropia) 2025-01-10 15:32:10 UTC
Created attachment 198471 [details]
Screenshot showing how even complex objects get 'truncated' at the end of page 490
Comment 9 Armin Le Grand (allotropia) 2025-01-10 20:39:29 UTC
It *is* Cairo :-( I added code to CairoPixelProcessor2D::paintPolyPoylgonRGBA to transform our basegfx::B2DPolyPolygon self to screen coordinates and paint that. This means:
- the transformations are correct
- the geometry is correct
- the setup for Cairo is correct
- Cairo creates errors when having to transform polygons with coordinates bigger than 8388656 which is 2^23

That is sad. Probably also happens with smaller -8388656 and also in X-coordinate.

I *could* always transform the polygon to screen coordinates, but that will kill the PolyPolygon buffering mechanism; These are held in cairo_path_t form at the original PolyPolygon to not need to transform them every time, but let Cairo do that. The idea is that when Cairo impl may use HW on the system to do that while rendering the PolyPolygon from the object model would have to be converted just once to Cairo form and used multiple times...

The only alternative I see is to only do that when that limit is reached, but that would mean to test that every time for every PolyPolygon and having double code for everything that has to hand over a PolyPolygon to Cairo :-(

Does someone know a Cairo developer...? BTW Cairo does NOT set the error variable which we test and would have shown that earlier...
Comment 10 Caolán McNamara 2025-01-14 09:58:58 UTC
I wonder if this is related to:
https://gitlab.freedesktop.org/cairo/cairo/-/issues/252

"The current format uses 24 bits to address the pixel and eight bits to address fractions of a pixel, leaving the valid coordinate range in the +-8M range"
Comment 11 Armin Le Grand (allotropia) 2025-01-15 13:49:03 UTC
Thanks Caolan, that explains it - thinking about consequences...
Comment 12 Armin Le Grand (allotropia) 2025-01-16 11:38:16 UTC
Thus Cairo is offering a full-fledged double precision API but internally uses a 24.8 fixed-float format. This seems to be the case for linux and windows implementations. Due to the API being on double precision this is an implementation detail, but it has to be taken into account when using cairo. It is not officially documented what is a shame - it should be somewhere in the readmes/documentation at a prominent place.
24.8 means coordinates are limited to +/- 2^23 (usually minus one) and [0/256 .. 255/256] places after comma. This is unfortunately not limited to the view coordinates - that would be acceptable, discrete/pixel areas of that size should be fine, BUT used with transformations. Thus when having any coordinates beyond that limit - and with double precision and transformations we have that here - cairo just 'cuts' the coordinates to it's min/max 24.8 format. This is not acceptable for e.g. a cad system, and even leads to problems in something like an office suite as we see. It implies that using cairo in a safe way means to basically use linear algebra yourself in your app and use cairo only without transformations and with discrete view coordinates (pixels). AND to know about that limitation in the 1st place ...

What a bad surprise.

Well, cairo works well and that problem happens until now in one place in writer below page 490. Let's be pragmatic. Key is to *detect* the situation. Always doing own transformations is possible, but not nice for explained reasons (see comment 9) and rarely needed.
Cairo *should* be fair, document this and allow you to test/ask if this limit exists in the version you are using, but it does not. Thus I suppose to detect that situation as 'cheap' as possible and react to it - e.g. in writer when below page 490.

With detecting as cheap as possible I mean: Naturally you would start to test your geometry to be painted if it is overlapping or outside that range defined by 24.8, but that would already involve transforming all geometry by world and view transformations. I think it should be okay to just take the target area (starting at 0,0 and having a size in pixels) and transforming that back to logical world coordinates. Then this needs to be checked only if view transformation changes, not object transformation. Usually view transformation is setup and all/many objects are painted to it using object transformations. Detection should then only be needed when view transform changes. Testing that...
Comment 13 Armin Le Grand (allotropia) 2025-01-16 21:06:31 UTC
Getting forward, but makes things not easier (I dislike that because of that sh*t I have to partially crack ope a up-to-now very clean structure of transformation, especially it all that is correct and I have to do it to fix an error in other SW).
Have a version that does correctly detect when to do and does that correction successfully for CairoPixelProcessor2D::paintPolyPoylgonRGBA. It indeeds need to detect that correction is needed only once per ViewTransformation, this is once per CairoPixelProcessor2D since ViewTransformation is initialized when constructing, but never changed. IF it may be changed in the future that can be detected, but for now assume it to be fixed for a CairoPixelProcessor2D used as tooling to render.
It tries to not change the ViewInformation2D local to CairoPixelProcessor2D, that may lead to other problems e.g. users of CairoPixelProcessor2D reading that values - these would get potentially an 'adapted' ViewTransformation. Instead, only do adaptions where and when stuff is handed to cairo. This also allows to use all testing/calulating unchanged - all that works perfectly (the error is elsewhere).
This will need some thorough work to detect all places that is/will be needed...
Comment 14 Armin Le Grand (allotropia) 2025-01-20 19:15:05 UTC
Have now a solution - detecting if cairo has the fault once per office runtime, detecting if workaround needed once per CairoSDPR and having adapted the helpers in the renderer to render in view coordinates when needed - self-transforming, using our working basegfx transformations. Luckily this will not need much runtime and will rarely trigger - SW after page 490, maybe draw/impress could construct pages that big in the settings. Also important: For tiled rendering a single SW page is used, thus might also run into that after MAAAANNNY pages...
Comment 15 Commit Notification 2025-01-21 15:50:51 UTC
Armin Le Grand (Collabora) committed a patch related to this issue.
It has been pushed to "master":

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

tdf#164403 CairoSDPR: Solve Cairo's 24.8 coordinate problem

It will be available in 25.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 Armin Le Grand (allotropia) 2025-01-21 15:51:20 UTC
Fix on https://gerrit.libreoffice.org/c/core/+/180529. Please also read the commit text with explanations.
Comment 17 BogdanB 2025-01-22 19:21:27 UTC
Armin, seems to be ok on master

Version: 25.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 21830c58f27977ca4a1dbacd2f41583c9f3ae775
CPU threads: 16; OS: Linux 6.8; UI render: default; VCL: gtk3
Locale: ro-RO (ro_RO.UTF-8); UI: en-US
Calc: threaded
Comment 18 BogdanB 2025-01-22 19:23:26 UTC
Created attachment 198680 [details]
screenshot

Armin, I don't know if it is related to your commit, but if I select text I get a very bold border for selection. See screenshot.
Comment 19 Armin Le Grand (allotropia) 2025-01-28 13:25:47 UTC
Hi BogdanB, thanks! Was a view days off, taking a look - is that below or above the 'magic' boundary...?
Comment 20 Armin Le Grand (allotropia) 2025-01-28 14:25:49 UTC
Hmmm. have now

Version: 25.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 76324a5c75b04c0288d8636b3e270f0273ce4e65
CPU threads: 16; OS: Linux 6.8; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

and did use the bugdoc with the 490+ pages, but could not reproduce, neither above nor below page 490.
@BogdanB: How to reproduce...?
Comment 21 Armin Le Grand (allotropia) 2025-01-28 14:51:31 UTC
Created attachment 198802 [details]
Example how 'my' local cairo version behaves at that 'magic' vertical boundary
Comment 22 BogdanB 2025-01-28 18:25:22 UTC
> and did use the bugdoc with the 490+ pages, but could not reproduce, neither
> above nor below page 490.
> @BogdanB: How to reproduce...?

From the edn of the page 490, make a selection and change the zoom in and out. Or try 491, selection, zoom in and out.
Comment 23 BogdanB 2025-01-28 18:27:36 UTC
Created attachment 198810 [details]
video testing the bug
Comment 24 Armin Le Grand (allotropia) 2025-01-28 19:47:02 UTC
Ah, got it now. It seems to happen when using

--without-system-cairo

in the configuration, as sberg mentioned in https://gerrit.libreoffice.org/c/core/+/180529. I built a version for that. The effect with the zoom is changing the visible area. The corrections I added happen as soon as not the whole visible area is inside the 'harmless' area. Will/should also happen when scrolling up/down.

Will have to look, but may be connected to the negative coordinates popping up when using --without-system-cairo.
@BogdanB: Are you using this? Do you have a version with '--with-system-cairo' and it does not happen there...?
Comment 25 BogdanB 2025-01-28 19:52:26 UTC
If you can tell me technically what to do in order to test I could help you. 
I dont remember my setup when building.
Comment 26 BogdanB 2025-01-28 19:55:52 UTC
grep cairo config.log
configure:41826: checking for gtk+-3.0 >= 3.20 gmodule-no-export-2.0 glib-2.0 >= 2.38 atk >= 2.28.1 cairo
configure:41833: $PKG_CONFIG --exists --print-errors "gtk+-3.0 >= 3.20 gmodule-no-export-2.0 glib-2.0 >= 2.38 atk >= 2.28.1 cairo"
configure:41850: $PKG_CONFIG --exists --print-errors "gtk+-3.0 >= 3.20 gmodule-no-export-2.0 glib-2.0 >= 2.38 atk >= 2.28.1 cairo"
configure:46779: checking whether to use non-standard RGBA32 cairo pixel order
configure:46799: checking whether to use the system cairo
configure:46810: checking for cairo >= 1.12.0 
configure:46817: $PKG_CONFIG --exists --print-errors "cairo >= 1.12.0 "
configure:46834: $PKG_CONFIG --exists --print-errors "cairo >= 1.12.0 "
config.status:1927: creating config_host/config_cairo_canvas.h
config.status:2097: config_host/config_cairo_canvas.h is unchanged
config.status:1927: creating config_host/config_cairo_rgba.h
config.status:2097: config_host/config_cairo_rgba.h is unchanged
pkg_cv_CAIRO_CFLAGS='-I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16'
pkg_cv_CAIRO_LIBS=-lcairo
pkg_cv_GTK3_CFLAGS='-pthread -I/usr/include/gtk-3.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/gtk-3.0 -I/usr/include/gio-unix-2.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/pango-1.0 -I/usr/include/fribidi -I/usr/include/harfbuzz -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/x86_64-linux-gnu -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16'
pkg_cv_GTK3_LIBS='-lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -lharfbuzz -lcairo-gobject -lgdk_pixbuf-2.0 -lgio-2.0 -lgmodule-2.0 -pthread -latk-1.0 -lgobject-2.0 -lglib-2.0 -lcairo'
CAIRO_CFLAGS='-isystem /usr/include/cairo -isystem /usr/include/glib-2.0 -isystem /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem /usr/include/pixman-1 -isystem /usr/include/uuid -isystem /usr/include/freetype2 -isystem /usr/include/libpng16'
CAIRO_LIBS=' -lcairo'
GTK3_CFLAGS='-pthread -isystem /usr/include/gtk-3.0 -isystem /usr/include/at-spi2-atk/2.0 -isystem /usr/include/at-spi-2.0 -isystem /usr/include/dbus-1.0 -isystem /usr/lib/x86_64-linux-gnu/dbus-1.0/include -isystem /usr/include/gtk-3.0 -isystem /usr/include/gio-unix-2.0 -isystem /usr/include/cairo -isystem /usr/include/pango-1.0 -isystem /usr/include/harfbuzz -isystem /usr/include/pango-1.0 -isystem /usr/include/fribidi -isystem /usr/include/harfbuzz -isystem /usr/include/cairo -isystem /usr/include/gdk-pixbuf-2.0 -isystem /usr/include/x86_64-linux-gnu -isystem /usr/include/libmount -isystem /usr/include/blkid -isystem /usr/include/atk-1.0 -isystem /usr/include/cairo -isystem /usr/include/glib-2.0 -isystem /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem /usr/include/pixman-1 -isystem /usr/include/uuid -isystem /usr/include/freetype2 -isystem /usr/include/libpng16 -DGDK_DISABLE_DEPRECATED -DGTK_DISABLE_DEPRECATED'
GTK3_LIBS=' -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -lharfbuzz -lcairo-gobject -lgdk_pixbuf-2.0 -lgio-2.0 -lgmodule-2.0 -pthread -latk-1.0 -lgobject-2.0 -lglib-2.0 -lcairo'
Comment 27 BogdanB 2025-01-28 19:56:51 UTC
./configure | grep cairo
configure: WARNING: dotnet not found, disabling .NET support
<string>:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
<string>:1: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead
<string>:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
<string>:1: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead
<string>:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
<string>:1: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead
<string>:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
<string>:1: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead
checking for gtk+-3.0 >= 3.20 gmodule-no-export-2.0 glib-2.0 >= 2.38 atk >= 2.28.1 cairo... yes
checking whether to use non-standard RGBA32 cairo pixel order... no
checking whether to use the system cairo... yes
checking for cairo >= 1.12.0 ... yes
config.status: creating config_host/config_cairo_canvas.h
config.status: config_host/config_cairo_canvas.h is unchanged
config.status: creating config_host/config_cairo_rgba.h
config.status: config_host/config_cairo_rgba.h is unchanged
Comment 28 Armin Le Grand (allotropia) 2025-01-28 20:07:07 UTC
Fix is at https://gerrit.libreoffice.org/c/core/+/180860. The version test for cairo in impl_cairo_set_hairline did not trigger wen using '--with-system-cairo' which means cairo version was/is < 18 in that case.
Comment 29 Armin Le Grand (allotropia) 2025-01-28 20:10:59 UTC
@BogdanB: Thanks, found and fixed :-) And Comment 27 shows that

checking whether to use the system cairo... yes
checking for cairo >= 1.12.0 ... yes

you use(d) --with-system-cairo, thus your cairo version seems to have been lower 1.18 - what is good since it uncovered that error ;-) thanks!
Comment 30 Commit Notification 2025-01-29 10:35:30 UTC
Armin Le Grand (Collabora) committed a patch related to this issue.
It has been pushed to "master":

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

tdf#164403 CairoSDPR: adapt hairline width

It will be available in 25.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.