Bug 126184 - UI 5.x Curved connectors path messed up when passed to 6.x
Summary: UI 5.x Curved connectors path messed up when passed to 6.x
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Draw (show other bugs)
Version:
(earliest affected)
6.1.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Michael Weghorn
URL:
Whiteboard: target:6.4.0 target:6.3.3 target:6.2.8
Keywords: bibisected, regression
: 120350 122955 123028 124993 127366 128031 (view as bug list)
Depends on:
Blocks: Connectors
  Show dependency treegraph
 
Reported: 2019-07-01 23:56 UTC by web22
Modified: 2023-05-08 16:10 UTC (History)
15 users (show)

See Also:
Crash report or crash signature:


Attachments
DRAW file created in Libre Office 5.x (16.91 KB, application/vnd.oasis.opendocument.graphics)
2019-07-04 22:39 UTC, web22
Details
How it looks in LibreOffice 5.2 alpha1 (102.23 KB, image/png)
2019-07-05 10:28 UTC, Xisco Faulí
Details
Example reduced (10.25 KB, application/vnd.oasis.opendocument.graphics)
2019-07-06 00:30 UTC, Regina Henschel
Details
Resulting PDF (9.31 KB, application/pdf)
2019-07-06 23:39 UTC, Regina Henschel
Details
RoundedRectangle.odg, orig file for the pdf (10.85 KB, application/vnd.oasis.opendocument.graphics)
2019-07-07 16:14 UTC, Regina Henschel
Details
AdditionalDot.odp, Example with arrow line end (13.22 KB, application/vnd.oasis.opendocument.presentation)
2019-07-07 21:53 UTC, Regina Henschel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description web22 2019-07-01 23:56:03 UTC
Context:
**************
Draw document with curved connectors created in 5.x version. The connectors paths are nicely set between the connected objects.
 
Upgrade of LibreOffice to 6.x version. Same document opened again.

Finding
**************
The connectors paths are now totally messed up: they are doing huge detours.
If try to control their path: doesn't work.
If try to delete a connector and recreate a new one: same, the new connector's path is following a huge arc ans is impossible to reduce.

Expected
**************
- Curved connector path stay the same when upgrading from 5.x to 6.x
- Curved connector path controllable in 6.x
- Curved connector path newly created in 6.x by default make no detour between the 2 connected objects
Comment 1 Roman Kuznetsov 2019-07-04 09:38:07 UTC
please attach your document from LO 5.x here
Comment 2 web22 2019-07-04 22:39:24 UTC
Created attachment 152571 [details]
DRAW file created in Libre Office 5.x
Comment 3 web22 2019-07-04 22:40:12 UTC
Requested file attached
Comment 4 Xisco Faulí 2019-07-05 10:28:56 UTC
Created attachment 152574 [details]
How it looks in LibreOffice 5.2 alpha1
Comment 6 Xisco Faulí 2019-07-05 10:44:54 UTC
I know how to fix it...
Comment 7 Xisco Faulí 2019-07-05 10:46:04 UTC
*** Bug 122955 has been marked as a duplicate of this bug. ***
Comment 8 Xisco Faulí 2019-07-05 10:46:20 UTC
*** Bug 116940 has been marked as a duplicate of this bug. ***
Comment 9 Regina Henschel 2019-07-06 00:30:20 UTC
Created attachment 152596 [details]
Example reduced

I think, that it is duplicate to bug 120350. The "missing" lines there are wrongly placed at the left top of the page and make the flow-chart objects much larger than they are, and therefore force the routing of the connector a wrong way.

I have attached a file, were I have removed most of the objects of the original file. That makes it easier for testing.

Open the file in LO 54 and move the objects to a position left-top of the magic 42cm. Or open the source of the file and remove the additional N command in the path (see bug 120350). In both cases the connectors will be fine, if you then open the file in a current LO version.

