Closed Bug 1187770 Opened 4 years ago Closed 4 years ago

Zero-length SVG lines with stroke-linecap="square" don't render

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: twointofive, Assigned: twointofive)

Details

Attachments

(3 files, 2 obsolete files)

Attached image testcase.svg
This is a cairo bug (so mostly seen on linux).  A zero-length svg line with stroke-linecap="square" should display a square centered at the start point = end point.

Expected results: Two lime squares.

Actual results: Nothing displayed.
Attached patch cairo patch.diff (obsolete) — Splinter Review
The testcase (without this patch) works under cairo if stroke-linecap="round" instead of "square", and it works for both square and round linecaps for firefox using D2D on Windows and with chrome and opera on linux.

The attached patch seems to fix this one, and it passes the svg reftests and mochitests, but that's as much testing as I've done, sorry.

Feel free to take this one.
I believe the current behaviour of Cairo matches what PDF does. I'm not in a huge rush to change it. I'm also surprised that this works in Chrome as I didn't think they supported degenerate dashes at all. Tom, can you look at how Chrome implements this?
Flags: needinfo?(twointofive)
Attached image tests.svg
I added some more tests to cover: zero-length lines, path with zero-length subpath, and line with zero-length dashes, all for square and round linecaps.

Cairo renders in all cases except for the zero-length lines with square linecaps.

Chrome doesn't render dashing with either linecap, but does render the zero-length line and zero-length subpath.

I didn't spend much time looking, but chrome does appear to have explicit support for zero-length subpaths:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/layout/svg/LayoutSVGPath.cpp&sq=package:chromium&rcl=1438037135&l=149
Apparently that doesn't include dashing though.

Also saw this bug discussing moving zero-length subpath support to skia:
https://code.google.com/p/chromium/issues/detail?id=422974&q=zero-length%20subpath&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Cr%20Status%20Owner%20Summary%20OS%20Modified
Flags: needinfo?(twointofive)
As far as cairo is concerned, I believe this is by design, since it's unclear what the result should be. See case (2) in the "Note" at http://cairographics.org/manual/cairo-cairo-t.html#cairo-stroke.
Right, okay, sorry, I was confused by being unaware of the work done in bug 589648: we're artificially lengthening the zero-length subpath in my test above so that cairo will render it.

I guess we need to do something similar for lines.

Moving to SVG component.
Assignee: nobody → twointofive
Component: Graphics → SVG
Attachment #8639121 - Attachment is obsolete: true
Attachment #8639121 - Flags: review?(jmuizelaar)
Attached patch patch.diff (obsolete) — Splinter Review
Attachment #8641926 - Flags: review?(longsonr)
Comment on attachment 8641926 [details] [diff] [review]
patch.diff

content (i.e. dom/svg) should not depend on layout/svg (it's the other way round). So can you move the nsSVGUtils stuff to SVGContentUtils please (but see below as the method you're adding is unnecessary)?

> 
>+void
>+SVGLineElement::MaybeAdjustForZeroLength(float aX1, float aY1,
>+                                         float* aX2, float aY2)

I think x2 would be better passed as a reference than a pointer

>+void
>+nsSVGUtils::GetPathStyleData(nsSVGPathGeometryElement* aContent,
>+                             uint8_t* aStrokeLineCap,
>+                             Float* aStrokeWidth)

Don't do this use the existing SVGContentUtils::GetStrokeOptions instead.

On the right lines, but would like to see another patch here hence the r-
Attachment #8641926 - Flags: review?(longsonr) → review-
Attached patch Patch v2Splinter Review
Attachment #8641926 - Attachment is obsolete: true
Attachment #8642062 - Flags: review?(longsonr)
Attachment #8642062 - Flags: review?(longsonr) → review+
https://hg.mozilla.org/mozilla-central/rev/bea2f46be0f1
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.