Closed Bug 1002466 Opened 10 years ago Closed 9 years ago

With Skia content, -moz-column-rule:dotted is not rendered at all

Categories

(Core :: Graphics, defect)

Other Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: bjacob, Assigned: bjacob)

References

Details

(Whiteboard: [skia-upstream])

Attachments

(2 files)

This causes the mochitest failures of the form:

 TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_visited_reftests.html | (20) != column-rule-1.html column-rule-1-notref.html

On https://tbpl.mozilla.org/?tree=Graphics
'solid' is correctly rendered, but 'dotted' is not rendered at all. Happens on Linux and Windows, not Mac.
Summary: With Skia content, moz-column:dotted is not rendered at all → With Skia content, -moz-column-rule:dotted is not rendered at all
Assignee: nobody → bjacob
Attached file minimal testcase
Some progress. (Thanks to :gw280 who spent time today to teach me Skia and Moz2D things).

The issue is that this stipple pattern has 'on' segments of length 0, which Moz2D thinks is fine (the rounded caps around it are meant to provide the round dots) but which Skia thinks means there is nothing to render. Thus, Skia is taking an early return, specifically in SkScan::AntiFillPath, here:

616	    if (ir.isEmpty()) {
617	        if (path.isInverseFillType()) {
618	            blitter->blitRegion(origClip);
619	        }
620	        return;
621	    }
(gdb) n
620	        return;
(gdb) bt 20
#0  SkScan::AntiFillPath (path=..., origClip=..., blitter=0x7fffffff8ec8, forceRLE=false)
    at /hack/mozilla-graphics/gfx/skia/trunk/src/core/SkScan_AntiPath.cpp:620
#1  0x00007ffff20de167 in SkScan::AntiFillPath (path=..., clip=..., blitter=0x7fffffff8ec8)
    at /hack/mozilla-graphics/gfx/skia/trunk/src/core/SkScan_AntiPath.cpp:732
#2  0x00007ffff20521df in SkDraw::drawPath (this=0x7fffffff9340, origSrcPath=..., 
    origPaint=..., prePathMatrix=0x50ff8001, pathIsMutable=false, drawCoverage=false)
    at /hack/mozilla-graphics/gfx/skia/trunk/src/core/SkDraw.cpp:1152
#3  0x00007ffff1ff4904 in SkDraw::drawPath (this=0x7fffffff9340, path=..., paint=..., 
    prePathMatrix=0x0, pathIsMutable=false)
    at /hack/mozilla-graphics/gfx/skia/trunk/include/core/SkDraw.h:50
#4  0x00007ffff1ff3bc1 in SkBitmapDevice::drawPath (this=0x7fffc6a6f200, draw=..., path=..., 
    paint=..., prePathMatrix=0x0, pathIsMutable=false)
    at /hack/mozilla-graphics/gfx/skia/trunk/src/core/SkBitmapDevice.cpp:420
#5  0x00007ffff2035dc1 in SkCanvas::drawPath (this=0x7fffc828c210, path=..., paint=...)
    at /hack/mozilla-graphics/gfx/skia/trunk/src/core/SkCanvas.cpp:2020
#6  0x00007fffeec2b776 in mozilla::gfx::DrawTargetSkia::Stroke (this=0x7fffc612cc80, 
    aPath=0x7fffc7797a90, aPattern=..., aStrokeOptions=..., aOptions=...)
    at /hack/mozilla-graphics/gfx/2d/DrawTargetSkia.cpp:415
#7  0x00007fffeed8725c in gfxContext::Stroke (this=0x7fffc76869c0)
    at /hack/mozilla-graphics/gfx/thebes/gfxContext.cpp:349
#8  0x00007ffff0b51f54 in nsCSSBorderRenderer::DrawDashedSide (this=0x7fffffff9ab8, 
    aSide=mozilla::css::eSideLeft)
    at /hack/mozilla-graphics/layout/base/nsCSSRenderingBorders.cpp:992
#9  0x00007ffff0b3e5a9 in nsCSSBorderRenderer::DrawBorders (this=0x7fffffff9ab8)
    at /hack/mozilla-graphics/layout/base/nsCSSRenderingBorders.cpp:1915
#10 0x00007ffff0b3beee in nsCSSRendering::PaintBorderWithStyleBorder (
    aPresContext=0x7fffc760c000, aRenderingContext=..., aForFrame=0x7fffcc9a02f0, 
    aDirtyRect=..., aBorderArea=..., aStyleBorder=..., aStyleContext=0x7fffcc99fbf0, 
    aSkipSides=7) at /hack/mozilla-graphics/layout/base/nsCSSRendering.cpp:573
#11 0x00007ffff0bf5175 in nsColumnSetFrame::PaintColumnRule (this=0x7fffcc9a02f0, 
    aCtx=0x7fffc76fa780, aDirtyRect=..., aPt=...)
    at /hack/mozilla-graphics/layout/generic/nsColumnSetFrame.cpp:114
#12 0x00007ffff0bf7f15 in PaintColumnRule (aFrame=0x7fffcc9a02f0, aCtx=0x7fffc76fa780, 
    aDirtyRect=..., aPt=...) at /hack/mozilla-graphics/layout/generic/nsColumnSetFrame.cpp:49