Do you have tested, that reintroducing the hack will not again result in bug 37559 and its duplicates?
Comment 10 Xisco Faulí 2019-07-06 13:00:06 UTC
*** Bug 117826 has been marked as a duplicate of this bug. ***
Comment 11 Xisco Faulí 2019-07-06 13:02:42 UTC
*** Bug 120350 has been marked as a duplicate of this bug. ***
Comment 12 Xisco Faulí 2019-07-06 13:03:58 UTC
*** Bug 117910 has been marked as a duplicate of this bug. ***
Comment 13 Xisco Faulí 2019-07-06 16:10:11 UTC
(In reply to Regina Henschel from comment #9)
> Created attachment 152596 [details]
> Do you have tested, that reintroducing the hack will not again result in bug
> 37559 and its duplicates?

Good point, in the end, I've re-introduced the hack and reduce its scope, so testQuadraticCurveTo::TestBody and testViewBoxLeftTop::TestBody pass.
Regarding bug 37559, I'll ask Mike Kaganski to reproduce it with my patch, I can't reproduce it myself. Give it a try if you have a chance too...
Comment 14 Regina Henschel 2019-07-06 18:08:51 UTC
Your patch introduces additional points top-left and bottom-right. It is not as bad as in bug 37559. They are only visible in edit mode at some zoom level and not directly written to ODF file format.

But if the shape is converted to curve, some things happen, where the user cannot know, what happens:

The additional points are written as single moveto add the end of the path. So they could be ignored in opening. But LibreOffice reads them and uses them for calculating frame and bound box. That becomes visible, if the object is rotated.

If the document is written to pptx, the additional points in (to curve converted) shape are written as moveto lineto. And this moveto lineto is kept on reopening. So the geometry gets two zero length paths. As long as "cap" is not implemented for zero length path, they are not visible. But from svg:d definition they should be visible, if a cap is defined in the line style.

And I'm concerned, that it is not clear, why the inner lines of the shapes are not rendered correctly, if the shape is beyond 42cm.
Comment 15 Regina Henschel 2019-07-06 18:40:22 UTC
My test were for your version #3, need some time to look at version #4.
Comment 16 Regina Henschel 2019-07-06 23:39:51 UTC
Created attachment 152607 [details]
Resulting PDF

Some more tests, now with your version 4.
The dots top-left and bottom-right are created and exported to pdf. They are visible in Acrobat Reader and in Chrome.
To see the dots, use a line style with true line width, no hairline, and set the cap to round. The problems from bug 37559 would be there again.
To see the dots in LibreOffice without converting to curve, disable anti-aliasing and reduce zoom, or look at the thumbnails in the slide pane or look at the slides in the slide sorter.

The problem, that the additional points produce a different bounding box exists for custom shapes too, not only for the shapes, which are converted to curve. This difference influences the text layout in Writer, if the shape uses a wrap with text besides the shape, "page wrap" e.g, without contour.
Comment 17 Regina Henschel 2019-07-07 01:10:40 UTC
Use document "Path M L N M L" https://bugs.documentfoundation.org/attachment.cgi?id=148736 from bug 120350 and open it step-by-step. You will come to viewcontactofsdrpathobj.cxx#95. There the pointer pPage is NULL. That will result in a page size of width=21000 and height=29700 in line 106/107 and the in a clip range of x in -21000..42000 and y in -29700..59400. That explains the magic 42cm. But of cause the questions remain, why is pPage=NULL, and why appear the lines, which are dropped because of the clip range, in the top-left corner of the page?
Comment 18 Xisco Faulí 2019-07-07 14:36:22 UTC
Hi Regina,
Could you please attach the original file from comment 16 ?
Comment 19 Regina Henschel 2019-07-07 16:14:52 UTC
Created attachment 152624 [details]
RoundedRectangle.odg, orig file for the pdf
Comment 20 Xisco Faulí 2019-07-07 19:20:39 UTC
(In reply to Regina Henschel from comment #19)
> Created attachment 152624 [details]
> RoundedRectangle.odg, orig file for the pdf

I've just submitted a new patch to gerrit that fixes this specific problem. Just to be clear here: I don't know the code so I'm just doing try/error until I see everything works and the tests pass.
@Regina, if you have a better and more elegant solution here, please go ahead a implement it, you know the code much more than I do
Comment 21 Regina Henschel 2019-07-07 21:53:48 UTC
Created attachment 152632 [details]
AdditionalDot.odp, Example with arrow line end

The version 5 does not work. The dots are still there. In addition a line arrow is shown on a closed polygon.

I think, that it is a wrong attempt. See my comment#17. I think it is necessary to find the root cause for the NULL pointer.
Comment 22 Xisco Faulí 2019-07-08 07:42:06 UTC
Putting this issue back to NEW, if I re-introduce the hack as in https://gerrit.libreoffice.org/#/c/75143/, bug 37559 is reintroduced as well. it has to be fixed in a different way...
Comment 23 Regina Henschel 2019-07-08 18:47:50 UTC
The problems have started with the fix for bug 63955. That fix introduced a clipping to a rectangle, which was build from a page size. But that page size doesn't exist as expected by the fix. That led to bug 97276, bug 97276, bug 98366 and bug 101187. Each of them where "fixed" by adding a special case to the fix from 63955. And now the fix from 63955 leads to the problems in this bug and its duplicates.

If I remove the clipping (currently lines #88 to #121 in viewcontactofsdrpathobj.cxx), all the mentioned cases work fine. Only bug 63955 would not be fixed.

But the problematic document in bug 63955 was created from a binary *.sdd file. The creation process was badly done. I had created a correct conversion at that time, which opens fine.

So my suggestion is, to remove lines #88 to #121 and find a better solution to prevent LibreOffice from crashing with problematic files. [The problem in the file from bug 63955 is a 19km long line.] I would also like to draw attention to the concerns about the fix expressed by Armin in the bug report 63955.
Comment 24 Xisco Faulí 2019-07-08 20:09:08 UTC
(In reply to Regina Henschel from comment #23)
> So my suggestion is, to remove lines #88 to #121 and find a better solution
> to prevent LibreOffice from crashing with problematic files. [The problem in
> the file from bug 63955 is a 19km long line.] I would also like to draw
> attention to the concerns about the fix expressed by Armin in the bug report
> 63955.

Hi Regina,
Thanks for the great analysis, indeed I do confirm removing that chunk of code would fix the issue.

@Caolán, Do you think bug 63955 could be fixed in a different way ? See < https://bugs.documentfoundation.org/show_bug.cgi?id=63955#c19 >
Comment 25 Caolán McNamara 2019-07-17 14:45:31 UTC
I'm all ears on an alternative fix
Comment 26 Michael Weghorn 2019-09-05 15:16:21 UTC
*** Bug 127366 has been marked as a duplicate of this bug. ***
Comment 27 Michael Weghorn 2019-09-06 07:45:49 UTC
(In reply to Caolán McNamara from comment #25)
> I'm all ears on an alternative fix

Given my very limited knowledge of drawinglayer, I'd leave implementing a new approach to somebody more capable in this area...

For now, I've created https://gerrit.libreoffice.org/#/c/78690/ to increase the limits for clipping to use the max paper dimensions. Comments welcome, since I'm far from being an expert in this area...
Comment 28 Commit Notification 2019-09-06 20:40:40 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

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

tdf#126184 Use max paper dimensions to calculate clip region

It will be available in 6.4.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 29 Luke 2019-09-07 15:34:54 UTC
Per the suggestion at the bottom of 
https://cgit.freedesktop.org/libreoffice/core/commit/?id=1dccd6814f1f

I created Bug 127424
Comment 30 Michael Weghorn 2019-09-08 07:35:21 UTC
(In reply to Luke from comment #29)
> Per the suggestion at the bottom of 
> https://cgit.freedesktop.org/libreoffice/core/commit/?id=1dccd6814f1f
> 
> I created Bug 127424

Thanks. Let's mark this one as fixed, since the behaviour described here and in the duplicates is fixed by the above commit.

Pending backport for 6.3: https://gerrit.libreoffice.org/#/c/78759/
Comment 31 Commit Notification 2019-09-08 10:36:23 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

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

tdf#126184 Use max paper dimensions to calculate clip region

It will be available in 6.3.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 32 Xisco Faulí 2019-09-09 23:17:25 UTC
Verified in

Version: 6.4.0.0.alpha0+
Build ID: f4f8bccbd4e2c3979a83d5b2f49e16a99a3a2016
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded

@Michael Weghorn, thanks for fixing this issue!!
Comment 33 Commit Notification 2019-09-12 05:37:46 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "libreoffice-6-2":

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

tdf#126184 Use max paper dimensions to calculate clip region

It will be available in 6.2.8.

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 34 Regina Henschel 2019-10-08 22:59:21 UTC
*** Bug 128031 has been marked as a duplicate of this bug. ***
Comment 35 Aron Budea 2023-05-08 16:09:22 UTC
*** Bug 123028 has been marked as a duplicate of this bug. ***
Comment 36 Aron Budea 2023-05-08 16:09:34 UTC
*** Bug 124993 has been marked as a duplicate of this bug. ***