83295 2012-04-05 11:09:00 -0700 [chromium] Drawing an accelerated canvas onto itself is slow. 2012-04-05 13:44:24 -0700 1 1 1 Unclassified WebKit Canvas 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 senorblanco senorblanco jamesr kbr oldest_to_newest 596718 0 senorblanco 2012-04-05 11:09:00 -0700 [chromium] Drawing an accelerated canvas onto itself is slow. 596721 1 135860 senorblanco 2012-04-05 11:11:57 -0700 Created attachment 135860 Test case from http://code.google.com/p/chromium/issues/detail?id=106646 596722 2 135861 senorblanco 2012-04-05 11:15:43 -0700 Created attachment 135861 Patch 596734 3 135861 jamesr 2012-04-05 11:21:57 -0700 Comment on attachment 135861 Patch Interesting. What if this is a gpu->cpu or cpu->gpu copy? Should we be checking the return value? 596807 4 senorblanco 2012-04-05 12:36:35 -0700 (In reply to comment #3) > (From update of attachment 135861 [details]) > Interesting. What if this is a gpu->cpu or cpu->gpu copy? Should we be checking the return value? Yeah, thanks for reminding me. Mike made me remove the copyTo() fallback from the internal implementation. 596823 5 senorblanco 2012-04-05 12:58:49 -0700 Note that it should always be GPU->GPU or CPU->CPU, since it's the same canvas. 596824 6 135881 senorblanco 2012-04-05 13:00:13 -0700 Created attachment 135881 Patch 596857 7 135881 jamesr 2012-04-05 13:32:46 -0700 Comment on attachment 135881 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135881&action=review > Source/WebCore/platform/graphics/skia/ImageSkia.cpp:483 > + if (!bitmap.deepCopyTo(&temp, bitmap.config())) > + bitmap.copyTo(&temp, bitmap.config()); gotcha. so given that the configs match, when would deepCopyTo fail? 596871 8 senorblanco 2012-04-05 13:40:57 -0700 (In reply to comment #7) > (From update of attachment 135881 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135881&action=review > > > Source/WebCore/platform/graphics/skia/ImageSkia.cpp:483 > > + if (!bitmap.deepCopyTo(&temp, bitmap.config())) > > + bitmap.copyTo(&temp, bitmap.config()); > > gotcha. so given that the configs match, when would deepCopyTo fail? Actually, it turns out that there is an internal copyTo fallback in the software case; it's only the texture-backed case that can fail. If the target has a texture which isn't a renderable format (eg., compressed texture), deepCopy() would fail, since the copy works by rendering via FBO. But given that this texture is a backing store, it should always succeed. So, never in this case. :) 596876 9 senorblanco 2012-04-05 13:44:24 -0700 Committed r113373: <http://trac.webkit.org/changeset/113373> 135860 2012-04-05 11:11:57 -0700 2012-04-05 11:11:57 -0700 Test case large-move-test.html text/html 2037 senorblanco PGh0bWw+CgogICAgPGhlYWQ+PHRpdGxlPkxhcmdlIE1vdmUgVGVzdDwvdGl0bGU+PC9oZWFkPgoK ICAgIDxib2R5PgoKICAgICAgICA8IS0tIFJlc3VsdHMgZGlzcGxheSAtLT4KICAgICAgICA8aDEg aWQ9InJlc3VsdHMiPihyZXN1bHRzIHBlbmRpbmcpPC9oMT4KCiAgICAgICAgPCEtLSBUZXN0IGNh bnZhcyAtLT4KICAgICAgICA8Y2FudmFzIGlkPSJjYW52YXMiIHdpZHRoPSIxMDI0IiBoZWlnaHQ9 Ijc2OCIKICAgICAgICAgICAgc3R5bGU9ImJvcmRlcjogMXB4IHNvbGlkIGJsYWNrIj48L2NhbnZh cz4KCiAgICAgICAgPHNjcmlwdCB0eXBlPSJ0ZXh0L2phdmFzY3JpcHQiPgoKICAgICAgICAgICAg Ly8gR2V0IGVsZW1lbnRzCiAgICAgICAgICAgIHZhciByZXN1bHRzID0gZG9jdW1lbnQuZ2V0RWxl bWVudEJ5SWQoInJlc3VsdHMiKTsKICAgICAgICAgICAgdmFyIGNhbnZhcyAgPSBkb2N1bWVudC5n ZXRFbGVtZW50QnlJZCgiY2FudmFzIik7CgogICAgICAgICAgICAvLyBHZXQgY29udGV4dAogICAg ICAgICAgICB2YXIgY3R4ID0gY2FudmFzLmdldENvbnRleHQoIjJkIik7CgogICAgICAgICAgICAv LyBEcmF3IGFyYml0cmFyeSByZWN0YW5nbGUgKHdoaWNoIHdlIHdpbGwgbW92ZSkKICAgICAgICAg ICAgY3R4LmJlZ2luUGF0aCgpOwogICAgICAgICAgICBjdHgucmVjdCgwLCAxMDAsIDY0MCw0ODAp OwogICAgICAgICAgICBjdHguY2xvc2VQYXRoKCk7CiAgICAgICAgICAgIGN0eC5maWxsU3R5bGUg PSAiI0FCQ0RFRiI7CiAgICAgICAgICAgIGN0eC5maWxsKCk7CgogICAgICAgICAgICAvLyBHZXQg c3RhcnQgdGltZQogICAgICAgICAgICB2YXIgc3RhcnQgPSBuZXcgRGF0ZSgpLmdldFRpbWUoKTsK CiAgICAgICAgICAgIC8vIE1vdmUgcmVjdGFuZ2xlIHVzaW5nIGRyYXdJbWFnZSgpIGNvcGllcywg b25lIHBpeGVsIGF0IGEgdGltZS4KICAgICAgICAgICAgLy8gU291cmNlIGFuZCBkZXN0aW5hdGlv biByZWdpb24gb3ZlcmxhcC4KICAgICAgICAgICAgLy8gVHJhaWwgaXMgZHJhd24gaW4gYmxhY2su CiAgICAgICAgICAgIGZvciAodmFyIGk9MDsgaTwyNTY7IGkrKykgewoKICAgICAgICAgICAgICAg IC8vIENvcHkgcmVjdGFuZ2xlIGZyb20gcHJldmlvdXMgbG9jYXRpb24gdG8gbmV3IGxvY2F0aW9u CiAgICAgICAgICAgICAgICBjdHguZHJhd0ltYWdlKGNhbnZhcywKICAgICAgICAgICAgICAgICAg ICAgICAgaSwgICAxMDAsIDY0MCwgNDgwLAogICAgICAgICAgICAgICAgICAgICAgICBpKzEsIDEw MCwgNjQwLCA0ODApOwoKICAgICAgICAgICAgICAgIC8vIERyYXcgYmxhY2sgdHJhaWwKICAgICAg ICAgICAgICAgIGN0eC5iZWdpblBhdGgoKTsKICAgICAgICAgICAgICAgIGN0eC5yZWN0KGksIDEw MCwgMSwgNDgwKTsKICAgICAgICAgICAgICAgIGN0eC5jbG9zZVBhdGgoKTsKICAgICAgICAgICAg ICAgIGN0eC5maWxsU3R5bGUgPSAiIzAwMDAwMCI7CiAgICAgICAgICAgICAgICBjdHguZmlsbCgp OwoKICAgICAgICAgICAgfQoKICAgICAgICAgICAgLy8gR2V0IGVuZCB0aW1lIGFuZCBkdXJhdGlv bgogICAgICAgICAgICB2YXIgZW5kID0gbmV3IERhdGUoKS5nZXRUaW1lKCk7CiAgICAgICAgICAg IHZhciBkdXJhdGlvbiA9IGVuZCAtIHN0YXJ0OwoKICAgICAgICAgICAgLy8gRGlzcGxheSByZXN1 bHRzCiAgICAgICAgICAgIHJlc3VsdHMudGV4dENvbnRlbnQgPQogICAgICAgICAgICAgICAgZHVy YXRpb24gKyAibXMgKCIgKyAoZHVyYXRpb24vMjU2KSArICJtcyBwZXIgZnJhbWUpIjsKCiAgICAg ICAgICAgIC8qCiAgICAgICAgICAgICAqIFJlc3VsdHMgb2Ygb3duIHRlc3QgcnVuczoKICAgICAg ICAgICAgICoKICAgICAgICAgICAgICogSGFyZHdhcmUgYWNjZWxlcmF0aW9uIE9GRjogMTMyOW1z ICg1LjE5MTQwNjI1bXMgcGVyIGZyYW1lKQogICAgICAgICAgICAgKiBIYXJkd2FyZSBhY2NlbGVy YXRpb24gT046ICAxMzI5bXMgKDUuMTkxNDA2MjVtcyBwZXIgZnJhbWUpCiAgICAgICAgICAgICAq LwoKICAgICAgICA8L3NjcmlwdD4KCiAgICA8L2JvZHk+CjwvaHRtbD4K 135861 2012-04-05 11:15:43 -0700 2012-04-05 13:00:09 -0700 Patch bug-83295-20120405141542.patch text/plain 1483 senorblanco SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDExMzMzOCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA0LTA1ICBTdGVwaGVu IFdoaXRlICA8c2Vub3JibGFuY29AY2hyb21pdW0ub3JnPgorCisgICAgICAgIFtjaHJvbWl1bV0g RHJhd2luZyBhbiBhY2NlbGVyYXRlZCBjYW52YXMgb250byBpdHNlbGYgaXMgc2xvdy4KKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTgzMjk1CisKKyAgICAg ICAgRml4ZWQgYnkgdXNpbmcgYSBHUFUtc2lkZSBkZWVwQ29weSgpIGluIHNraWEsIHNvIHdlIGRv bid0IHRyaWdnZXIgYQorICAgICAgICByZWFkYmFjayBhbmQgcmUtdXBsb2FkLgorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIENvcnJlY3RuZXNzIGlzIGNv dmVyZWQgYnkgZXhpc3RpbmcgdGVzdHMuIFBlcmZvcm1hbmNlIGlzIGNvdmVyZWQKKyAgICAgICAg YnkgdGhlIHRlc3QgY2FzZSBhdHRhY2hlZCB0byB0aGUgYnVnLgorCisgICAgICAgICogcGxhdGZv cm0vZ3JhcGhpY3Mvc2tpYS9JbWFnZVNraWEuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Qml0bWFw SW1hZ2VTaW5nbGVGcmFtZVNraWE6OmNyZWF0ZSk6CisKIDIwMTItMDQtMDUgIE1pbiBRaW4gIDxx aW5taW5AZ29vZ2xlLmNvbT4KIAogICAgICAgICBGaXggZnVsbHNjcmVlbiB2aWRlbyBidXR0b24g Zm9yIGFuZHJvaWQKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEv SW1hZ2VTa2lhLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9za2lhL0ltYWdlU2tpYS5jcHAJKHJldmlzaW9uIDExMzMzOCkKKysrIFNvdXJjZS9XZWJD b3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvSW1hZ2VTa2lhLmNwcAkod29ya2luZyBjb3B5KQpA QCAtNDc5LDcgKzQ3OSw3IEBAIFBhc3NSZWZQdHI8Qml0bWFwSW1hZ2VTaW5nbGVGcmFtZVNraWE+ IEIKIHsKICAgICBpZiAoY29weVBpeGVscykgewogICAgICAgICBTa0JpdG1hcCB0ZW1wOwotICAg ICAgICBiaXRtYXAuY29weVRvKCZ0ZW1wLCBiaXRtYXAuY29uZmlnKCkpOworICAgICAgICBiaXRt YXAuZGVlcENvcHlUbygmdGVtcCwgYml0bWFwLmNvbmZpZygpKTsKICAgICAgICAgcmV0dXJuIGFk b3B0UmVmKG5ldyBCaXRtYXBJbWFnZVNpbmdsZUZyYW1lU2tpYSh0ZW1wKSk7CiAgICAgfQogICAg IHJldHVybiBhZG9wdFJlZihuZXcgQml0bWFwSW1hZ2VTaW5nbGVGcmFtZVNraWEoYml0bWFwKSk7 Cg== 135881 2012-04-05 13:00:13 -0700 2012-04-05 13:32:45 -0700 Patch bug-83295-20120405160012.patch text/plain 1540 senorblanco SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDExMzMzOCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA0LTA1ICBTdGVwaGVu IFdoaXRlICA8c2Vub3JibGFuY29AY2hyb21pdW0ub3JnPgorCisgICAgICAgIFtjaHJvbWl1bV0g RHJhd2luZyBhbiBhY2NlbGVyYXRlZCBjYW52YXMgb250byBpdHNlbGYgaXMgc2xvdy4KKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTgzMjk1CisKKyAgICAg ICAgRml4ZWQgYnkgdXNpbmcgYSBHUFUtc2lkZSBkZWVwQ29weSgpIGluIHNraWEsIHNvIHdlIGRv bid0IHRyaWdnZXIgYQorICAgICAgICByZWFkYmFjayBhbmQgcmUtdXBsb2FkLgorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIENvcnJlY3RuZXNzIGlzIGNv dmVyZWQgYnkgZXhpc3RpbmcgdGVzdHMuIFBlcmZvcm1hbmNlIGlzIGNvdmVyZWQKKyAgICAgICAg YnkgdGhlIHRlc3QgY2FzZSBhdHRhY2hlZCB0byB0aGUgYnVnLgorCisgICAgICAgICogcGxhdGZv cm0vZ3JhcGhpY3Mvc2tpYS9JbWFnZVNraWEuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Qml0bWFw SW1hZ2VTaW5nbGVGcmFtZVNraWE6OmNyZWF0ZSk6CisKIDIwMTItMDQtMDUgIE1pbiBRaW4gIDxx aW5taW5AZ29vZ2xlLmNvbT4KIAogICAgICAgICBGaXggZnVsbHNjcmVlbiB2aWRlbyBidXR0b24g Zm9yIGFuZHJvaWQKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEv SW1hZ2VTa2lhLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9za2lhL0ltYWdlU2tpYS5jcHAJKHJldmlzaW9uIDExMzMzOCkKKysrIFNvdXJjZS9XZWJD b3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvSW1hZ2VTa2lhLmNwcAkod29ya2luZyBjb3B5KQpA QCAtNDc5LDcgKzQ3OSw4IEBAIFBhc3NSZWZQdHI8Qml0bWFwSW1hZ2VTaW5nbGVGcmFtZVNraWE+ IEIKIHsKICAgICBpZiAoY29weVBpeGVscykgewogICAgICAgICBTa0JpdG1hcCB0ZW1wOwotICAg ICAgICBiaXRtYXAuY29weVRvKCZ0ZW1wLCBiaXRtYXAuY29uZmlnKCkpOworICAgICAgICBpZiAo IWJpdG1hcC5kZWVwQ29weVRvKCZ0ZW1wLCBiaXRtYXAuY29uZmlnKCkpKQorICAgICAgICAgICAg Yml0bWFwLmNvcHlUbygmdGVtcCwgYml0bWFwLmNvbmZpZygpKTsKICAgICAgICAgcmV0dXJuIGFk b3B0UmVmKG5ldyBCaXRtYXBJbWFnZVNpbmdsZUZyYW1lU2tpYSh0ZW1wKSk7CiAgICAgfQogICAg IHJldHVybiBhZG9wdFJlZihuZXcgQml0bWFwSW1hZ2VTaW5nbGVGcmFtZVNraWEoYml0bWFwKSk7 Cg==