Bug Hunting Session
Bug 121845 - custom shape with command U (angleellipse) is wrongly drawn
Summary: custom shape with command U (angleellipse) is wrongly drawn
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Draw (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Regina Henschel
URL:
Whiteboard: target:6.3.0
Keywords:
Depends on: 123157
Blocks: Shapes Shapes-Custom
  Show dependency treegraph
 
Reported: 2018-12-01 18:07 UTC by Regina Henschel
Modified: 2019-04-17 10:09 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Wrong rendering of angleellipse (73.91 KB, application/vnd.oasis.opendocument.presentation)
2018-12-01 18:07 UTC, Regina Henschel
Details
Examples with lot of edge cases (401.17 KB, application/x-zip-compressed)
2019-01-12 23:22 UTC, Regina Henschel
Details
ODF angle orientation (18.66 KB, image/png)
2019-02-19 18:41 UTC, Regina Henschel
Details
Files from unit test (29.08 KB, application/x-zip-compressed)
2019-03-01 17:44 UTC, Regina Henschel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Regina Henschel 2018-12-01 18:07:32 UTC
Created attachment 147205 [details]
Wrong rendering of angleellipse

The command U in the enhanced-path is wrongly drawn. You see the problem, if you compare the endpoints of the curve with the handle.

The essential part in page 2 of the attached file is
<draw:enhanced-geometry draw:mirror-horizontal="false" draw:mirror-vertical="false" svg:viewBox="0 0 21600 21600" draw:text-areas="3000 3000 18000 18000" draw:type="non-primitive" draw:modifiers="200" draw:enhanced-path="U 10800 10800 10800 10800 $0 270 F N">
      <draw:handle draw:handle-position="10800 $0" draw:handle-polar="10800 10800"/>
     </draw:enhanced-geometry>

The modifier is directly referenced in the polar-handle and in the enhanced-path.

Expected behavior: The curve starts at the handle and goes clockwise to the direction 270°, which is down vertical axis. That would generate a large arc.

Current behavior: The curve is drawn counter clockwise and it does not start at the handle.

I had reported the problem already for Apache OpenOffice in https://bz.apache.org/ooo/show_bug.cgi?id=121491

This error prevents a proper import and converting from ooxml-arc.

[The spec has radius and angle in draw:handle-position the other way round. But because PowerPoint has it the same way as LibreOffice, that will likely be changed in the spec and is not part of this issue. http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part1.html#attribute-draw_enhanced-path]
Comment 1 Regina Henschel 2018-12-30 17:24:00 UTC
I'll work on it.
Comment 2 Xisco Faulí 2019-01-09 15:54:19 UTC
(In reply to Regina Henschel from comment #0)
> I had reported the problem already for Apache OpenOffice in
> https://bz.apache.org/ooo/show_bug.cgi?id=121491

Then, changing it to Inherit from OOo
Comment 3 Regina Henschel 2019-01-12 23:22:59 UTC
Created attachment 148277 [details]
Examples with lot of edge cases

The enhanced-path has the form
U 10800 10800 10800 10800 a0 a1 N
and in the files with name .._T the form
U 10800 10800 10800 10800 a0 a1 T 32400 10800 10800 10800 a0 a1 N
That means a line goes from the first ellipse segment to the second. The second segment has the same form as the first, only right shifted.
The values a0 and a1 vary from file to file, the values are contained in the file name, "von a0 zu a1".
Angles are in degree in file format.
Comment 4 Regina Henschel 2019-01-16 12:14:14 UTC
I halt development, because it is not clear yet, whether the calculation of position from angle is false for path or false for handle. I'll try to get answers on dev mailing list and ODF TC.
Comment 5 Regina Henschel 2019-02-19 18:41:28 UTC
Created attachment 149428 [details]
ODF angle orientation

In the meantime the new command (G) ARCANGLETO was brought to ODF TC
https://issues.oasis-open.org/browse/OFFICE-4026
It has a definition of coordinate system and angle position that it equal to the one used in OOXML. I will attach an illustration here.

Because commands can be mixed inside a path and use the same handles and formulas, the commands U and T should use the same orientation. I will change the implementation for that. This is not visible in preset shapes, because in all cases, where a command U is used in a preset shape, the angles are 0 and 360, so that a full ellipse is drawn.

The ODF 1.2 spec is not detailed. I would like to change it like this:
ODF rules:
Third and forth parameter are horizontal and vertical radius.
An angle determines the start or end point of the segment by intersection of the second angle leg with the
ellipse. The first angle leg is always the positive x-axis.
For the position of the points, the angle is used modulo 360deg in range [0deg..360deg[. The position of range [0deg..360deg[ is the same as in command ARCANGLETO, with 0deg right, 90deg bottom, 180deg left and 270deg top.
Only if abs(end angle - start angle) == 360 deg, a full ellipse is drawn. Consumers may round the difference to compensate rounding errors in formulas.
The segment is always drawn clock wise (in user view) from start point to end point. If authors need counter-clock wise segments or several turns, they can use the command ARCANGLETO.
The end point of the segment becomes the new "current" point.

I'm going to change the implementation that way.
Comment 6 Regina Henschel 2019-02-19 19:28:29 UTC
A proposal is in https://gerrit.libreoffice.org/68029 for discussion.

Remarks on the patch:

The proposal uses the ODF rules from my previous comment.

Only for shapes, that where user-defined in VML, the ODF angle orientation is wrong. LibreOffice has up to version 6.2 no direct support for the VML-commands 'ae' and 'al', which correspond to the ODF commands U and T. But such shapes might appear, if a user has resaved a file from 'MS Office 2003 XML' format into an old MS Office binary format.

To generate such files, you can do this:  Start with a file in "Word 2003 XML" format e.g. generated by MS Word. Edit the path in the source using VML commands "ae" or "al". Then reopen file in MS Word and save it to "doc" format. That works for MS Office 365. The same way is not possible for Excel or PowerPoint, but it might be possible in a MS Office 2003 or 2007. I cannot test it, perhaps someone can generate such test documents?

Ideally such files would be already converted in the import filter. But because that is unlikely to happen, I have added a special rendering for files in MS binary format. Such special handling has exists already, but was faulty in cases, where the swing angle was greater than 360deg and so the stroke makes several turns. Besides that the solution had worked till OOo3.2.1, but then broke because of the Symphony patch (see bug 123157).

My solution removes the Symphony patch. The Symphony patch tries to identify shapes, which were written by old Symphony versions by examine svg:viewBox and comparing actual shape properties with defaults. This has too much false positive. The problem in such cases are: (1) The svg:viewBox was changed, which alters the shape geometry on rendering. (2) The size of shapes was halved for shapes, which were never touched by Symphony. (3) The used checks prevent the special MS binary shape rendering although it was needed. The problems become visible not in preset shapes, but in user-defined shapes. 

The polar handle from MS binary format uses angles in fixed-float format deg*2^16. In case its modifier values are used directly in the path, shape changing will not work, because angles are divided by 2^16 and LO handles generate values from range ]-180deg..180deg] and so values near zero are used in rendering. But that is a problem of the handle and not of the command U or T. I have not solved that problem.

The commands U and T only differ in the way, that the command U makes an implicit 'moveto' to the start point of the segment. The implementation till LO 6.2 has the error, that this 'moveto' was done only once for the first command U in a series of commands. I have therefor moved the check into the loop.

I have changed the rendering to use the basegfx:: methods instead of the tools:: methods. Thereby I have corrected the drawing of the full ellipse. Im my patch id=c4c1636b5132261e64492de38f252b19b77e69b8 I had set it to always start at 12 o'clock, because that was the case before. But because the start angle is 0deg, it has to start at 3 o'clock. The difference becomes visible, if the stroke gets line end arrows (test with shape type 'ring') or in case a command follows the command U, which uses the end point of the ellipse as "current point" to start.
I'm not sure about my changes to createPolygonFromEllipse. I could remove it, but it may be useful for svdocirc.cxx#220. Removing or adapting svdocirc can be made in a follow-up patch.

The preset shape type 'ring' shows line end arrows on its ellipses, the other preset shapes do not show line end arrows on ellipses. That is caused by
// #i76201# Add conversion to closed polygon when first and last points are equal
basegfx::utils::checkClosed(aNewB2DPolygon);
I have not touched that, because that is a separate problem. My opinion is, that this auto-close should be removed in general.

The proposal has no unit tests yet.
Comment 7 Regina Henschel 2019-03-01 17:44:17 UTC
Created attachment 149678 [details]
Files from unit test
Comment 8 Commit Notification 2019-03-04 08:52:23 UTC
Regina Henschel committed a patch related to this issue.
It has been pushed to "master":

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

tdf#121845 rework custom shape path command U and T

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 9 Regina Henschel 2019-03-15 09:12:59 UTC
Fix is contained in Version: 6.3.0.0.alpha0+ (x64)
Build ID: 13a260f59e421f3e67845f8f2eb22b8f0f8fcaf0
CPU threads: 8; OS: Windows 10.0; UI render: GL; VCL: win; 
TinderBox: Win-x86_64@42, Branch:master, Time: 2019-03-11_02:46:09
Locale: en-US (en_US); UI-Language: en-US
Calc: threaded