Issue 119118 - z command in d attribute in svg sets the 'current point' wrong
z command in d attribute in svg sets the 'current point' wrong
Status: ACCEPTED
Product: Draw
Classification: Application
Component: code
recent-trunk
PC All
: P3 normal (vote)
: ---
Assigned To: Armin Le Grand
:
: 119115 (view as issue list)
Depends on:
Blocks: 123410
  Show dependency treegraph
 
Reported: 2012-03-23 17:54 UTC by Regina Henschel
Modified: 2013-11-05 16:33 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation on: ---
Developer Difficulty: ---


Attachments
closed subpath with following line (4.21 KB, image/png)
2013-09-28 21:31 UTC, Regina Henschel
no flags Details
The svg graphic itself. (1.08 KB, image/svg+xml)
2013-09-28 22:07 UTC, Regina Henschel
no flags Details
Set current point correctly and allow non moveto after z (6.11 KB, patch)
2013-10-05 17:26 UTC, Regina Henschel
rb.henschel: review?
Details | Diff
draw element with close command followed by line command (20.70 KB, application/zip)
2013-11-03 16:43 UTC, Regina Henschel
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description Regina Henschel 2012-03-23 17:54:24 UTC
OOo has handled the z command in the d attribute wrong. It should set the 'current point' to the start of the subpath, but the current point remains at the position, where it is, when closing is performed. Therefore the next m movement uses a wrong reference for its relative movement.
http://www.w3.org/TR/SVG/paths.html#PathDataClosePathCommand

LibreOffice has addressed this with the commit http://cgit.freedesktop.org/libreoffice/core/commit/?id=13e1fe0561facccc80628c031065ad91adfdeade [and an additional fix, which will follow.]

The consequence is, that documents, which will be generated by LibreOffice 3.5.2 are shown different in AOO and vs.
Comment 1 Regina Henschel 2012-03-25 19:57:32 UTC
Hi Armin, this is the corresponding issue to 119115.
Comment 2 Armin Le Grand 2012-03-26 16:24:55 UTC
ALG: Thaks Regina, taking over.
Comment 3 Armin Le Grand 2012-03-26 16:27:45 UTC
ALG: #119115# is double to his one.
Comment 4 Regina Henschel 2012-03-27 10:16:09 UTC
*** Issue 119115 has been marked as a duplicate of this issue. ***
Comment 5 Armin Le Grand 2012-03-29 11:33:53 UTC
ALG: Would be nice to handle together with #112442#
Comment 6 Regina Henschel 2013-09-28 21:31:25 UTC
Created attachment 81667 [details]
closed subpath with following line

The right part should be a rectangle. IE 10 shows it correctly.
Comment 7 Regina Henschel 2013-09-28 22:07:00 UTC
Created attachment 81669 [details]
The svg graphic itself.

This is the original .svg graphic. The previous attached .png file shows how it is rendered in IE 10, which renders it correctly.
Comment 8 Regina Henschel 2013-10-05 17:26:27 UTC
Created attachment 81710 [details]
Set current point correctly and allow non moveto after z

With the current implementation of the property "closed" it is not possible to match svg:d totally. Reasoning: svg:d allows, that after a z command, other commands than m,M may follow. Especially it is possible to use lines. In AOO the end of this lines are connected to the start point because the property "close" cannot be set to a part of the path. Despite the mentioned fix in LO, LO 4.2 shows such svg-graphics still wrong.

My idea is, to finish the path after a z command in any case. If a open path end in svg:d follows after z, it remains open in AOO. This way marker-ends are drawn correctly, the shape itself looks correct, and no additional line appears.

The shortcoming is, that the separated subpath generates an additional marker-mid at the connecting point, and that this marker and the marker-mid of the close part in this point, have the wrong direction. And I see no way to detect a true moveto from such pseudo moveto, which will affect solutions for bug 123410.

I assume that a wrong marker-mid is not so bad as a wrong marker-end or as additional lines for shape closing. Therefore I suggest to use this patch, till there will be at some time a change in the implementation of draw:path.
Comment 9 Armin Le Grand 2013-10-09 10:21:51 UTC
ALG: Hi Regina, thanks for the patch. It surely does the right thing, but I would like to move this behind aw080 because:
- currently we have two svg:d im/exporters, one in xmloff and one in basegfx
- in aw080, these are already consolidated (to the one in basegfx)
- I want to support plane svg:d there (handle 'z' correctly) and add a compatibility mode (cm) there
- if cm true and import, handle 'z' wrong as usual
- if cm false and import, do correct 'z', use e.g. in svg import
- if cm true in export, add a 'M' after each 'z' to avoid the old error
- if cm false in export, handle correct as pure svg:d

