Issue 123379 - [SVG] wrong kind of markers on mid-position in paths
Summary: [SVG] wrong kind of markers on mid-position in paths
Status: CLOSED FIXED
Alias: None
Product: Draw
Classification: Application
Component: open-import (show other issues)
Version: 4.1.0-dev
Hardware: All All
: P3 Normal (vote)
Target Milestone: 4.1.0
Assignee: Armin Le Grand
QA Contact:
URL:
Keywords:
: 123384 123401 (view as issue list)
Depends on:
Blocks:
 
Reported: 2013-09-28 21:58 UTC by Regina Henschel
Modified: 2017-05-20 10:33 UTC (History)
3 users (show)

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


Attachments
examples of wrong marker position and direction. The .odg file has all graphics and pictures linked. (52.33 KB, application/zip)
2013-09-28 21:58 UTC, Regina Henschel
no flags Details
Patch offered by Christina Roßmanith to fix this issue (1.82 KB, patch)
2013-10-08 09:05 UTC, Armin Le Grand
no flags Details | Diff
Patch to solve various marker problems (15.17 KB, patch)
2013-10-09 13:31 UTC, Armin Le Grand
no flags Details | Diff
Patch to solve various marker problems (20.67 KB, patch)
2013-10-10 10:09 UTC, Armin Le Grand
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Regina Henschel 2013-09-28 21:58:56 UTC
Created attachment 81668 [details]
examples of wrong marker position and direction. The .odg file has all graphics and pictures linked.

The attached zip-file contains four svg graphics with closed and open parts and with sub-paths. The pictures are screenshots from IE 10, which renders it correctly.

Main problem: The marker-start and marker-end may only occur at the start and end of the whole path, not at mid-positions, even not if they are start or end of a sub-path. For discussion of this problem read thread http://lists.w3.org/Archives/Public/www-svg/2013Sep/0041.html
 
Additional problem: The direction of the markers should be multiples of 45°, because the line directions are vertical and horizontal, but they are more or less not diagonal, where they should be diagonal.

marker-start is green, marker-mid is red, marker-end is blue.
Comment 1 Regina Henschel 2013-09-29 13:36:23 UTC
I have moved the wrong direction of the markers to bug 123384, because it is a different problem. This issue is about using marker-start and marker-end on mid positions.
Comment 2 Regina Henschel 2013-10-05 13:11:34 UTC
Christina creates a fix for LO. I hope she gives it to AOO too.
Comment 3 Armin Le Grand 2013-10-08 09:05:16 UTC
Created attachment 81724 [details]
Patch offered by Christina Roßmanith to fix this issue

ALG: Adding patch from Christina Roßmanith which fixes this issue. Thanks to Christina! Also thanks to Regina for the task and actions.
Comment 4 Armin Le Grand 2013-10-08 09:07:12 UTC
ALG: Grepping task...
Comment 5 SVN Robot 2013-10-09 10:25:01 UTC
"regina" committed SVN revision 1530554 into trunk:
#i123379 SVG wrong kind of markers on mid position
Comment 6 Armin Le Grand 2013-10-09 13:10:02 UTC
ALG: I took a deeper look and started to rework parts of the marker creation code to be on the safe side (patch will follow soon).

@Regina: The 1st example (Marker_on_closed_with_subpath.svg) looks now exactly as in Firefox, also the same in Inkscape. Both have the start marker (the green one) oriented to the right.
But: Your example from IE shows it oriented to the top-right.

