73830 2011-12-05 07:52:38 -0800 optimize TransformationMatrix::scale by not calling through to generic multiply 2011-12-06 12:44:41 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 reed reed cmarrin dglazkov kbr simon.fraser webkit.review.bot oldest_to_newest 515081 0 reed 2011-12-05 07:52:38 -0800 optimize TransformationMatrix::scale by not calling through to generic multiply 515084 1 117880 reed 2011-12-05 07:54:32 -0800 Created attachment 117880 Patch 515104 2 117880 webkit.review.bot 2011-12-05 08:44:04 -0800 Comment on attachment 117880 Patch Attachment 117880 did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10731720 New failing tests: svg/custom/linking-uri-01-b.svg 515215 3 117880 kbr 2011-12-05 11:07:58 -0800 Comment on attachment 117880 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117880&action=review Is the new SVG failure spurious? It's hard to believe that it could be caused by this patch. > Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:695 > + m_matrix[0][0] *= sx; > + m_matrix[0][1] *= sx; > + m_matrix[0][2] *= sx; > + m_matrix[0][3] *= sx; > + > + m_matrix[1][0] *= sy; > + m_matrix[1][1] *= sy; > + m_matrix[1][2] *= sy; > + m_matrix[1][3] *= sy; Could this one at least call scaleNonUniform to reduce code duplication? Would that eliminate the benefit of the optimization? 515241 4 reed 2011-12-05 11:29:03 -0800 I'll run drt locally and see if I can repro the failure. *possibly* we are getting different precision with this version, and therefore a slightly different matrix? I think we can have the scale3 function call the 2D one, and then scale its column. I'll try that in the next patch. 515828 5 118031 reed 2011-12-06 05:17:25 -0800 Created attachment 118031 Patch 515853 6 reed 2011-12-06 05:57:42 -0800 previous failure must have been a flake, as the new patch is the same (modulo sharing code between the scaleNonUniform and scale3D) 516025 7 118031 kbr 2011-12-06 10:42:42 -0800 Comment on attachment 118031 Patch Looks good. r=me 516073 8 118031 webkit.review.bot 2011-12-06 11:27:50 -0800 Comment on attachment 118031 Patch Clearing flags on attachment: 118031 Committed r102157: <http://trac.webkit.org/changeset/102157> 516074 9 webkit.review.bot 2011-12-06 11:27:55 -0800 All reviewed patches have been landed. Closing bug. 516080 10 simon.fraser 2011-12-06 11:30:19 -0800 Why were none of the main authors of TransformationMatrix.cpp cc'd on this bug? 516089 11 kbr 2011-12-06 11:36:02 -0800 (In reply to comment #10) > Why were none of the main authors of TransformationMatrix.cpp cc'd on this bug? Sorry, that was a simple oversight on my part... Please let us know if you find any issue with this change. I'm happy to roll it out and help rework it if necessary. 516125 12 reed 2011-12-06 11:57:58 -0800 My bad too. Is there a generic way to know who the interested authors are for a given directory/file? 516165 13 reed 2011-12-06 12:44:41 -0800 using 'svn blame' I see cmarrin and simon. Will remember to do that next time. 117880 2011-12-05 07:54:32 -0800 2011-12-06 05:17:22 -0800 Patch bug-73830-20111205105434.patch text/plain 2415 reed SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwMjAwMCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDExLTEyLTA1ICBNaWtlIFJl ZWQgIDxyZWVkQGdvb2dsZS5jb20+CisKKyAgICAgICAgb3B0aW1pemUgVHJhbnNmb3JtYXRpb25N YXRyaXg6OnNjYWxlIGJ5IG5vdCBjYWxsaW5nIHRocm91Z2ggdG8gZ2VuZXJpYyBtdWx0aXBseQor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzM4MzAKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBObyBuZXcgdGVz dHMuIG5vIGZ1bmN0aW9uYWwgY2hhbmdlLCBqdXN0IGZhc3QuCisKKyAgICAgICAgT24gdGhpcyBz aXRlLCBzY2FsZSgpIHdlbnQgZnJvbSAxLjclIG9mIHRoZSB0aW1lIHRvIGVmZmVjdGl2ZWx5IDAK KyAgICAgICAgaHR0cDovL2llLm1pY3Jvc29mdC5jb20vdGVzdGRyaXZlL1BlcmZvcm1hbmNlL1Nw ZWVkUmVhZGluZy9EZWZhdWx0Lmh0bWwKKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3Ry YW5zZm9ybXMvVHJhbnNmb3JtYXRpb25NYXRyaXguY3BwOgorICAgICAgICAoV2ViQ29yZTo6VHJh bnNmb3JtYXRpb25NYXRyaXg6OnNjYWxlTm9uVW5pZm9ybSk6CisgICAgICAgIChXZWJDb3JlOjpU cmFuc2Zvcm1hdGlvbk1hdHJpeDo6c2NhbGUzZCk6CisKIDIwMTEtMTItMDUgIEVyaWMgQ2FybHNv biAgPGVyaWMuY2FybHNvbkBhcHBsZS5jb20+CiAKICAgICAgICAgT3V0LW9mLWJhbmQgdGV4dCB0 cmFja3MgbWF5IG9ubHkgbG9hZCBmcm9tIHNhbWUgb3JpZ2luIGFzIHRoZSBtZWRpYSBlbGVtZW50 J3MgRG9jdW1lbnQncyBvcmlnaW4KSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBo aWNzL3RyYW5zZm9ybXMvVHJhbnNmb3JtYXRpb25NYXRyaXguY3BwCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3RyYW5zZm9ybXMvVHJhbnNmb3JtYXRpb25N YXRyaXguY3BwCShyZXZpc2lvbiAxMDE5ODkpCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9n cmFwaGljcy90cmFuc2Zvcm1zL1RyYW5zZm9ybWF0aW9uTWF0cml4LmNwcAkod29ya2luZyBjb3B5 KQpAQCAtNjcwLDIyICs2NzAsMzQgQEAgRmxvYXRRdWFkIFRyYW5zZm9ybWF0aW9uTWF0cml4Ojpt YXBRdWFkKAogCiBUcmFuc2Zvcm1hdGlvbk1hdHJpeCYgVHJhbnNmb3JtYXRpb25NYXRyaXg6OnNj YWxlTm9uVW5pZm9ybShkb3VibGUgc3gsIGRvdWJsZSBzeSkKIHsKLSAgICBUcmFuc2Zvcm1hdGlv bk1hdHJpeCBtYXQ7Ci0gICAgbWF0Lm1fbWF0cml4WzBdWzBdID0gc3g7Ci0gICAgbWF0Lm1fbWF0 cml4WzFdWzFdID0gc3k7Ci0KLSAgICBtdWx0aXBseShtYXQpOworICAgIG1fbWF0cml4WzBdWzBd ICo9IHN4OworICAgIG1fbWF0cml4WzBdWzFdICo9IHN4OworICAgIG1fbWF0cml4WzBdWzJdICo9 IHN4OworICAgIG1fbWF0cml4WzBdWzNdICo9IHN4OworICAgIAorICAgIG1fbWF0cml4WzFdWzBd ICo9IHN5OworICAgIG1fbWF0cml4WzFdWzFdICo9IHN5OworICAgIG1fbWF0cml4WzFdWzJdICo9 IHN5OworICAgIG1fbWF0cml4WzFdWzNdICo9IHN5OwogICAgIHJldHVybiAqdGhpczsKIH0KIAog VHJhbnNmb3JtYXRpb25NYXRyaXgmIFRyYW5zZm9ybWF0aW9uTWF0cml4OjpzY2FsZTNkKGRvdWJs ZSBzeCwgZG91YmxlIHN5LCBkb3VibGUgc3opCiB7Ci0gICAgVHJhbnNmb3JtYXRpb25NYXRyaXgg bWF0OwotICAgIG1hdC5tX21hdHJpeFswXVswXSA9IHN4OwotICAgIG1hdC5tX21hdHJpeFsxXVsx XSA9IHN5OwotICAgIG1hdC5tX21hdHJpeFsyXVsyXSA9IHN6OwotCi0gICAgbXVsdGlwbHkobWF0 KTsKKyAgICBtX21hdHJpeFswXVswXSAqPSBzeDsKKyAgICBtX21hdHJpeFswXVsxXSAqPSBzeDsK KyAgICBtX21hdHJpeFswXVsyXSAqPSBzeDsKKyAgICBtX21hdHJpeFswXVszXSAqPSBzeDsKKyAg ICAKKyAgICBtX21hdHJpeFsxXVswXSAqPSBzeTsKKyAgICBtX21hdHJpeFsxXVsxXSAqPSBzeTsK KyAgICBtX21hdHJpeFsxXVsyXSAqPSBzeTsKKyAgICBtX21hdHJpeFsxXVszXSAqPSBzeTsKKyAg ICAKKyAgICBtX21hdHJpeFsyXVswXSAqPSBzejsKKyAgICBtX21hdHJpeFsyXVsxXSAqPSBzejsK KyAgICBtX21hdHJpeFsyXVsyXSAqPSBzejsKKyAgICBtX21hdHJpeFsyXVszXSAqPSBzejsKICAg ICByZXR1cm4gKnRoaXM7CiB9CiAK 118031 2011-12-06 05:17:25 -0800 2011-12-06 11:27:50 -0800 Patch bug-73830-20111206081724.patch text/plain 2026 reed SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwMjEyMykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDExLTEyLTA2ICBNaWtlIFJl ZWQgIDxyZWVkQGdvb2dsZS5jb20+CisKKyAgICAgICAgb3B0aW1pemUgVHJhbnNmb3JtYXRpb25N YXRyaXg6OnNjYWxlIGJ5IG5vdCBjYWxsaW5nIHRocm91Z2ggdG8gZ2VuZXJpYyBtdWx0aXBseQor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzM4MzAKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBObyBuZXcgdGVz dHMuIE9wdGltaXphdGlvbiBvbmx5LCBleGlzdGluZyB0ZXN0cyBleGVyY2lzZSB0aGUgY29kZQor CisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvdHJhbnNmb3Jtcy9UcmFuc2Zvcm1hdGlvbk1h dHJpeC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpUcmFuc2Zvcm1hdGlvbk1hdHJpeDo6c2NhbGVO b25Vbmlmb3JtKToKKyAgICAgICAgKFdlYkNvcmU6OlRyYW5zZm9ybWF0aW9uTWF0cml4OjpzY2Fs ZTNkKToKKwogMjAxMS0xMi0wNiAgQmVuamFtaW4gUG91bGFpbiAgPGJlbmphbWluQHdlYmtpdC5v cmc+CiAKICAgICAgICAgU2ltcGxpZnkgS1VSTCdzIGNoZWNrRW5jb2RlZFN0cmluZygpCkluZGV4 OiBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy90cmFuc2Zvcm1zL1RyYW5zZm9ybWF0 aW9uTWF0cml4LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy90cmFuc2Zvcm1zL1RyYW5zZm9ybWF0aW9uTWF0cml4LmNwcAkocmV2aXNpb24gMTAyMTIx KQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvdHJhbnNmb3Jtcy9UcmFuc2Zv cm1hdGlvbk1hdHJpeC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTY3MCwyMiArNjcwLDI2IEBAIEZs b2F0UXVhZCBUcmFuc2Zvcm1hdGlvbk1hdHJpeDo6bWFwUXVhZCgKIAogVHJhbnNmb3JtYXRpb25N YXRyaXgmIFRyYW5zZm9ybWF0aW9uTWF0cml4OjpzY2FsZU5vblVuaWZvcm0oZG91YmxlIHN4LCBk b3VibGUgc3kpCiB7Ci0gICAgVHJhbnNmb3JtYXRpb25NYXRyaXggbWF0OwotICAgIG1hdC5tX21h dHJpeFswXVswXSA9IHN4OwotICAgIG1hdC5tX21hdHJpeFsxXVsxXSA9IHN5OwotCi0gICAgbXVs dGlwbHkobWF0KTsKKyAgICBtX21hdHJpeFswXVswXSAqPSBzeDsKKyAgICBtX21hdHJpeFswXVsx XSAqPSBzeDsKKyAgICBtX21hdHJpeFswXVsyXSAqPSBzeDsKKyAgICBtX21hdHJpeFswXVszXSAq PSBzeDsKKyAgICAKKyAgICBtX21hdHJpeFsxXVswXSAqPSBzeTsKKyAgICBtX21hdHJpeFsxXVsx XSAqPSBzeTsKKyAgICBtX21hdHJpeFsxXVsyXSAqPSBzeTsKKyAgICBtX21hdHJpeFsxXVszXSAq PSBzeTsKICAgICByZXR1cm4gKnRoaXM7CiB9CiAKIFRyYW5zZm9ybWF0aW9uTWF0cml4JiBUcmFu c2Zvcm1hdGlvbk1hdHJpeDo6c2NhbGUzZChkb3VibGUgc3gsIGRvdWJsZSBzeSwgZG91YmxlIHN6 KQogewotICAgIFRyYW5zZm9ybWF0aW9uTWF0cml4IG1hdDsKLSAgICBtYXQubV9tYXRyaXhbMF1b MF0gPSBzeDsKLSAgICBtYXQubV9tYXRyaXhbMV1bMV0gPSBzeTsKLSAgICBtYXQubV9tYXRyaXhb Ml1bMl0gPSBzejsKLQotICAgIG11bHRpcGx5KG1hdCk7CisgICAgc2NhbGVOb25Vbmlmb3JtKHN4 LCBzeSk7CisgICAgCisgICAgbV9tYXRyaXhbMl1bMF0gKj0gc3o7CisgICAgbV9tYXRyaXhbMl1b MV0gKj0gc3o7CisgICAgbV9tYXRyaXhbMl1bMl0gKj0gc3o7CisgICAgbV9tYXRyaXhbMl1bM10g Kj0gc3o7CiAgICAgcmV0dXJuICp0aGlzOwogfQogCg==