66854 2011-08-24 06:43:13 -0700 [Qt] Path::boundingRect() is unnecessarily slow. 2011-08-26 06:36:14 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED Qt, QtTriaged P2 Normal --- 66945 1 kling kling benjamin ossy tonikitoo webkit.review.bot oldest_to_newest 456073 0 kling 2011-08-24 06:43:13 -0700 This will allow us to remove the Qt-specific optimization in CanvasRenderingContext2D::stroke(). 456074 1 104989 kling 2011-08-24 06:43:51 -0700 Created attachment 104989 Proposed patch 456090 2 104989 kling 2011-08-24 07:28:52 -0700 Comment on attachment 104989 Proposed patch Clearing r? while discussing this with some random Belgian person. 456753 3 kling 2011-08-25 03:43:09 -0700 Looking at other ports, and clients of Path::boundingRect(), it turns out that it's not necessary to return a 100% accurate bounding rectangle, since it's only used for dirty region computation and short-circuiting the SVG path-based elements' hit testing. Changing the scope of the bug to always use QPainterPath::controlPointRect() in Qt's Path::boundingRect() since this is significantly faster and matches the current behavior of (at least) Skia's Path. 456754 4 105159 kling 2011-08-25 03:47:40 -0700 Created attachment 105159 Proposed patch 456756 5 105159 benjamin 2011-08-25 03:49:23 -0700 Comment on attachment 105159 Proposed patch Great 456766 6 105159 webkit.review.bot 2011-08-25 04:48:17 -0700 Comment on attachment 105159 Proposed patch Clearing flags on attachment: 105159 Committed r93774: <http://trac.webkit.org/changeset/93774> 456767 7 webkit.review.bot 2011-08-25 04:48:23 -0700 All reviewed patches have been landed. Closing bug. 456896 8 ossy 2011-08-25 09:02:30 -0700 Reopen, because it was rolled out by https://trac.webkit.org/changeset/93790 This patch broke zillion tests on the Qt bot. You guys should have run layout tests before landing. Please fix the bug before re-landing. 456915 9 tonikitoo 2011-08-25 09:40:46 -0700 (In reply to comment #8) > Reopen, because it was rolled out by https://trac.webkit.org/changeset/93790 > > This patch broke zillion tests on the Qt bot. You guys should have run > layout tests before landing. Please fix the bug before re-landing. zillion != 26 :) 456917 10 ossy 2011-08-25 09:42:13 -0700 (In reply to comment #9) > zillion != 26 :) You're right, but 26 !=0, and all test must pass. 456973 11 kling 2011-08-25 10:39:34 -0700 (In reply to comment #10) > (In reply to comment #9) > > zillion != 26 :) > > You're right, but 26 !=0, and all test must pass. Like I said on IRC, those failures are actually progressions and should just be updated. I couldn't do it at the time because I didn't have a checkout. I'll roll it back in along with a rebaseline. Thanks for playing another round of the mindless rollout game :) 456977 12 ossy 2011-08-25 10:44:20 -0700 I don't know anything about your patch... I tried to ping you on #qtwebkit, but you didn't answer. I had to do it, because red bot can't catch new regressions ... That's why we need always green bots. That's why developers have to run layout tests before landing and not leave the tree red for hours ... 457543 13 kling 2011-08-26 06:36:14 -0700 Committed r93873: <http://trac.webkit.org/changeset/93873> 104989 2011-08-24 06:43:51 -0700 2011-08-25 03:47:40 -0700 Proposed patch bug-66854.diff text/plain 3679 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAwNDNjNjhmLi40NTk1MWZlIDEwMDc1NQotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjIg QEAKKzIwMTEtMDgtMjQgIEFuZHJlYXMgS2xpbmcgIDxrbGluZ0B3ZWJraXQub3JnPgorCisgICAg ICAgIFBhdGg6IEFkZCBmYXN0Qm91bmRpbmdSZWN0KCkgZm9yIGZhc3QsIGFwcHJveGltYXRlIGJv dW5kaW5nIHJlY3RzLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9NjY4NTQKKworICAgICAgICBUaGlzIGlzIG9ubHkgc3BlY2lhbGl6ZWQgZm9yIFF0IHNv IGZhciwgb3RoZXIgcG9ydHMgc2ltcGx5IHJldHVybgorICAgICAgICBQYXRoOjpib3VuZGluZ1Jl Y3QoKS4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAq IGh0bWwvY2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5jcHA6CisgICAgICAgIChXZWJD b3JlOjpDYW52YXNSZW5kZXJpbmdDb250ZXh0MkQ6OnN0cm9rZSk6IENhbGwgZmFzdEJvdW5kaW5n UmVjdCgpCisgICAgICAgIHRvIGNvbXB1dGUgdGhlIGRpcnR5IHJlY3QgYW5kIHJlbW92ZSBhIFF0 LXNwZWNpZmljIG9wdGltaXphdGlvbi4KKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9QYXRo LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlBhdGg6OmZhc3RCb3VuZGluZ1JlY3QpOgorICAgICAg ICAqIHBsYXRmb3JtL2dyYXBoaWNzL1BhdGguaDoKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGlj cy9xdC9QYXRoUXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6UGF0aDo6ZmFzdEJvdW5kaW5nUmVj dCk6CisKIDIwMTEtMDgtMjMgIEFuZHJlYXMgS2xpbmcgIDxrbGluZ0B3ZWJraXQub3JnPgogCiAg ICAgICAgIEhUTUxJbWFnZUVsZW1lbnQ6IFJlb3JkZXIgbWVtYmVycyB0byByZWR1Y2UgbWVtb3J5 IHVzYWdlLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC9jYW52YXMvQ2FudmFzUmVu ZGVyaW5nQ29udGV4dDJELmNwcCBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvY2FudmFzL0NhbnZhc1Jl bmRlcmluZ0NvbnRleHQyRC5jcHAKaW5kZXggYTQ3MGNiYS4uYzg3MWZjNyAxMDA2NDQKLS0tIGEv U291cmNlL1dlYkNvcmUvaHRtbC9jYW52YXMvQ2FudmFzUmVuZGVyaW5nQ29udGV4dDJELmNwcAor KysgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL2NhbnZhcy9DYW52YXNSZW5kZXJpbmdDb250ZXh0MkQu Y3BwCkBAIC05MjQsMTEgKzkyNCw3IEBAIHZvaWQgQ2FudmFzUmVuZGVyaW5nQ29udGV4dDJEOjpz dHJva2UoKQogICAgICAgICByZXR1cm47CiAKICAgICBpZiAoIW1fcGF0aC5pc0VtcHR5KCkpIHsK LSNpZiBQTEFURk9STShRVCkKLSAgICAgICAgRmxvYXRSZWN0IGRpcnR5UmVjdCA9IG1fcGF0aC5w bGF0Zm9ybVBhdGgoKS5jb250cm9sUG9pbnRSZWN0KCk7Ci0jZWxzZQotICAgICAgICBGbG9hdFJl Y3QgZGlydHlSZWN0ID0gbV9wYXRoLmJvdW5kaW5nUmVjdCgpOwotI2VuZGlmCisgICAgICAgIEZs b2F0UmVjdCBkaXJ0eVJlY3QgPSBtX3BhdGguZmFzdEJvdW5kaW5nUmVjdCgpOwogICAgICAgICAv LyBGYXN0IGFwcHJveGltYXRpb24gb2YgdGhlIHN0cm9rZSdzIGJvdW5kaW5nIHJlY3QuCiAgICAg ICAgIC8vIFRoaXMgeWllbGRzIGEgc2xpZ2h0bHkgb3ZlcnNpemVkIHJlY3QgYnV0IGlzIHZlcnkg ZmFzdAogICAgICAgICAvLyBjb21wYXJlZCB0byBQYXRoOjpzdHJva2VCb3VuZGluZ1JlY3QoKS4K ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL1BhdGguY3BwIGIv U291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvUGF0aC5jcHAKaW5kZXggZmYwZmQyMi4u ZjdmZWJkMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvUGF0 aC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvUGF0aC5jcHAKQEAg LTkyLDYgKzkyLDEzIEBAIGZsb2F0IFBhdGg6Om5vcm1hbEFuZ2xlQXRMZW5ndGgoZmxvYXQgbGVu Z3RoLCBib29sJiBvaykgY29uc3QKIH0KICNlbmRpZgogCisjaWYgIVBMQVRGT1JNKFFUKQorRmxv YXRSZWN0IFBhdGg6OmZhc3RCb3VuZGluZ1JlY3QoKSBjb25zdAoreworICAgIHJldHVybiBib3Vu ZGluZ1JlY3QoKTsKK30KKyNlbmRpZgorCiB2b2lkIFBhdGg6OmFkZFJvdW5kZWRSZWN0KGNvbnN0 IFJvdW5kZWRSZWN0JiByKQogewogICAgIGFkZFJvdW5kZWRSZWN0KHIucmVjdCgpLCByLnJhZGlp KCkudG9wTGVmdCgpLCByLnJhZGlpKCkudG9wUmlnaHQoKSwgci5yYWRpaSgpLmJvdHRvbUxlZnQo KSwgci5yYWRpaSgpLmJvdHRvbVJpZ2h0KCkpOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vZ3JhcGhpY3MvUGF0aC5oIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3MvUGF0aC5oCmluZGV4IDRiZDAwNjguLmNmMDBjYjcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL3BsYXRmb3JtL2dyYXBoaWNzL1BhdGguaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9ncmFwaGljcy9QYXRoLmgKQEAgLTExMCw2ICsxMTAsOSBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7 CiAgICAgICAgIGJvb2wgc3Ryb2tlQ29udGFpbnMoU3Ryb2tlU3R5bGVBcHBsaWVyKiwgY29uc3Qg RmxvYXRQb2ludCYpIGNvbnN0OwogICAgICAgICBGbG9hdFJlY3QgYm91bmRpbmdSZWN0KCkgY29u c3Q7CiAgICAgICAgIEZsb2F0UmVjdCBzdHJva2VCb3VuZGluZ1JlY3QoU3Ryb2tlU3R5bGVBcHBs aWVyKiA9IDApIGNvbnN0OworCisgICAgICAgIC8vIFJldHVybnMgYSBib3VuZGluZyByZWN0IHRo YXQgbWF5IGJlIHNsaWdodGx5IGxhcmdlciB0aGFuIGJvdW5kaW5nUmVjdCgpIGJ1dCBuZXZlciBz bWFsbGVyLgorICAgICAgICBGbG9hdFJlY3QgZmFzdEJvdW5kaW5nUmVjdCgpIGNvbnN0OwogICAg ICAgICAKICAgICAgICAgZmxvYXQgbGVuZ3RoKCkgY29uc3Q7CiAgICAgICAgIEZsb2F0UG9pbnQg cG9pbnRBdExlbmd0aChmbG9hdCBsZW5ndGgsIGJvb2wmIG9rKSBjb25zdDsKZGlmZiAtLWdpdCBh L1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3F0L1BhdGhRdC5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9xdC9QYXRoUXQuY3BwCmluZGV4IDQ3YmVlN2EuLjgz MjM1NjEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3F0L1Bh dGhRdC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvUGF0aFF0 LmNwcApAQCAtMTUyLDYgKzE1MiwxMSBAQCBGbG9hdFJlY3QgUGF0aDo6Ym91bmRpbmdSZWN0KCkg Y29uc3QKICAgICByZXR1cm4gbV9wYXRoLmJvdW5kaW5nUmVjdCgpOwogfQogCitGbG9hdFJlY3Qg UGF0aDo6ZmFzdEJvdW5kaW5nUmVjdCgpIGNvbnN0Cit7CisgICAgcmV0dXJuIG1fcGF0aC5jb250 cm9sUG9pbnRSZWN0KCk7Cit9CisKIEZsb2F0UmVjdCBQYXRoOjpzdHJva2VCb3VuZGluZ1JlY3Qo U3Ryb2tlU3R5bGVBcHBsaWVyKiBhcHBsaWVyKSBjb25zdAogewogICAgIEdyYXBoaWNzQ29udGV4 dCogY29udGV4dCA9IHNjcmF0Y2hDb250ZXh0KCk7Cg== 105159 2011-08-25 03:47:40 -0700 2011-08-25 04:48:17 -0700 Proposed patch bug-66854.diff text/plain 2700 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA1ZmQ4ODM4Li5iMjMyMTNlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTgg QEAKKzIwMTEtMDgtMjUgIEFuZHJlYXMgS2xpbmcgIDxrbGluZ0B3ZWJraXQub3JnPgorCisgICAg ICAgIFtRdF0gUGF0aDo6Ym91bmRpbmdSZWN0KCkgaXMgdW5uZWNlc3NhcmlseSBzbG93LgorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjY4NTQKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGh0bWwvY2FudmFz L0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpDYW52YXNS ZW5kZXJpbmdDb250ZXh0MkQ6OnN0cm9rZSk6IFJlbW92ZSBRdC1zcGVjaWZpYyBvcHRpbWl6YXRp b24KKyAgICAgICAgc2luY2UgY2FsbGluZyBQYXRoOjpib3VuZGluZ1JlY3QoKSB3aWxsIG5vdyBk byBleGFjdGx5IHRoZSBzYW1lIHRoaW5nLgorICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3F0 L1BhdGhRdC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpQYXRoOjpib3VuZGluZ1JlY3QpOiBSZXR1 cm4gUVBhaW50ZXJQYXRoOjpjb250cm9sUG9pbnRSZWN0KCkgc2luY2UKKyAgICAgICAgdGhhdCBp cyBtdWNoIGZhc3RlciB0aGFuIGNvbXB1dGluZyB0aGUgKHByZWNpc2UpIFFQYWludGVyUGF0aDo6 Ym91bmRpbmdSZWN0KCkKKyAgICAgICAgYW5kIFBhdGg6OmJvdW5kaW5nUmVjdCgpIGlzIG5vdCBl eHBlY3RlZCB0byBiZSAxMDAlIHByZWNpc2UgYW55d2F5LgorCiAyMDExLTA4LTI1ICBKZXJlbXkg TW9za292aWNoICA8amVyZW15QGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZW1vdmUgQVRTRm9u dCooKSBmdW5jdGlvbnMgaW4gZmF2b3Igb2YgQ0dGb250IGluIEZvbnRDdXN0b21QbGF0Zm9ybURh dGEuY3BwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9odG1sL2NhbnZhcy9DYW52YXNSZW5k ZXJpbmdDb250ZXh0MkQuY3BwIGIvU291cmNlL1dlYkNvcmUvaHRtbC9jYW52YXMvQ2FudmFzUmVu ZGVyaW5nQ29udGV4dDJELmNwcAppbmRleCA5ZDg1OWMzLi41NDkwMGQ3IDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViQ29yZS9odG1sL2NhbnZhcy9DYW52YXNSZW5kZXJpbmdDb250ZXh0MkQuY3BwCisr KyBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvY2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5j cHAKQEAgLTkyNCwxMSArOTI0LDcgQEAgdm9pZCBDYW52YXNSZW5kZXJpbmdDb250ZXh0MkQ6OnN0 cm9rZSgpCiAgICAgICAgIHJldHVybjsKIAogICAgIGlmICghbV9wYXRoLmlzRW1wdHkoKSkgewot I2lmIFBMQVRGT1JNKFFUKQotICAgICAgICBGbG9hdFJlY3QgZGlydHlSZWN0ID0gbV9wYXRoLnBs YXRmb3JtUGF0aCgpLmNvbnRyb2xQb2ludFJlY3QoKTsKLSNlbHNlCiAgICAgICAgIEZsb2F0UmVj dCBkaXJ0eVJlY3QgPSBtX3BhdGguYm91bmRpbmdSZWN0KCk7Ci0jZW5kaWYKICAgICAgICAgLy8g RmFzdCBhcHByb3hpbWF0aW9uIG9mIHRoZSBzdHJva2UncyBib3VuZGluZyByZWN0LgogICAgICAg ICAvLyBUaGlzIHlpZWxkcyBhIHNsaWdodGx5IG92ZXJzaXplZCByZWN0IGJ1dCBpcyB2ZXJ5IGZh c3QKICAgICAgICAgLy8gY29tcGFyZWQgdG8gUGF0aDo6c3Ryb2tlQm91bmRpbmdSZWN0KCkuCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9xdC9QYXRoUXQuY3Bw IGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvUGF0aFF0LmNwcAppbmRleCA0 N2JlZTdhLi4zYjBjYTFiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9xdC9QYXRoUXQuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNz L3F0L1BhdGhRdC5jcHAKQEAgLTEsNyArMSw4IEBACiAvKgotICogQ29weXJpZ2h0IChDKSAyMDA2 IFphY2sgUnVzaW4gICA8emFja0BrZGUub3JnPgotICogICAgICAgICAgICAgICAyMDA2IFJvYiBC dWlzICAgICA8YnVpc0BrZGUub3JnPgotICogICAgICAgICAgICAgICAyMDA5LCAyMDEwIERpcmsg U2NodWx6ZSA8a3JpdEB3ZWJraXQub3JnPgorICogQ29weXJpZ2h0IChDKSAyMDA2IFphY2sgUnVz aW4gPHphY2tAa2RlLm9yZz4KKyAqIENvcHlyaWdodCAoQykgMjAwNiBSb2IgQnVpcyA8YnVpc0Br ZGUub3JnPgorICogQ29weXJpZ2h0IChDKSAyMDA5LCAyMDEwIERpcmsgU2NodWx6ZSA8a3JpdEB3 ZWJraXQub3JnPgorICogQ29weXJpZ2h0IChDKSAyMDEwLCAyMDExIEFuZHJlYXMgS2xpbmcgPGts aW5nQHdlYmtpdC5vcmc+CiAgKgogICogQWxsIHJpZ2h0cyByZXNlcnZlZC4KICAqCkBAIC0xNDks NyArMTUwLDcgQEAgdm9pZCBQYXRoOjp0cmFuc2xhdGUoY29uc3QgRmxvYXRTaXplJiBzaXplKQog CiBGbG9hdFJlY3QgUGF0aDo6Ym91bmRpbmdSZWN0KCkgY29uc3QKIHsKLSAgICByZXR1cm4gbV9w YXRoLmJvdW5kaW5nUmVjdCgpOworICAgIHJldHVybiBtX3BhdGguY29udHJvbFBvaW50UmVjdCgp OwogfQogCiBGbG9hdFJlY3QgUGF0aDo6c3Ryb2tlQm91bmRpbmdSZWN0KFN0cm9rZVN0eWxlQXBw bGllciogYXBwbGllcikgY29uc3QK