IE seems to use prev and next tangents to get the direction
All others seem to only use next in case of the start vector (what makes sense to me). I would guess IE is wrong here. What do you think?
Comment 7 Armin Le Grand 2013-10-09 13:10:50 UTC
ALG: @Regina: too early ;-) I am on it, there are other caveats, also adding the corrected orientations...
Comment 8 Armin Le Grand 2013-10-09 13:14:39 UTC
ALG: Checked safari with 1st example, green arrow is to the right (as Inkscape and Mozilla), so seems to be correct. Nonetheless, all mid markers on the square are wrongly oriented in Safari ;-(
Comment 9 Armin Le Grand 2013-10-09 13:28:13 UTC
ALG: One more question: When loading 1st example in IE it shows me the SVG source. How do I get IE to show me the SVG as graphic?
Comment 10 Armin Le Grand 2013-10-09 13:31:03 UTC
Created attachment 81733 [details]
Patch to solve various marker problems

ALG: This patch corrects start/mid/end marker usage and marker orientations
Comment 11 Armin Le Grand 2013-10-09 15:35:08 UTC
*** Issue 123384 has been marked as a duplicate of this issue. ***
Comment 12 Regina Henschel 2013-10-09 16:56:07 UTC
(In reply to Armin Le Grand from comment #9)
> ALG: One more question: When loading 1st example in IE it shows me the SVG
> source. How do I get IE to show me the SVG as graphic?

I have got IE 10.0.9. on Windows 7. I need not do anything, it shows SVG out of the box. I start IE, than Datei > Öffnen > Durchsuchen > OK (sorry, my IE is German).

IE 10 seems to be nearer to the spec than Seamonkey 2.21 or Chrome, but also renders sometimes different than I expect. 

If there is uncertainty about the spec, we (or I) can ask on their mailing list.
Comment 13 Regina Henschel 2013-10-09 19:42:04 UTC
(In reply to Armin Le Grand from comment #10)
> Created attachment 81733 [details]
> Patch to solve various marker problems
> 
> ALG: This patch corrects start/mid/end marker usage and marker orientations

The logic in line 932
   if(!bIsFirstMarker && (nPointIndex || bSubPolygonPathIsClosed))
looks wrong to me. For the first point of a closed path you get
   if(!true && (false || true))
which result in false and skips the 'then' case, and so has (0.0,0.0) for the incoming tangent vector. But it has to be the tangent of the last path segment.

You see the error in the test files. For closed paths the green and blue marker should be diagonal and not in edge direction.

I would combine it with bug 123401. Idea:
Determine how many steps back are allowed at the most, which depends on index and on whether the path is closed or not.
Step back till a non-zero-length path segment is found or max step count exceeded.
If a non-zero-length path segment exists, its tangent vector is aPrev.
Determine how many steps forward are allowed at the most.
Step forward till a non-zero-length path segment is found or max step count exceeded.
If a non-zero-length path segment exists, its tangent vector is aCurrent.

Needs atan2 a domain check? In http://www.cplusplus.com/reference/cmath/atan2/ I read "If both arguments passed are zero, a domain error occurs." But I could not force a problem using zero-length segments.
Comment 14 Regina Henschel 2013-10-09 19:55:55 UTC
From http://www.w3.org/TR/SVG/painting.html#OrientAttribute,
"In all cases for closed subpaths (e.g., subpaths which end with a 'closepath' command), the orientation of the marker corresponding to the initial point of the subpath is calculated assuming that:
    the path segment going into the vertex is the path segment corresponding to the closepath
    the path segment coming out of the vertex is the first path segment in the subpath"

The special rule for "When a 'closepath' command is followed by a command other than a 'moveto' command" is not yet relevant, because such path is currently not possible in Draw.
Comment 15 Armin Le Grand 2013-10-10 09:58:58 UTC
ALG: Hi Regina, your comment 14 clearly defines that the start marker indeed has to be diagonal, taking incoming and outgoing tangents into account. The code in 932 is wrong in the sense that I tried to suppress incoming for this cases (same for outgoing and end marker).
This also means that Inscape, Mozilla and Safari are wrong. From some point on compatibility gets the stronger argument, hmmm.
Intuitively, there are also arguments for both:

Pro: it's a start marker, thus mostly used for start arrows - even when closed, it should only rely on outgoing tangent
Contra: As for every marker on a point, if the point has incoming and outgoing, use both.

I have now prepared a version always using incoming and outgoing, combined with the changes made for task 123401. Loos as if that one will also be solved by this one. It is still good to have separate tasks, you never know befor looking into it, but the main point being separated descriptions. One problem, one task ;-)

BTW: Nice bugdocs, only one attachment, one file 'to load them all' and the separate data files...

Preparing patch...
Comment 16 Armin Le Grand 2013-10-10 10:09:07 UTC
Created attachment 81740 [details]
Patch to solve various marker problems

ALG: This time with dynamic lookup of entering/leaving tangent for a point and use of both when existent, even for start/end markers
Comment 17 Armin Le Grand 2013-10-11 16:57:31 UTC
ALG: Checked again, and looks good. Committing this version. If we need further changes, no problem.
Comment 18 SVN Robot 2013-10-11 16:57:47 UTC
"alg" committed SVN revision 1531353 into trunk:
i123379 various SVG marker corrections
Comment 19 Armin Le Grand 2013-10-11 16:57:59 UTC
*** Issue 123401 has been marked as a duplicate of this issue. ***
Comment 20 Armin Le Grand 2013-10-11 16:58:23 UTC
ALG: Okay, committed, done for now.