70646 2011-10-21 13:56:45 -0700 Ensure periodic flushing of accelerated canvas drawing context 2011-10-21 17:00:57 -0700 1 1 1 Unclassified WebKit Canvas 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mdelaney7 mdelaney7 darin mdelaney7 simon.fraser oldest_to_newest 488635 0 mdelaney7 2011-10-21 13:56:45 -0700 If flushes don't happen regularly enough on a canvas drawing context that is accelerated, subsequent call performance can get pretty bad. "Regularly enough" is geared toward a 30-60hz paint cycle. So, this patch ensures that we're getting flushes of a rate in that range if they're not already happening (e.g. from HTMLCanvasElement::paint calls) in that range. 488650 1 112020 mdelaney7 2011-10-21 14:15:03 -0700 Created attachment 112020 Patch 488657 2 112020 simon.fraser 2011-10-21 14:23:01 -0700 Comment on attachment 112020 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112020&action=review > Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp:168 > + m_context = adoptPtr(new GraphicsContext(cgContext.get())); Should you set m_data.m_lastFlushTime on context creation? 488660 3 112020 darin 2011-10-21 14:25:47 -0700 Comment on attachment 112020 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112020&action=review > Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp:184 > GraphicsContext* ImageBuffer::context() const Is this really the best bottleneck for the timing logic? Can we instead put this at the ends of the drawing operations that accumulate state in the context? Or are those too many of those. This hook seems particularly awkward since it’s just a getter. 488662 4 112020 darin 2011-10-21 14:27:51 -0700 Comment on attachment 112020 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112020&action=review > Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp:188 > + double elapsedTime = currentTimeMS() - m_data.m_lastFlushTime; What is the performance cost of doing this every time we call context()? Seems like it might be high. 488702 5 mdelaney7 2011-10-21 15:08:04 -0700 (In reply to comment #2) > Should you set m_data.m_lastFlushTime on context creation? Yea, that's better. (In reply to comment #3) > > Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp:184 > > GraphicsContext* ImageBuffer::context() const > > Is this really the best bottleneck for the timing logic? Can we instead put this at the ends of the drawing operations that accumulate state in the context? Or are those too many of those. This hook seems particularly awkward since it’s just a getter. That's what I had originally, but there are about 5-10 calls and it would require having all this logic in GraphicsContext which becomes a bit more cluttered. (In reply to comment #4) > (From update of attachment 112020 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=112020&action=review > > > Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp:188 > > + double elapsedTime = currentTimeMS() - m_data.m_lastFlushTime; > > What is the performance cost of doing this every time we call context()? Seems like it might be high. Running it locally, I didn't notice any difference in perf on one page I had open. I'll try it out on a handful of other pages now. 488761 6 mdelaney7 2011-10-21 16:42:55 -0700 Couldn't find any performance deficit, so I think we're good. Committing now. 488768 7 mdelaney7 2011-10-21 17:00:57 -0700 Committed r98171: <http://trac.webkit.org/changeset/98171> 112020 2011-10-21 14:15:03 -0700 2011-10-21 14:27:51 -0700 Patch bug-70646-20111021141502.patch text/plain 6280 mdelaney7 SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk4MTQ4KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjUgQEAKKzIwMTEtMTAtMjEgIE1hdHRoZXcg RGVsYW5leSAgPG1kZWxhbmV5QGFwcGxlLmNvbT4KKworICAgICAgICBFbnN1cmUgcGVyaW9kaWMg Zmx1c2hpbmcgb2YgY2FudmFzIGRyYXdpbmcgY29udGV4dAorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzA2NDYKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBObyBuZXcgdGVzdHMuIE5vIGN1cnJlbnQgd2F5IHRv IHRyYWNrIHRlc3RzIHRoYXQgY2F1c2UgaGFuZ3Mgb3IKKyAgICAgICAgbm9uLWRldGVybWluaXN0 aWMgZHJvcHMgaW4gcGVyZm9ybWFuY2UuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9j Zy9JbWFnZUJ1ZmZlckNHLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkltYWdlQnVmZmVyOjpJbWFn ZUJ1ZmZlcik6CisgICAgICAgIChXZWJDb3JlOjpJbWFnZUJ1ZmZlcjo6Y29udGV4dCk6IEZsdXNo ZXMgY29udGV4dCBpZiB3ZSdyZSBiZXlvbmQgZmx1c2ggaW50ZXJ2YWwuCisgICAgICAgIChXZWJD b3JlOjpJbWFnZUJ1ZmZlcjo6Y29weU5hdGl2ZUltYWdlKTogVXBkYXRlcyBsYXN0IGZsdXNoIHRp bWVzdGFtcC4KKyAgICAgICAgKFdlYkNvcmU6OkltYWdlQnVmZmVyOjpnZXRVbm11bHRpcGxpZWRJ bWFnZURhdGEpOiBVcGRhdGVzIGxhc3QgZmx1c2ggdGltZXN0YW1wLgorICAgICAgICAoV2ViQ29y ZTo6SW1hZ2VCdWZmZXI6OmdldFByZW11bHRpcGxpZWRJbWFnZURhdGEpOiBVcGRhdGVzIGxhc3Qg Zmx1c2ggdGltZXN0YW1wLgorICAgICAgICAoV2ViQ29yZTo6SW1hZ2VCdWZmZXI6OnB1dFVubXVs dGlwbGllZEltYWdlRGF0YSk6IFVwZGF0ZXMgbGFzdCBmbHVzaCB0aW1lc3RhbXAuCisgICAgICAg IChXZWJDb3JlOjpJbWFnZUJ1ZmZlcjo6cHV0UHJlbXVsdGlwbGllZEltYWdlRGF0YSk6IFVwZGF0 ZXMgbGFzdCBmbHVzaCB0aW1lc3RhbXAuCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvY2cv SW1hZ2VCdWZmZXJEYXRhQ0cuY3BwOgorICAgICAgICAoV2ViQ29yZTo6SW1hZ2VCdWZmZXJEYXRh OjpJbWFnZUJ1ZmZlckRhdGEpOgorICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2NnL0ltYWdl QnVmZmVyRGF0YUNHLmg6IEFkZHMgYSB0aW1lc3RhbXAgb2YgbGFzdCB0cmFja2VkIGZsdXNoLgor CiAyMDExLTEwLTIxICBBZGFtIEJhcnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+CiAKICAgICAgICAg RXZlbnQuaCBoYXMgdG9vIG1hbnkgdmlydHVhbCBpc011bWJsZUV2ZW50KCkgZnVuY3Rpb25zCklu ZGV4OiBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jZy9JbWFnZUJ1ZmZlckNHLmNw cAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jZy9JbWFn ZUJ1ZmZlckNHLmNwcAkocmV2aXNpb24gOTgwMTApCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9ncmFwaGljcy9jZy9JbWFnZUJ1ZmZlckNHLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMzgsNiAr MzgsNyBAQAogI2luY2x1ZGUgPG1hdGguaD4KICNpbmNsdWRlIDx3dGYvQXNzZXJ0aW9ucy5oPgog I2luY2x1ZGUgPHd0Zi9DaGVja2VkQXJpdGhtZXRpYy5oPgorI2luY2x1ZGUgPHd0Zi9DdXJyZW50 VGltZS5oPgogI2luY2x1ZGUgPHd0Zi9NYWluVGhyZWFkLmg+CiAjaW5jbHVkZSA8d3RmL093bkFy cmF5UHRyLmg+CiAjaW5jbHVkZSA8d3RmL1JldGFpblB0ci5oPgpAQCAtMTY0LDcgKzE2NSw3IEBA IEltYWdlQnVmZmVyOjpJbWFnZUJ1ZmZlcihjb25zdCBJbnRTaXplJiAKICAgICBpZiAoIWNnQ29u dGV4dCkKICAgICAgICAgcmV0dXJuOwogCi0gICAgbV9jb250ZXh0PSBhZG9wdFB0cihuZXcgR3Jh cGhpY3NDb250ZXh0KGNnQ29udGV4dC5nZXQoKSkpOworICAgIG1fY29udGV4dCA9IGFkb3B0UHRy KG5ldyBHcmFwaGljc0NvbnRleHQoY2dDb250ZXh0LmdldCgpKSk7CiAgICAgbV9jb250ZXh0LT5z Y2FsZShGbG9hdFNpemUoMSwgLTEpKTsKICAgICBtX2NvbnRleHQtPnRyYW5zbGF0ZSgwLCAtaGVp Z2h0LnVuc2FmZUdldCgpKTsKICAgICBtX2NvbnRleHQtPnNldElzQWNjZWxlcmF0ZWRDb250ZXh0 KGFjY2VsZXJhdGVSZW5kZXJpbmcpOwpAQCAtMTgyLDYgKzE4MywxOCBAQCBzaXplX3QgSW1hZ2VC dWZmZXI6OmRhdGFTaXplKCkgY29uc3QKIAogR3JhcGhpY3NDb250ZXh0KiBJbWFnZUJ1ZmZlcjo6 Y29udGV4dCgpIGNvbnN0CiB7CisgICAgLy8gRm9yY2UgYSBmbHVzaCBpZiBsYXN0IGZsdXNoIHdh cyBtb3JlIHRoYW4gMjBtcyBhZ28KKyAgICBpZiAobV9jb250ZXh0LT5pc0FjY2VsZXJhdGVkQ29u dGV4dCgpKSB7CisgICAgICAgIGRvdWJsZSBlbGFwc2VkVGltZSA9IGN1cnJlbnRUaW1lTVMoKSAt IG1fZGF0YS5tX2xhc3RGbHVzaFRpbWU7CisgICAgICAgIGRvdWJsZSBtYXhGbHVzaEludGVydmFs ID0gMjA7IC8vIGluIG1zCisKKyAgICAgICAgaWYgKGVsYXBzZWRUaW1lID4gbWF4Rmx1c2hJbnRl cnZhbCkgeworICAgICAgICAgICAgQ0dDb250ZXh0UmVmIGNvbnRleHQgPSBtX2NvbnRleHQtPnBs YXRmb3JtQ29udGV4dCgpOworICAgICAgICAgICAgQ0dDb250ZXh0Rmx1c2goY29udGV4dCk7Cisg ICAgICAgICAgICBtX2RhdGEubV9sYXN0Rmx1c2hUaW1lID0gY3VycmVudFRpbWVNUygpOworICAg ICAgICB9CisgICAgfQorCiAgICAgcmV0dXJuIG1fY29udGV4dC5nZXQoKTsKIH0KIApAQCAtMjEy LDggKzIyNSwxMCBAQCBOYXRpdmVJbWFnZVB0ciBJbWFnZUJ1ZmZlcjo6Y29weU5hdGl2ZUltCiAg ICAgICAgIH0KICAgICB9CiAjaWYgVVNFKElPU1VSRkFDRV9DQU5WQVNfQkFDS0lOR19TVE9SRSkK LSAgICBlbHNlCisgICAgZWxzZSB7CiAgICAgICAgIGltYWdlID0gd2tJT1N1cmZhY2VDb250ZXh0 Q3JlYXRlSW1hZ2UoY29udGV4dCgpLT5wbGF0Zm9ybUNvbnRleHQoKSk7CisgICAgICAgIG1fZGF0 YS5tX2xhc3RGbHVzaFRpbWUgPSBjdXJyZW50VGltZU1TKCk7CisgICAgfQogI2VuZGlmCiAKICAg ICByZXR1cm4gaW1hZ2U7CkBAIC0yNjIsMjkgKzI3NywzNyBAQCB2b2lkIEltYWdlQnVmZmVyOjpj bGlwKEdyYXBoaWNzQ29udGV4dCogCiAKIFBhc3NSZWZQdHI8Qnl0ZUFycmF5PiBJbWFnZUJ1ZmZl cjo6Z2V0VW5tdWx0aXBsaWVkSW1hZ2VEYXRhKGNvbnN0IEludFJlY3QmIHJlY3QpIGNvbnN0CiB7 Ci0gICAgaWYgKG1fY29udGV4dC0+aXNBY2NlbGVyYXRlZENvbnRleHQoKSkKKyAgICBpZiAobV9j b250ZXh0LT5pc0FjY2VsZXJhdGVkQ29udGV4dCgpKSB7CiAgICAgICAgIENHQ29udGV4dEZsdXNo KGNvbnRleHQoKS0+cGxhdGZvcm1Db250ZXh0KCkpOworICAgICAgICBtX2RhdGEubV9sYXN0Rmx1 c2hUaW1lID0gY3VycmVudFRpbWVNUygpOworICAgIH0KICAgICByZXR1cm4gbV9kYXRhLmdldERh dGEocmVjdCwgbV9zaXplLCBtX2NvbnRleHQtPmlzQWNjZWxlcmF0ZWRDb250ZXh0KCksIHRydWUp OwogfQogCiBQYXNzUmVmUHRyPEJ5dGVBcnJheT4gSW1hZ2VCdWZmZXI6OmdldFByZW11bHRpcGxp ZWRJbWFnZURhdGEoY29uc3QgSW50UmVjdCYgcmVjdCkgY29uc3QKIHsKLSAgICBpZiAobV9jb250 ZXh0LT5pc0FjY2VsZXJhdGVkQ29udGV4dCgpKQorICAgIGlmIChtX2NvbnRleHQtPmlzQWNjZWxl cmF0ZWRDb250ZXh0KCkpIHsKICAgICAgICAgQ0dDb250ZXh0Rmx1c2goY29udGV4dCgpLT5wbGF0 Zm9ybUNvbnRleHQoKSk7CisgICAgICAgIG1fZGF0YS5tX2xhc3RGbHVzaFRpbWUgPSBjdXJyZW50 VGltZU1TKCk7CisgICAgfQogICAgIHJldHVybiBtX2RhdGEuZ2V0RGF0YShyZWN0LCBtX3NpemUs IG1fY29udGV4dC0+aXNBY2NlbGVyYXRlZENvbnRleHQoKSwgZmFsc2UpOwogfQogCiB2b2lkIElt YWdlQnVmZmVyOjpwdXRVbm11bHRpcGxpZWRJbWFnZURhdGEoQnl0ZUFycmF5KiBzb3VyY2UsIGNv bnN0IEludFNpemUmIHNvdXJjZVNpemUsIGNvbnN0IEludFJlY3QmIHNvdXJjZVJlY3QsIGNvbnN0 IEludFBvaW50JiBkZXN0UG9pbnQpCiB7Ci0gICAgaWYgKG1fY29udGV4dC0+aXNBY2NlbGVyYXRl ZENvbnRleHQoKSkKKyAgICBpZiAobV9jb250ZXh0LT5pc0FjY2VsZXJhdGVkQ29udGV4dCgpKSB7 CiAgICAgICAgIENHQ29udGV4dEZsdXNoKGNvbnRleHQoKS0+cGxhdGZvcm1Db250ZXh0KCkpOwor ICAgICAgICBtX2RhdGEubV9sYXN0Rmx1c2hUaW1lID0gY3VycmVudFRpbWVNUygpOworICAgIH0K ICAgICBtX2RhdGEucHV0RGF0YShzb3VyY2UsIHNvdXJjZVNpemUsIHNvdXJjZVJlY3QsIGRlc3RQ b2ludCwgbV9zaXplLCBtX2NvbnRleHQtPmlzQWNjZWxlcmF0ZWRDb250ZXh0KCksIHRydWUpOwog fQogCiB2b2lkIEltYWdlQnVmZmVyOjpwdXRQcmVtdWx0aXBsaWVkSW1hZ2VEYXRhKEJ5dGVBcnJh eSogc291cmNlLCBjb25zdCBJbnRTaXplJiBzb3VyY2VTaXplLCBjb25zdCBJbnRSZWN0JiBzb3Vy Y2VSZWN0LCBjb25zdCBJbnRQb2ludCYgZGVzdFBvaW50KQogewotICAgIGlmIChtX2NvbnRleHQt PmlzQWNjZWxlcmF0ZWRDb250ZXh0KCkpCisgICAgaWYgKG1fY29udGV4dC0+aXNBY2NlbGVyYXRl ZENvbnRleHQoKSkgewogICAgICAgICBDR0NvbnRleHRGbHVzaChjb250ZXh0KCktPnBsYXRmb3Jt Q29udGV4dCgpKTsKKyAgICAgICAgbV9kYXRhLm1fbGFzdEZsdXNoVGltZSA9IGN1cnJlbnRUaW1l TVMoKTsKKyAgICB9CiAgICAgbV9kYXRhLnB1dERhdGEoc291cmNlLCBzb3VyY2VTaXplLCBzb3Vy Y2VSZWN0LCBkZXN0UG9pbnQsIG1fc2l6ZSwgbV9jb250ZXh0LT5pc0FjY2VsZXJhdGVkQ29udGV4 dCgpLCBmYWxzZSk7CiB9CiAKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNz L2NnL0ltYWdlQnVmZmVyRGF0YUNHLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9ncmFwaGljcy9jZy9JbWFnZUJ1ZmZlckRhdGFDRy5jcHAJKHJldmlzaW9uIDk4MDEw KQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2cvSW1hZ2VCdWZmZXJEYXRh Q0cuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01Myw2ICs1Myw3IEBAIEltYWdlQnVmZmVyRGF0YTo6 SW1hZ2VCdWZmZXJEYXRhKGNvbnN0IEkKIDogbV9kYXRhKDApCiAjaWYgVVNFKElPU1VSRkFDRV9D QU5WQVNfQkFDS0lOR19TVE9SRSkKICwgbV9zdXJmYWNlKDApCissIG1fbGFzdEZsdXNoVGltZSgw KQogI2VuZGlmCiB7CiB9CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9j Zy9JbWFnZUJ1ZmZlckRhdGFDRy5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL2dyYXBoaWNzL2NnL0ltYWdlQnVmZmVyRGF0YUNHLmgJKHJldmlzaW9uIDk4MDEwKQorKysg U291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2cvSW1hZ2VCdWZmZXJEYXRhQ0cuaAko d29ya2luZyBjb3B5KQpAQCAtNTQsNiArNTQsNyBAQCBwdWJsaWM6CiAgICAgQ2hlY2tlZDx1bnNp Z25lZCwgUmVjb3JkT3ZlcmZsb3c+IG1fYnl0ZXNQZXJSb3c7CiAgICAgQ0dDb2xvclNwYWNlUmVm IG1fY29sb3JTcGFjZTsKICAgICBSZXRhaW5QdHI8SU9TdXJmYWNlUmVmPiBtX3N1cmZhY2U7Cisg ICAgbXV0YWJsZSBkb3VibGUgbV9sYXN0Rmx1c2hUaW1lOwogCiAgICAgUGFzc1JlZlB0cjxCeXRl QXJyYXk+IGdldERhdGEoY29uc3QgSW50UmVjdCYgcmVjdCwgY29uc3QgSW50U2l6ZSYgc2l6ZSwg Ym9vbCBhY2NlbGVyYXRlUmVuZGVyaW5nLCBib29sIHVubXVsdGlwbGllZCkgY29uc3Q7CiAgICAg dm9pZCBwdXREYXRhKEJ5dGVBcnJheSomIHNvdXJjZSwgY29uc3QgSW50U2l6ZSYgc291cmNlU2l6 ZSwgY29uc3QgSW50UmVjdCYgc291cmNlUmVjdCwgY29uc3QgSW50UG9pbnQmIGRlc3RQb2ludCwg Y29uc3QgSW50U2l6ZSYgc2l6ZSwgYm9vbCBhY2NlbGVyYXRlUmVuZGVyaW5nLCBib29sIHVubXVs dGlwbGllZCk7Cg==