44014 2010-08-14 13:39:09 -0700 [Qt] Path: Fast approximation of stroke bounding rects 2010-08-17 11:44:15 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED Performance, Qt, QtTriaged P2 Normal --- 44018 1 kling webkit-qt-unassigned ariya.hidayat oldest_to_newest 264734 0 kling 2010-08-14 13:39:09 -0700 Path::strokeBoundingRect() is used by CanvasRenderingContext2D to determine the dirty region following a stroke() call. We currently implement this using QPainterPathStroker which is very expensive for complex paths. Instead of this we can use QPainterPath::controlPointRect() to approximate the bounding rect. The result will be slightly larger than what the current implementation would return but will run in a fraction of the time. 264735 1 kling 2010-08-14 13:42:36 -0700 Example page that spends a lot of time in Qt's strokeBoundingRect(): http://sebleedelisle.com/demos/html5landscape.html 264736 2 64425 kling 2010-08-14 13:43:21 -0700 Created attachment 64425 Proposed patch 264749 3 kling 2010-08-14 14:48:33 -0700 (In reply to comment #1) > Example page that spends a lot of time in Qt's strokeBoundingRect(): > http://sebleedelisle.com/demos/html5landscape.html Speed improvement for this page with the proposed patch: 18.6% About the patch, if we want to narrow down the returned rect further, I suppose the lineWidth could be divided by 2. 264753 4 64425 ariya.hidayat 2010-08-14 15:15:18 -0700 Comment on attachment 64425 Proposed patch Very nice! 264755 5 64425 kling 2010-08-14 15:44:26 -0700 Comment on attachment 64425 Proposed patch Clearing flags on attachment: 64425 Committed r65374: <http://trac.webkit.org/changeset/65374> 264756 6 kling 2010-08-14 15:44:35 -0700 All reviewed patches have been landed. Closing bug. 264765 7 webkit.review.bot 2010-08-14 16:16:18 -0700 http://trac.webkit.org/changeset/65374 might have broken Qt Linux Release 264766 8 kling 2010-08-14 16:21:06 -0700 (In reply to comment #7) > http://trac.webkit.org/changeset/65374 might have broken Qt Linux Release Gaaahhh, that's what I get for compiling without SVG support.. 264908 9 kling 2010-08-15 17:43:40 -0700 http://trac.webkit.org/changeset/65374 broke a bunch of SVG tests. Reopening. 265649 10 64609 kling 2010-08-17 11:06:18 -0700 Created attachment 64609 Proposed patch v2 Move the (Qt-specific) logic in CRC2D::stroke() this time so as not to affect SVG. 265661 11 64609 ariya.hidayat 2010-08-17 11:20:15 -0700 Comment on attachment 64609 Proposed patch v2 WebCore/html/canvas/CanvasRenderingContext2D.cpp:833 + boundingRect.inflate(state().m_miterLimit + state().m_lineWidth); This means we still ignore the CanvasStrokeStyleApplier, not sure how severe it would be. A workaround is to expand the boundingRect by the stroke width, sadly this does not take into account miter join. Clearing the review flag while this is being thought off. 265669 12 64609 ariya.hidayat 2010-08-17 11:31:55 -0700 Comment on attachment 64609 Proposed patch v2 LGTM. re=me 265673 13 64609 kling 2010-08-17 11:44:04 -0700 Comment on attachment 64609 Proposed patch v2 Clearing flags on attachment: 64609 Committed r65525: <http://trac.webkit.org/changeset/65525> 265674 14 kling 2010-08-17 11:44:15 -0700 All reviewed patches have been landed. Closing bug. 64425 2010-08-14 13:43:21 -0700 2010-08-17 11:06:18 -0700 Proposed patch bug-44014.diff text/plain 1900 kling ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NDYzZGI3Ni4uOWNlZGZjZiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAxMC0wOC0xNCAgQW5kcmVhcyBLbGlu ZyAgPGFuZHJlYXMua2xpbmdAbm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFtRdF0gUGF0aDogRmFzdCBhcHByb3hpbWF0aW9uIG9mIHN0 cm9rZSBib3VuZGluZyByZWN0cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9NDQwMTQKKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3F0L1BhdGhR dC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpQYXRoOjpzdHJva2VCb3VuZGluZ1JlY3QpOiBVc2Ug UVBhaW50ZXJQYXRoOjpjb250cm9sUG9pbnRSZWN0KCkKKyAgICAgICAgdG8gY2FsY3VsYXRlIGEg c2xpZ2h0bHkgbGFyZ2VyIGJvdW5kaW5nIHJlY3QgaW4gYSBmcmFjdGlvbiBvZiB0aGUgdGltZS4K KwogMjAxMC0wOC0xNCAgUGF0cmljayBHYW5zdGVyZXIgIDxwYXJvZ2FAcGFyb2dhLmNvbT4KIAog ICAgICAgICBSZXZpZXdlZCBieSBLZW5uZXRoIFJvaGRlIENocmlzdGlhbnNlbi4KZGlmZiAtLWdp dCBhL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvUGF0aFF0LmNwcCBiL1dlYkNvcmUvcGxh dGZvcm0vZ3JhcGhpY3MvcXQvUGF0aFF0LmNwcAppbmRleCBkZTlkZTA3Li44OTZkMGM3IDEwMDY0 NAotLS0gYS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3F0L1BhdGhRdC5jcHAKKysrIGIvV2Vi Q29yZS9wbGF0Zm9ybS9ncmFwaGljcy9xdC9QYXRoUXQuY3BwCkBAIC0xNjMsMTkgKzE2MywxNyBA QCBGbG9hdFJlY3QgUGF0aDo6Ym91bmRpbmdSZWN0KCkgY29uc3QKIEZsb2F0UmVjdCBQYXRoOjpz dHJva2VCb3VuZGluZ1JlY3QoU3Ryb2tlU3R5bGVBcHBsaWVyKiBhcHBsaWVyKQogewogICAgIEdy YXBoaWNzQ29udGV4dCogZ2MgPSBzY3JhdGNoQ29udGV4dCgpOwotICAgIFFQYWludGVyUGF0aFN0 cm9rZXIgc3Ryb2tlOwotICAgIGlmIChhcHBsaWVyKSB7CisKKyAgICBpZiAoYXBwbGllcikKICAg ICAgICAgYXBwbGllci0+c3Ryb2tlU3R5bGUoZ2MpOwogCi0gICAgICAgIFFQZW4gcGVuID0gZ2Mt PnBlbigpOwotICAgICAgICBzdHJva2Uuc2V0V2lkdGgocGVuLndpZHRoRigpKTsKLSAgICAgICAg c3Ryb2tlLnNldENhcFN0eWxlKHBlbi5jYXBTdHlsZSgpKTsKLSAgICAgICAgc3Ryb2tlLnNldEpv aW5TdHlsZShwZW4uam9pblN0eWxlKCkpOwotICAgICAgICBzdHJva2Uuc2V0TWl0ZXJMaW1pdChw ZW4ubWl0ZXJMaW1pdCgpKTsKLSAgICAgICAgc3Ryb2tlLnNldERhc2hQYXR0ZXJuKHBlbi5kYXNo UGF0dGVybigpKTsKLSAgICAgICAgc3Ryb2tlLnNldERhc2hPZmZzZXQocGVuLmRhc2hPZmZzZXQo KSk7Ci0gICAgfQotICAgIHJldHVybiBzdHJva2UuY3JlYXRlU3Ryb2tlKG1fcGF0aCkuYm91bmRp bmdSZWN0KCk7CisgICAgUVBlbiBwZW4gPSBnYy0+cGVuKCk7CisKKyAgICAvLyBGYXN0IGFwcHJv eGltYXRpb24gb2YgdGhlIHN0cm9rZSdzIGJvdW5kaW5nIHJlY3QuCisgICAgLy8gVGhpcyB5aWVs ZHMgYSBzbGlnaHRseSBvdmVyc2l6ZWQgcmVjdCBidXQgaXMgdmVyeSBmYXN0IGNvbXBhcmVkIHRv IFFQYWludGVyUGF0aFN0cm9rZXIuCisgICAgRmxvYXRSZWN0IHJlY3QgPSBtX3BhdGguY29udHJv bFBvaW50UmVjdCgpOworICAgIHJlY3QuaW5mbGF0ZShwZW4ubWl0ZXJMaW1pdCgpICsgcGVuLndp ZHRoRigpKTsKKyAgICByZXR1cm4gcmVjdDsKIH0KIAogdm9pZCBQYXRoOjptb3ZlVG8oY29uc3Qg RmxvYXRQb2ludCYgcG9pbnQpCg== 64609 2010-08-17 11:06:18 -0700 2010-08-17 11:44:04 -0700 Proposed patch v2 bug-44014.diff text/plain 1646 kling ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg MzZiNmQxMi4uZDc1NjAxZSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNiBAQAorMjAxMC0wOC0xNyAgQW5kcmVhcyBLbGlu ZyAgPGFuZHJlYXMua2xpbmdAbm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFtRdF0gUGF0aDogRmFzdCBhcHByb3hpbWF0aW9uIG9mIHN0 cm9rZSBib3VuZGluZyByZWN0cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9NDQwMTQKKworICAgICAgICBJbiBjYW52YXMncyBzdHJva2UoKSwgdXNlIFFQ YWludGVyUGF0aDo6Y29udHJvbFBvaW50UmVjdCgpIHRvIGNhbGN1bGF0ZQorICAgICAgICBhIHNs aWdodGx5IGxhcmdlciBib3VuZGluZyByZWN0IGluIGEgZnJhY3Rpb24gb2YgdGhlIHRpbWUuCisK KyAgICAgICAgKiBodG1sL2NhbnZhcy9DYW52YXNSZW5kZXJpbmdDb250ZXh0MkQuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6Q2FudmFzUmVuZGVyaW5nQ29udGV4dDJEOjpzdHJva2UpOgorCiAyMDEw LTA4LTE3ICBUb3IgQXJuZSBWZXN0YsO4ICA8dG9yLmFybmUudmVzdGJvQG5va2lhLmNvbT4KIAog ICAgICAgICBSZXZpZXdlZCBieSBFcmljIENhcmxzb24uCmRpZmYgLS1naXQgYS9XZWJDb3JlL2h0 bWwvY2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5jcHAgYi9XZWJDb3JlL2h0bWwvY2Fu dmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5jcHAKaW5kZXggNzAyYjEwOS4uODhiNWE4NCAx MDA2NDQKLS0tIGEvV2ViQ29yZS9odG1sL2NhbnZhcy9DYW52YXNSZW5kZXJpbmdDb250ZXh0MkQu Y3BwCisrKyBiL1dlYkNvcmUvaHRtbC9jYW52YXMvQ2FudmFzUmVuZGVyaW5nQ29udGV4dDJELmNw cApAQCAtODI1LDggKzgyNSwxNiBAQCB2b2lkIENhbnZhc1JlbmRlcmluZ0NvbnRleHQyRDo6c3Ry b2tlKCkKICAgICAgICAgYy0+YmVnaW5QYXRoKCk7CiAgICAgICAgIGMtPmFkZFBhdGgobV9wYXRo KTsKIAorI2lmIFBMQVRGT1JNKFFUKQorICAgICAgICAvLyBGYXN0IGFwcHJveGltYXRpb24gb2Yg dGhlIHN0cm9rZSdzIGJvdW5kaW5nIHJlY3QuCisgICAgICAgIC8vIFRoaXMgeWllbGRzIGEgc2xp Z2h0bHkgb3ZlcnNpemVkIHJlY3QgYnV0IGlzIHZlcnkgZmFzdAorICAgICAgICAvLyBjb21wYXJl ZCB0byBQYXRoOjpzdHJva2VCb3VuZGluZ1JlY3QoKS4KKyAgICAgICAgRmxvYXRSZWN0IGJvdW5k aW5nUmVjdCA9IG1fcGF0aC5wbGF0Zm9ybVBhdGgoKS5jb250cm9sUG9pbnRSZWN0KCk7CisgICAg ICAgIGJvdW5kaW5nUmVjdC5pbmZsYXRlKHN0YXRlKCkubV9taXRlckxpbWl0ICsgc3RhdGUoKS5t X2xpbmVXaWR0aCk7CisjZWxzZQogICAgICAgICBDYW52YXNTdHJva2VTdHlsZUFwcGxpZXIgc3Ry b2tlQXBwbGllcih0aGlzKTsKICAgICAgICAgRmxvYXRSZWN0IGJvdW5kaW5nUmVjdCA9IG1fcGF0 aC5zdHJva2VCb3VuZGluZ1JlY3QoJnN0cm9rZUFwcGxpZXIpOworI2VuZGlmCiAgICAgICAgIHdp bGxEcmF3KGJvdW5kaW5nUmVjdCk7CiAKICAgICAgICAgYy0+c3Ryb2tlUGF0aCgpOwo=