Bug 59764 - Canvas: Use fast, approximate dirty rects for stroke()
Summary: Canvas: Use fast, approximate dirty rects for stroke()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords: Performance
Depends on:
Blocks:
 
Reported: 2011-04-28 17:35 PDT by Andreas Kling
Modified: 2011-07-11 09:48 PDT (History)
6 users (show)

See Also:


Attachments
Proposed patch (2.08 KB, patch)
2011-04-28 17:40 PDT, Andreas Kling
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2011-04-28 17:35:30 PDT
Qt already has an optimization in place for CanvasRenderingContext2D::stroke() where we take the QPainterPath::controlPointRect() and grow it by the miterLimit and the lineWidth to create an approximate dirty rect which is significantly faster than rasterizing the path and calculating the exact bounding rect.

We should share this optimization with other ports, using the inflated Path::boundingRect(), which is significantly faster on CG, and likely on other rendering libraries as well.
Comment 1 Andreas Kling 2011-04-28 17:40:00 PDT
Created attachment 91606 [details]
Proposed patch
Comment 2 Eric Seidel (no email) 2011-05-01 09:42:17 PDT
What perf test shows this change?
Comment 3 Oliver Hunt 2011-05-01 11:52:34 PDT
Comment on attachment 91606 [details]
Proposed patch

Does this handle acute angles with mitres correctly?  I recall the the reason we didn't fix this in the past was due to potentially unbounded mitre size
Comment 4 Andreas Kling 2011-05-02 01:48:30 PDT
(In reply to comment #3)
> What perf test shows this change?

Spotted it on http://grantkot.com/sine.html which divides its rendering time between CGContextReplacePathWithStrokedPath (for bounding box calculation) and CGContextStrokePath (for actual rendering.)

(In reply to comment #3)
> Does this handle acute angles with mitres correctly?  I recall the the reason we didn't fix this in the past was due to potentially unbounded mitre size

How do you end up with an unbounded miter? The CanvasRenderingContext2D.miterLimit exists to prevent exactly that, no?
Comment 5 Matthew Delaney 2011-05-02 17:16:35 PDT
I believe Oliver is talking about the case where the user sets the miter limit to be huge and then has a very acute angle between two path segments and then strokes that path.

Otherwise, here are a few others things I noticed. If we do end up not needing m_path.strokeBoundingRect, then we can likely get rid of it and also do the same alternative bounding rect calculation in the only other place this method (strokeBoundingRect) is used: RenderSVGPath.cpp.

You mentioned it's significantly faster on CG - did you take down any performance measurements for any tests that would benefit from this? You mentioned the grantkot.com/sine.html page, but I don't see that that's a performance test. Did you instrument a local copy to get some FPS count for it or just notice that it was faster "to the eye"?
Comment 6 Andreas Kling 2011-05-05 06:42:25 PDT
(In reply to comment #5)
> I believe Oliver is talking about the case where the user sets the miter limit to be huge and then has a very acute angle between two path segments and then strokes that path.

Unless I'm misunderstanding something, that should be covered by the miterLimit property; if the stroke would extend outside the approximated rect, it would be drawn with a bevel join instead.

> Otherwise, here are a few others things I noticed. If we do end up not needing m_path.strokeBoundingRect, then we can likely get rid of it and also do the same alternative bounding rect calculation in the only other place this method (strokeBoundingRect) is used: RenderSVGPath.cpp.

RenderSVGPath needs the bounding rect to be correct, since it's used for more than invalidating the dirty region.

> You mentioned it's significantly faster on CG - did you take down any performance measurements for any tests that would benefit from this? You mentioned the grantkot.com/sine.html page, but I don't see that that's a performance test. Did you instrument a local copy to get some FPS count for it or just notice that it was faster "to the eye"?

I was playing around with a Mac for once, and wanted to try the "Instruments" application, so I ran this page through the time profiler tool. Like I said above, CPU time was (pretty much) equally divided by measuring the stroke and painting it. I could whip up a reduction with some human-friendly output if you like. :)
Comment 7 Brent Fulgham 2011-07-10 19:48:58 PDT
This has been sitting here for a couple of months, and seems like a good idea.  Can we move this along somehow?

Ollie -- are your concerns addressed by Andreas' responses?
Comment 8 Oliver Hunt 2011-07-11 09:00:51 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > I believe Oliver is talking about the case where the user sets the miter limit to be huge and then has a very acute angle between two path segments and then strokes that path.
> 
> Unless I'm misunderstanding something, that should be covered by the miterLimit property; if the stroke would extend outside the approximated rect, it would be drawn with a bevel join instead.
> 

What's the default miterLimit? to my knowledge by default there is no limit...
Comment 9 Nikolas Zimmermann 2011-07-11 09:12:12 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > I believe Oliver is talking about the case where the user sets the miter limit to be huge and then has a very acute angle between two path segments and then strokes that path.
> > 
> > Unless I'm misunderstanding something, that should be covered by the miterLimit property; if the stroke would extend outside the approximated rect, it would be drawn with a bevel join instead.
> > 
> 
> What's the default miterLimit? to my knowledge by default there is no limit...

In SVG the default miterLimit is 4.
Comment 10 Andreas Kling 2011-07-11 09:35:52 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > I believe Oliver is talking about the case where the user sets the miter limit to be huge and then has a very acute angle between two path segments and then strokes that path.
> > 
> > Unless I'm misunderstanding something, that should be covered by the miterLimit property; if the stroke would extend outside the approximated rect, it would be drawn with a bevel join instead.
> > 
> 
> What's the default miterLimit? to my knowledge by default there is no limit...

The 2d canvas context has a default miterLimit of 10. ( http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-context-2d-miterlimit )
Comment 11 Oliver Hunt 2011-07-11 09:39:00 PDT
Comment on attachment 91606 [details]
Proposed patch

r=me
Comment 12 Andreas Kling 2011-07-11 09:48:03 PDT
Committed r90758: <http://trac.webkit.org/changeset/90758>