To not do double work and to avoid problems, I would like to do this evtl. additional change after this, please be patient ;-)
Comment 10 Armin Le Grand 2013-10-29 13:50:02 UTC
ALG: Doing changes in that direction in task 123433...
Comment 11 Armin Le Grand 2013-10-29 14:14:22 UTC
ALG: @Regina: Please have a look if this is done with the changes to task 123433.
Comment 12 Regina Henschel 2013-10-30 10:43:55 UTC
The line now starts at the correct position, but I'm not sure about the way it is done. Please look at the example in bug 123410. It seems as if there is a point too much. If I understand it correctly, when another command than moveto follows on the close-path command, than it takes the point, which was generated by the close-path command as start point. So there will be not two but only one mid-marker.

Do you agree with my interpretation? Or should I ask on the www-svg mailinglist?
Comment 13 Armin Le Grand 2013-10-30 14:57:35 UTC
ALG: @Regina: Please ask. I interpret '8.3.3 The "closepath" command' as follows:

If a "closepath" is followed immediately by a "moveto", then the "moveto" identifies the start point of the next subpath. If a "closepath" is followed immediately by any other command, then the next subpath starts at the same initial point as the current subpath.

E.g. 'M 10,20 z v 20'

For me this means that the 'v' creates a line from (10,20) to (10, 40) because of the 2nd sentence of the citate above: (10,20) is 'the same initial point as the current subpath' and gets created indirectly.
Comment 14 Regina Henschel 2013-10-30 21:54:17 UTC
The import of svg-graphics uses the correct reference point now, but the same path data in a draw:path element still uses the wrong reference point. I thought the fix is for svg:d attributes everywhere?
Comment 15 Armin Le Grand 2013-10-31 09:46:54 UTC
ALG: No, please re-read above and task 123433:
There are two usages of svg:d in AOO:

(a) ODF XML context
(b) SVG import

For (a) it is
- for import: not possible to get away from the slightly changed paradigm/error of the new last point in svg:d consuming without potentially breaking all existing ODF files
- for export: possible to get around this by writing 'M' after 'z' instead of 'm' so that it is a valid svg_d and does not contain the error -> all derivates can read it

This means:
- All files written from now on can be imported independent from that error
- All files written earlier still need that 'wrong' (or: ODF-specific) interpretation of the new start point and use that
- Derivates (like LO) will be able to read new files without error
- Reading files from derivates will still lead to errors; of course these derivates have the choice to do the same changes if they are interested that the OOO-version which is most used in the world can load their user's stuff without that error...

For (b) pure svg:d is now used (currently only import anyways; the svg exporter has it's own svg:d writer).
Comment 16 Regina Henschel 2013-11-03 16:43:44 UTC
Created attachment 81871 [details]
draw element with close command followed by line command

The attached .odg document has an identical string in the d attribute in the draw element as the .svg file in the path element.

I think, that it is needed, that AOO can read paths, interpreting them strictly in ODF XML import. As AOO cannot detect, what application has produced the file, the user should get the possibility to tell AOO how to interpret the drawings, with a default suitable for ordinary AOO users.

Find the mails about the command sequence LZL in comparison to LZML in thread http://lists.w3.org/Archives/Public/www-svg/2013Oct/0034.html.

Even when in case of command sequence LZL the start point of the next subpath is created indirectly, it remains, that the directions of their marker-mid drawings are different. So there is a need to distinguish a command sequence LZL from LZML.
Comment 17 Armin Le Grand 2013-11-05 16:33:08 UTC
ALG: Hi Regina, I do not get what you want to have changed here. Maybe because we mix too many different things. Thus:
- What need for change do you see in the ODF case?
- What need for change do you see in the SVG import case?
From my POV both look pretty good, except the not corectable mess with relative startpoints of sub-polygons in the ODF case where I think we have a good compromize. Have you tried exporting ODF in a current version, it will produce a svg:d which will not have that old error, but can be loaded in all OOO/AOO-versions; it should also be usable in svg's directly since it's a valid svg:d without that old relative offset error.
On import the situation is as you propose: The default is to correctly read all existing ODFs, thus it is unavoidable to keep the error in ODF svg:d import. I see no way to get rid of this, the better solution would be if all ODF producers would adapt to writing svg:d without that error in the future, as I have already done.