#13 0x00007ffff0bc3ee8 in nsDisplayGeneric::Paint (this=0x7fffc6a8c420, 
    aBuilder=0x7fffffffb238, aCtx=0x7fffc76fa780)
    at /hack/mozilla-graphics/layout/generic/../base/nsDisplayList.h:1846
#14 0x00007ffff0aee25d in mozilla::FrameLayerBuilder::PaintItems (this=0x7fffc66bbcc0, 
    aItems=..., aRect=..., aContext=0x7fffc76869c0, aRC=0x7fffc76fa780, 
    aBuilder=0x7fffffffb238, aPresContext=0x7fffc760c000, aOffset=..., aXScale=1, aYScale=1, 
    aCommonClipCount=0) at /hack/mozilla-graphics/layout/base/FrameLayerBuilder.cpp:3635
#15 0x00007ffff0aef27e in mozilla::FrameLayerBuilder::DrawThebesLayer (
    aLayer=0x7fffc6a8d000, aContext=0x7fffc76869c0, aRegionToDraw=..., 
    aClip=mozilla::layers::DRAW_SNAPPED, aRegionToInvalidate=..., 
    aCallbackData=0x7fffffffb238)
    at /hack/mozilla-graphics/layout/base/FrameLayerBuilder.cpp:3797
#16 0x00007fffeedf4f0d in mozilla::layers::BasicThebesLayer::PaintBuffer (
    this=0x7fffc6a8d000, aContext=0x7fffc76869c0, aRegionToDraw=..., 
    aExtendedRegionToDraw=..., aRegionToInvalidate=..., aDidSelfCopy=false, 
    aClip=mozilla::layers::DRAW_SNAPPED, 
---Type <return> to continue, or q <return> to quit---
    aCallback=0x7ffff0aeebc0 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*)>, aCallbackData=0x7fffffffb238)
    at /hack/mozilla-graphics/gfx/layers/basic/BasicThebesLayer.h:112
#17 0x00007fffeede97a7 in mozilla::layers::BasicThebesLayer::Validate (this=0x7fffc6a8d000, 
    aCallback=0x7ffff0aeebc0 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*)>, aCallbackData=0x7fffffffb238)
    at /hack/mozilla-graphics/gfx/layers/basic/BasicThebesLayer.cpp:200
#18 0x00007fffeede9957 in non-virtual thunk to mozilla::layers::BasicThebesLayer::Validate(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*) (this=0x7fffc6a8d1f8, 
    aCallback=0x7ffff0aeebc0 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*)>, aCallbackData=0x7fffffffb238) at Unified_cpp_gfx_layers1.cpp:218
#19 0x00007fffeede425e in mozilla::layers::BasicContainerLayer::Validate (
    this=0x7fffc6a8cc00, 
    aCallback=0x7ffff0aeebc0 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*)>, aCallbackData=0x7fffffffb238)
    at /hack/mozilla-graphics/gfx/layers/basic/BasicContainerLayer.cpp:122
(More stack frames follow...)
Related: https://code.google.com/p/skia/issues/detail?id=1040

tldr: Skia considers zero length lines as degenerate enough to skip drawing them, even if the linecap style is set to round or square. In the latter two cases, on most 2d rasterisers (cairo, quartz, d2d, etc), the line segment has length zero but the linecap is still rendered, resulting in either a circular dot or a square to be rendered. This is also consistent with the SVG spec.

The bug linked above suggests that the Skia team doesn't consider this a valid bug due to internal API consistencies, so we may have to hack around this. My proposed solution is to check if any of the dash lengths are 0.0 and replace them with SK_ScalarNearlyZero.
(Sorry for hijacking your bug Benoit!)
Well this does sound evil, but it might be OK in an evil world!

In my local testing 0.01 did make things look about right, so I do believe that this will make this bug disappear; but this also seems slightly unsatisfactory so I would like to keep debugging a bit more before giving up on "a better fix".

I'm not the right person to review Skia patches, I believe; but you could maybe push this to the Graphics tree without review and see what TBPL says? Or can you push Graphics to try?
And, you're not hijacking my bug, you're helping me. Thanks!
(In reply to Benoit Jacob [:bjacob] from comment #7)
> I'm not the right person to review Skia patches, I believe; but you could
> maybe push this to the Graphics tree without review and see what TBPL says?

Way ahead of you :)
Attachment #8414838 - Flags: review?(bjacob)
Attachment #8414838 - Flags: review?(matt.woodrow)
Comment on attachment 8414838 [details] [diff] [review]
0001-Bug-1002466-Use-SK_ScalarNearlyZero-instead-of-0.0f-.patch

Review of attachment 8414838 [details] [diff] [review]:
-----------------------------------------------------------------

This is honestly just awful. I'd prefer if we wait to see if skia will fix this properly upstream, but you can land this if you need to in the interim.
Attachment #8414838 - Flags: review?(matt.woodrow) → review+
Whiteboard: [skia-upstream]
Chrome doesn't currently support dotted borders so they don't run into this: http://www.impressivewebs.com/comparison-css-border-style/

It would be nice to fix this upstream, but it might not be super easy. Let's take this patch for now, just to avoid blocking on it.
https://hg.mozilla.org/mozilla-central/rev/74399f9da0e5
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Blocks: 1214309
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: