59764 2011-04-28 17:35:30 -0700 Canvas: Use fast, approximate dirty rects for stroke() 2011-07-11 09:48:03 -0700 1 1 1 Unclassified WebKit Canvas 528+ (Nightly build) All All RESOLVED FIXED Performance P2 Normal --- 1 kling kling bfulgham eric krit mdelaney7 oliver zimmermann oldest_to_newest 394978 0 kling 2011-04-28 17:35:30 -0700 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. 394985 1 91606 kling 2011-04-28 17:40:00 -0700 Created attachment 91606 Proposed patch 395918 2 eric 2011-05-01 09:42:17 -0700 What perf test shows this change? 395977 3 91606 oliver 2011-05-01 11:52:34 -0700 Comment on attachment 91606 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 396298 4 kling 2011-05-02 01:48:30 -0700 (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? 396818 5 mdelaney7 2011-05-02 17:16:35 -0700 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"? 398693 6 kling 2011-05-05 06:42:25 -0700 (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. :) 434654 7 bfulgham 2011-07-10 19:48:58 -0700 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? 434910 8 oliver 2011-07-11 09:00:51 -0700 (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... 434918 9 zimmermann 2011-07-11 09:12:12 -0700 (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. 434931 10 kling 2011-07-11 09:35:52 -0700 (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 ) 434934 11 91606 oliver 2011-07-11 09:39:00 -0700 Comment on attachment 91606 Proposed patch r=me 434940 12 kling 2011-07-11 09:48:03 -0700 Committed r90758: <http://trac.webkit.org/changeset/90758> 91606 2011-04-28 17:40:00 -0700 2011-07-11 09:39:00 -0700 Proposed patch bug-59764.diff text/plain 2129 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBmNjAyOTg4Li5iZjJkNzBmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkg QEAKKzIwMTEtMDQtMjggIEFuZHJlYXMgS2xpbmcgIDxrbGluZ0B3ZWJraXQub3JnPgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIENhbnZhczogVXNlIGZh c3QsIGFwcHJveGltYXRlIGRpcnR5IHJlY3RzIGZvciBzdHJva2UoKQorICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTk3NjQKKworICAgICAgICBObyBuZXcg dGVzdHMsIHRoaXMgaXMgYW4gb3B0aW1pemF0aW9uLgorCisgICAgICAgICogaHRtbC9jYW52YXMv Q2FudmFzUmVuZGVyaW5nQ29udGV4dDJELmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkNhbnZhc1Jl bmRlcmluZ0NvbnRleHQyRDo6c3Ryb2tlKTogSW5zdGVhZCBvZiB1c2luZworICAgICAgICBQYXRo OjpzdHJva2VCb3VuZGluZ1JlY3QoKSB0byBjYWxjdWxhdGUgdGhlIGV4YWN0IGJvdW5kaW5nIHJl Y3QKKyAgICAgICAgZm9yIGEgcGF0aCBzdHJva2UsIGdldCB0aGUgUGF0aDo6Ym91bmRpbmdSZWN0 KCkgYW5kIGluZmxhdGUgaXQgYnkKKyAgICAgICAgbWl0ZXJMaW1pdCArIGxpbmVXaWR0aCB0byBn ZXQgYSBzbGlnaHRseSBvdmVyc2l6ZWQgZGlydHkgcmVjdAorICAgICAgICBpbiBhIGZyYWN0aW9u IG9mIHRoZSB0aW1lLgorCiAyMDExLTA0LTI4ICBBcm5vIFJlbmV2aWVyICA8YXJub0ByZW5ldmll ci5uZXQ+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgR3VzdGF2byBOb3JvbmhhIFNpbHZhLgpkaWZm IC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC9jYW52YXMvQ2FudmFzUmVuZGVyaW5nQ29udGV4 dDJELmNwcCBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvY2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRl eHQyRC5jcHAKaW5kZXggZmYxMjE2OC4uYjA5MzNkZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNv cmUvaHRtbC9jYW52YXMvQ2FudmFzUmVuZGVyaW5nQ29udGV4dDJELmNwcAorKysgYi9Tb3VyY2Uv V2ViQ29yZS9odG1sL2NhbnZhcy9DYW52YXNSZW5kZXJpbmdDb250ZXh0MkQuY3BwCkBAIC05NTQs MTcgKzk1NCwxNyBAQCB2b2lkIENhbnZhc1JlbmRlcmluZ0NvbnRleHQyRDo6c3Ryb2tlKCkKIAog ICAgIGlmICghbV9wYXRoLmlzRW1wdHkoKSkgewogI2lmIFBMQVRGT1JNKFFUKQorICAgICAgICBG bG9hdFJlY3QgZGlydHlSZWN0ID0gbV9wYXRoLnBsYXRmb3JtUGF0aCgpLmNvbnRyb2xQb2ludFJl Y3QoKTsKKyNlbHNlCisgICAgICAgIEZsb2F0UmVjdCBkaXJ0eVJlY3QgPSBtX3BhdGguYm91bmRp bmdSZWN0KCk7CisjZW5kaWYKICAgICAgICAgLy8gRmFzdCBhcHByb3hpbWF0aW9uIG9mIHRoZSBz dHJva2UncyBib3VuZGluZyByZWN0LgogICAgICAgICAvLyBUaGlzIHlpZWxkcyBhIHNsaWdodGx5 IG92ZXJzaXplZCByZWN0IGJ1dCBpcyB2ZXJ5IGZhc3QKICAgICAgICAgLy8gY29tcGFyZWQgdG8g UGF0aDo6c3Ryb2tlQm91bmRpbmdSZWN0KCkuCi0gICAgICAgIEZsb2F0UmVjdCBib3VuZGluZ1Jl Y3QgPSBtX3BhdGgucGxhdGZvcm1QYXRoKCkuY29udHJvbFBvaW50UmVjdCgpOwotICAgICAgICBi b3VuZGluZ1JlY3QuaW5mbGF0ZShzdGF0ZSgpLm1fbWl0ZXJMaW1pdCArIHN0YXRlKCkubV9saW5l V2lkdGgpOwotI2Vsc2UKLSAgICAgICAgQ2FudmFzU3Ryb2tlU3R5bGVBcHBsaWVyIHN0cm9rZUFw cGxpZXIodGhpcyk7Ci0gICAgICAgIEZsb2F0UmVjdCBib3VuZGluZ1JlY3QgPSBtX3BhdGguc3Ry b2tlQm91bmRpbmdSZWN0KCZzdHJva2VBcHBsaWVyKTsKLSNlbmRpZgorICAgICAgICBkaXJ0eVJl Y3QuaW5mbGF0ZShzdGF0ZSgpLm1fbWl0ZXJMaW1pdCArIHN0YXRlKCkubV9saW5lV2lkdGgpOwor CiAgICAgICAgIGMtPnN0cm9rZVBhdGgobV9wYXRoKTsKLSAgICAgICAgZGlkRHJhdyhib3VuZGlu Z1JlY3QpOworICAgICAgICBkaWREcmF3KGRpcnR5UmVjdCk7CiAgICAgfQogCiAjaWYgRU5BQkxF KERBU0hCT0FSRF9TVVBQT1JUKQo=