Bug 122899 - FILEOPEN: ppt: old kind arc from MS Office 97 is broken
Summary: FILEOPEN: ppt: old kind arc from MS Office 97 is broken
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
4.3 all versions
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.3.0
Keywords: bibisected, bisected, regression
Depends on:
Blocks: Shapes PPT
  Show dependency treegraph
 
Reported: 2019-01-23 13:42 UTC by Xisco Faulí
Modified: 2019-04-23 11:54 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
comparison MSO 2010 and LibreOffice 6.3 Master (59.04 KB, image/png)
2019-01-23 13:42 UTC, Xisco Faulí
Details
symphony example where problem began (60.00 KB, application/vnd.ms-powerpoint)
2019-02-06 17:04 UTC, Caolán McNamara
Details
Testdocument for wrong scaling (27.00 KB, application/msword)
2019-03-08 18:25 UTC, Regina Henschel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Xisco Faulí 2019-01-23 13:42:46 UTC
Created attachment 148562 [details]
comparison MSO 2010 and LibreOffice 6.3 Master

This is a follow-up of bug 122204

Steps to reproduce:
1. Open attachment 147680 [details] from bug 122204
2. Go to slide 4.

-> See comparison

Reproduced in

Version: 6.3.0.0.alpha0+
Build ID: 0750be852e865c6c5f6f26365064126c10770f31
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
Comment 1 Xisco Faulí 2019-01-23 13:49:35 UTC
Regression introduced by:

https://cgit.freedesktop.org/libreoffice/core/commit/?id=1cbec9cd986100de185f6dc10301a54f6604e6af

author	Andre Fischer <af@apache.org>	2012-07-09 11:10:26 +0000
committer	Caolán McNamara <caolanm@redhat.com>	2014-03-05 09:23:40 +0000
commit	1cbec9cd986100de185f6dc10301a54f6604e6af (patch)
tree	b057df936591641fdd29065fab44e0484061b641
parent	6536826f2f4c747582d60ed40b0418c6a67a9829 (diff)
Resolves: #i119480# Fixed import of curves from PPT

Bisected with: bibisect-43max
Comment 2 Xisco Faulí 2019-01-23 14:04:30 UTC
it seems the original issue ( https://bz.apache.org/ooo/show_bug.cgi?id=119480 ) isn't completely fixed either. See https://bz.apache.org/ooo/attachment.cgi?id=77675.
Reverting the patch fixes this issue...
Comment 3 Xisco Faulí 2019-01-23 15:19:36 UTC
if we change https://opengrok.libreoffice.org/xref/core/filter/source/msfilter/msdffimp.cxx?r=979aed6b#4586

from

fNumber = basegfx::rad2deg(atan2(double(aStartPt.X() - cent.X()),
double(aStartPt.Y() - cent.Y())) + F_PI); // 0..360.0

to

fNumber = 270.0;

this issue is fixed...
However, I do not understand the code...
Comment 4 Caolán McNamara 2019-02-06 17:04:11 UTC
Created attachment 148963 [details]
symphony example where problem began
Comment 5 Regina Henschel 2019-03-03 20:01:26 UTC
I see nStartAngle 9000 and nEndAngle 9100, which is correct, only rounded. So that part in msdffimp.cxx seems to be OK. But I wonder, why a special treating for mso_sptArc is needed at all and why /svx/source/xoutdev/_xpoly.cxx is used.

The error happens only for a small angle difference. With 2deg difference, the rendering is OK. So it is likely some double <-> int rounding problem.
Comment 6 Regina Henschel 2019-03-05 00:21:30 UTC
The main problem is in the part around filter\source\msfilter\msdffimp.cxx#4606.

There
XPolygon aXPoly( aPolyBoundRect.Center(), aPolyBoundRect.GetWidth() / 2, aPolyBoundRect.GetHeight() / 2, static_cast<sal_uInt16>(nStartAngle) / 10, static_cast<sal_uInt16>(nEndAngle) / 10, true );
is called. It should generate a clockwise Béziercurve from start angle to end angle. But in case of the test document, a wrong curve with a very short arc is generated.

With this short arc, later on in #4608, a rectangle aPolyPieRect with small width is generated. That leads to a far to large fXScale in #4639 and a totally wrong fXOfs in #4627.
As a result the new bounding rectangle and thus size and position of the shape results in the values posX=-927.66cm and width=522.83cm, which you see in the Position&Size dialog in the loaded file.

The solution should be, to do not use these old methods from _xpoly.cxx, but make a solution, where the essential calculation is done with the methods from basegfx.
Comment 7 Regina Henschel 2019-03-05 15:22:16 UTC
Some steps for going from XPolygon to B2DPolygon can be found in https://cgit.freedesktop.org/libreoffice/core/tree/filter/source/msfilter/msdffimp.cxx?h=aoo/aw080 an older, unfortunately not finished work from Armin.
Comment 8 Regina Henschel 2019-03-08 18:25:18 UTC
Created attachment 149833 [details]
Testdocument for wrong scaling

The patch in  https://gerrit.libreoffice.org/68941 fixes the underlying problem, that was not detected in the past. Instead a 'shape repair' was tried.

Even after applying the patch the shapes are not correct. Because of the 'shape repair' try an unsuitable scaling is done.

A custom shape has a frame rectangle (which you see as resize handles), which corresponds to the shape width and height as set in the Position&Size dialog. The shape itself is drawn into this rectangle and its outline need not reach the edges of the frame rectangle or it might exceed the frame rectangle. The actually used area is called the bounding box. The current import scales the shape so, that its bounding box gets that size, which is set as frame rectangle in the file.

The attached file shows this error. The frame rectangle should have width 9cm and height 6cm but is wider. Open the file in Word to see the difference to LibreOffice.

So the problem here is not only an accuracy problem, but the scaling method itself has to be fixed.
Comment 9 Regina Henschel 2019-03-12 17:25:02 UTC
I have made a new bug report bug 124029 for the different handling of the viewbox, so this one is only about the signed<->unsigned integer error, handled in https://gerrit.libreoffice.org/68941. That makes bug handling easier.
Comment 10 Commit Notification 2019-03-14 12:31:55 UTC
Regina Henschel committed a patch related to this issue.
It has been pushed to "master":

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

tdf#122899 use unsigned integer for mso_sptArc

It will be available in 6.3.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 11 Xisco Faulí 2019-04-17 15:26:54 UTC
A polite ping to Regina Henschel:
Is this bug fixed? if so, could you please close it as RESOLVED FIXED ? Otherwise, Could you please explain what's missing?
Thanks
Comment 12 Xisco Faulí 2019-04-17 15:28:06 UTC
The original problem reported here seems to be fixed in

Version: 6.3.0.0.alpha0+
Build ID: 26e85974a0287ab5869e7ff0145a66b853d66a02
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
Comment 13 Regina Henschel 2019-04-17 16:42:29 UTC
It is fixed.
Comment 14 Xisco Faulí 2019-04-23 11:54:14 UTC
Verified in

Version: 6.3.0.0.alpha0+
Build ID: b34786d2774b261be48de92f65a5d0aa3c32b289
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

@Regina Henschel, thanks for fixing this issue!