110003 2013-02-15 22:31:29 -0800 Printing WebGL canvases in Chrome uses stale data after first print 2013-02-20 17:36:47 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 bajones bajones dino esprehn+autocc kbr ojan.autocc webkit.review.bot oldest_to_newest 834635 0 bajones 2013-02-15 22:31:29 -0800 Printing WebGL canvases in Chrome uses stale data after first print 834636 1 188692 bajones 2013-02-15 22:33:13 -0800 Created attachment 188692 Patch 834642 2 188692 jbauman 2013-02-15 22:54:39 -0800 Comment on attachment 188692 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188692&action=review > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:743 > + canvas()->clearPresentationCopy(); I suppose one place this breaks down is when in the course of one javascript sequence you draw to the canvas, do window.print, do a readpixels from it, then do window.print again. Although that case might already be broken. This would also be broken if anyone ever used WebGL with compositing disabled, though that never actually happens. It might make more sense to do the canvas()->clearPresentationCopy() in WebGLRenderingContext::markContextChanged, and not in this method. 834764 3 bajones 2013-02-16 11:00:41 -0800 (In reply to comment #2) > (From update of attachment 188692 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=188692&action=review > > > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:743 > > + canvas()->clearPresentationCopy(); > > I suppose one place this breaks down is when in the course of one javascript sequence you draw to the canvas, do window.print, do a readpixels from it, then do window.print again. Although that case might already be broken. That's an interesting edge case. It would be good to know what the current behavior of this is, and if this patch changes it. > This would also be broken if anyone ever used WebGL with compositing disabled, though that never actually happens. Ken Russell and I talked about this, and we agree that it never happens in practice. We are discussing disabling WebGL context creation if accelerated compositing is off. > It might make more sense to do the canvas()->clearPresentationCopy() in WebGLRenderingContext::markContextChanged, and not in this method. paintRenderingResultsToCanvas is only called when printing, where a little extra overhead won't make much difference. markContextChanged is called frequently while drawing. Even though clearPresentationCopy is not an expensive call, I'd prefer not to add unnecessary calls to the main draw loop simply for the sake of printing. 836643 4 189136 kbr 2013-02-19 12:10:30 -0800 Created attachment 189136 Test case suggested by jbauman @jbauman: here's the test case you suggested. In both Chrome Canary and the current WebKit nightly, the readPixels results contain (0, 0, 0, 0) after the window.print call, so clearly that's causing a re-composite and blowing away of the back buffer which isn't spec compliant. In Chrome, the second printing result contains red instead of green, although WebKit correctly contains green. Brandon, can you try this test case with your patch? If it fixes Chrome's stale printing result without regressing anything else, then I am in favor of getting this fix in and proceeding with the other bugs afterward. 836696 5 bajones 2013-02-19 12:57:43 -0800 (In reply to comment #4) > Created an attachment (id=189136) [details] > Test case suggested by jbauman > > @jbauman: here's the test case you suggested. In both Chrome Canary and the current WebKit nightly, the readPixels results contain (0, 0, 0, 0) after the window.print call, so clearly that's causing a re-composite and blowing away of the back buffer which isn't spec compliant. In Chrome, the second printing result contains red instead of green, although WebKit correctly contains green. > > Brandon, can you try this test case with your patch? If it fixes Chrome's stale printing result without regressing anything else, then I am in favor of getting this fix in and proceeding with the other bugs afterward. Just tested: The behavior of the test case is identical with or without the patch. 836754 6 188692 kbr 2013-02-19 14:00:27 -0800 Comment on attachment 188692 Patch Thanks for testing. r=me 836935 7 188692 webkit.review.bot 2013-02-19 16:20:29 -0800 Comment on attachment 188692 Patch Rejecting attachment 188692 from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 188692, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 13671 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 53>At revision 13671. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/16640023 837566 8 189337 bajones 2013-02-20 10:42:04 -0800 Created attachment 189337 Patch 837586 9 189337 webkit.review.bot 2013-02-20 11:14:47 -0800 Comment on attachment 189337 Patch Rejecting attachment 189337 from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 189337, '--port=chromium-xvfb']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: toinstalled/mechanize/_urllib2_fork.py", line 332, in _call_chain result = func(*args) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1170, in https_open return self.do_open(conn_factory, req) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1118, in do_open raise URLError(err) urllib2.URLError: <urlopen error [Errno 110] Connection timed out> Full output: http://queues.webkit.org/results/16659436 837997 10 189337 webkit.review.bot 2013-02-20 17:36:44 -0800 Comment on attachment 189337 Patch Clearing flags on attachment: 189337 Committed r143545: <http://trac.webkit.org/changeset/143545> 837998 11 webkit.review.bot 2013-02-20 17:36:47 -0800 All reviewed patches have been landed. Closing bug. 188692 2013-02-15 22:33:13 -0800 2013-02-20 10:42:03 -0800 Patch bug-110003-20130215222939.patch text/plain 1585 bajones U3VidmVyc2lvbiBSZXZpc2lvbjogMTQzMDQ1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNzkwZGUyMDhmMTQyYzE3 NzQzNzc1NzAwZDFmZGU3MTliMmZiNjFjNS4uZjRjMzFhOGFjM2U1YzAzM2EyMmVlMDQ1ODdkYmFl YWViZTY0OWNhYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEzLTAyLTE1ICBCcmFu ZG9uIEpvbmVzICA8YmFqb25lc0Bnb29nbGUuY29tPgorCisgICAgICAgIFByaW50aW5nIFdlYkdM IGNhbnZhc2VzIGluIENocm9tZSB1c2VzIHN0YWxlIGRhdGEgYWZ0ZXIgZmlyc3QgcHJpbnQKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExMDAwMworCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE5vIG5ldyB0ZXN0 cyAoT09QUyEpLgorCisgICAgICAgICogaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdDb250ZXh0 LmNwcDoKKyAgICAgICAgKFdlYkNvcmUpOgorICAgICAgICAoV2ViQ29yZTo6V2ViR0xSZW5kZXJp bmdDb250ZXh0OjpwYWludFJlbmRlcmluZ1Jlc3VsdHNUb0NhbnZhcyk6CisKIDIwMTMtMDItMTQg IEVtaWwgQSBFa2x1bmQgIDxlYWVAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIENsYW1wIHNwYW4g dmFsdWUgaW4gUmVuZGVyVGFibGVDZWxsOjpwYXJzZVtDb2x8Um93XVNwYW5Gcm9tRE9NCmRpZmYg LS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQu Y3BwIGIvU291cmNlL1dlYkNvcmUvaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdDb250ZXh0LmNw cAppbmRleCBkN2E2YzJkNDhlZWFhMGUxOGQyYzNjNGFlNjZlMWM2MmRjMDU4MDRlLi4zZjU5OGMw Njk5ODFhNGU3MmY2ODA3NTI4MGZiNTY5ODYxYzM3NGNhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2Vi Q29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQuY3BwCisrKyBiL1NvdXJjZS9X ZWJDb3JlL2h0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5jcHAKQEAgLTczOSw2ICs3 MzksOSBAQCB2b2lkIFdlYkdMUmVuZGVyaW5nQ29udGV4dDo6bWFya0xheWVyQ29tcG9zaXRlZCgp CiAKIHZvaWQgV2ViR0xSZW5kZXJpbmdDb250ZXh0OjpwYWludFJlbmRlcmluZ1Jlc3VsdHNUb0Nh bnZhcygpCiB7CisgICAgaWYgKGNhbnZhcygpLT5kb2N1bWVudCgpLT5wcmludGluZygpKQorICAg ICAgICBjYW52YXMoKS0+Y2xlYXJQcmVzZW50YXRpb25Db3B5KCk7CisKICAgICAvLyBVbnRpbCB0 aGUgY2FudmFzIGlzIHdyaXR0ZW4gdG8gYnkgdGhlIGFwcGxpY2F0aW9uLCB0aGUgY2xlYXIgdGhh dAogICAgIC8vIGhhcHBlbmVkIGFmdGVyIGl0IHdhcyBjb21wb3NpdGVkIHNob3VsZCBiZSBpZ25v cmVkIGJ5IHRoZSBjb21wb3NpdG9yLgogICAgIGlmIChtX2NvbnRleHQtPmxheWVyQ29tcG9zaXRl ZCgpICYmICFtX2F0dHJpYnV0ZXMucHJlc2VydmVEcmF3aW5nQnVmZmVyKSB7Cg== 189136 2013-02-19 12:10:30 -0800 2013-02-19 12:10:30 -0800 Test case suggested by jbauman printing-test.html text/html 1296 kbr PGhlYWQ+CjxzY3JpcHQgbGFuZ3VhZ2U9ImphdmFzY3JpcHQiPgpmdW5jdGlvbiBvdXRwdXQoc3Ry KSB7CiAgZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoImNvbnNvbGUiKS5pbm5lckhUTUwgKz0gc3Ry ICsgIjxicj4iOwp9CgpmdW5jdGlvbiBleHBlY3RQaXhlbChwaXhlbHMsIGNvbG9yKSB7CiAgaWYg KHBpeGVsc1swXSA9PSBjb2xvclswXSAmJiBwaXhlbHNbMV0gPT0gY29sb3JbMV0gJiYgcGl4ZWxz WzJdID09IGNvbG9yWzJdICYmIHBpeGVsc1szXSA9PSBjb2xvclszXSkgewogICAgb3V0cHV0KCJw aXhlbCBhdCAoMCwwKSB3YXMgIiArIEpTT04uc3RyaW5naWZ5KGNvbG9yKSArICJhcyBleHBlY3Rl ZCIpOwogIH0gZWxzZSB7CiAgICBvdXRwdXQoInBpeGVsIGF0ICgwLDApIHdhcyBbIiArIHBpeGVs c1swXSArICIsICIgKyBwaXhlbHNbMV0gKyAiLCAiICsgcGl4ZWxzWzJdICsgIiwgIiArIHBpeGVs c1szXSArICJdLCBleHBlY3RlZCAiICsgSlNPTi5zdHJpbmdpZnkoY29sb3IpKTsKICB9Cn0KCmZ1 bmN0aW9uIHByaW50UmVuZGVyQW5kUHJpbnQoKSB7CiAgdmFyIGdsID0gZG9jdW1lbnQuZ2V0RWxl bWVudEJ5SWQoImNhbnZhcyIpLmdldENvbnRleHQoImV4cGVyaW1lbnRhbC13ZWJnbCIpOwogIGds LmNsZWFyQ29sb3IoMS4wLCAwLjAsIDAuMCwgMS4wKTsKICBnbC5jbGVhcihnbC5DT0xPUl9CVUZG RVJfQklUKTsKICB3aW5kb3cucHJpbnQoKTsKICB2YXIgd2lkdGggPSA2NCwgaGVpZ2h0ID0gNjQ7 CiAgdmFyIHBpeGVscyA9IG5ldyBVaW50OEFycmF5KHdpZHRoICogaGVpZ2h0ICogNCk7CiAgZ2wu cmVhZFBpeGVscygwLCAwLCB3aWR0aCwgaGVpZ2h0LCBnbC5SR0JBLCBnbC5VTlNJR05FRF9CWVRF LCBwaXhlbHMpOwogIGV4cGVjdFBpeGVsKHBpeGVscywgWzI1NSwgMCwgMCwgMjU1XSk7CiAgZ2wu Y2xlYXJDb2xvcigwLjAsIDEuMCwgMC4wLCAxLjApOwogIGdsLmNsZWFyKGdsLkNPTE9SX0JVRkZF Ul9CSVQpOwogIHdpbmRvdy5wcmludCgpOwogIGV4cGVjdFBpeGVsKHBpeGVscywgWzAsIDI1NSwg MCwgMjU1XSk7Cn0KCmZ1bmN0aW9uIHN0YXJ0KCkgewogIHZhciByYWYgPSB3aW5kb3cucmVxdWVz dEFuaW1hdGlvbkZyYW1lIHx8IHdpbmRvdy53ZWJraXRSZXF1ZXN0QW5pbWF0aW9uRnJhbWU7CiAg cmFmKHByaW50UmVuZGVyQW5kUHJpbnQpOwp9Cjwvc2NyaXB0Pgo8L2hlYWQ+Cjxib2R5IG9ubG9h ZD0ic3RhcnQoKSI+CjxjYW52YXMgaWQ9ImNhbnZhcyIgd2lkdGg9IjY0IiBoZWlnaHQ9IjY0Ij48 L2NhbnZhcz4KPGRpdiBpZD0iY29uc29sZSI+PC9kaXY+CjwvYm9keT4K 189337 2013-02-20 10:42:04 -0800 2013-02-20 17:36:44 -0800 Patch bug-110003-20130220103826.patch text/plain 1641 bajones U3VidmVyc2lvbiBSZXZpc2lvbjogMTQzNDcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMGUyYWQ2MDY4OWU0OGEx ZjkyOTg0ZDZlNzAxMzc2YTQ3NDJjOWVjMC4uYzMyODFmNTQ3Y2JkZWY4NmNmNjNkNWNiM2FlMGQx ZWM2MWE3ZWIxZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEzLTAyLTIwICBCcmFu ZG9uIEpvbmVzICA8YmFqb25lc0Bnb29nbGUuY29tPgorIAorICAgICAgICBQcmludGluZyBXZWJH TCBjYW52YXNlcyBpbiBDaHJvbWUgdXNlcyBzdGFsZSBkYXRhIGFmdGVyIGZpcnN0IHByaW50Cisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMTAwMDMKKyAK KyAgICAgICAgUmV2aWV3ZWQgYnkgS2VubmV0aCBSdXNzZWxsLgorIAorICAgICAgICBVbmFibGUg dG8gdmFsaWRhdGUgcHJpbnRlZCBjb250ZW50IGF1dG9tYXRpY2FsbHkuIE5vIHJlZ3Jlc3Npb24g aW4gZXhpc3RpbmcgV2ViR0wgdGVzdHMuCisgCisgICAgICAgICAqIGh0bWwvY2FudmFzL1dlYkdM UmVuZGVyaW5nQ29udGV4dC5jcHA6CisgICAgICAgICAoV2ViQ29yZSk6CisgICAgICAgICAoV2Vi Q29yZTo6V2ViR0xSZW5kZXJpbmdDb250ZXh0OjpwYWludFJlbmRlcmluZ1Jlc3VsdHNUb0NhbnZh cyk6CisKIDIwMTMtMDItMjAgIE1hcmsgTGFtICA8bWFyay5sYW1AYXBwbGUuY29tPgogCiAgICAg ICAgIFJlbmFtZSBEYXRhYmFzZUJhY2tlbmQgdG8gRGF0YWJhc2VCYWNrZW5kQmFzZS4KZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5j cHAgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQuY3Bw CmluZGV4IGQ3YTZjMmQ0OGVlYWEwZTE4ZDJjM2M0YWU2NmUxYzYyZGMwNTgwNGUuLjNmNTk4YzA2 OTk4MWE0ZTcyZjY4MDc1MjgwZmI1Njk4NjFjMzc0Y2EgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL2h0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5jcHAKKysrIGIvU291cmNlL1dl YkNvcmUvaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdDb250ZXh0LmNwcApAQCAtNzM5LDYgKzcz OSw5IEBAIHZvaWQgV2ViR0xSZW5kZXJpbmdDb250ZXh0OjptYXJrTGF5ZXJDb21wb3NpdGVkKCkK IAogdm9pZCBXZWJHTFJlbmRlcmluZ0NvbnRleHQ6OnBhaW50UmVuZGVyaW5nUmVzdWx0c1RvQ2Fu dmFzKCkKIHsKKyAgICBpZiAoY2FudmFzKCktPmRvY3VtZW50KCktPnByaW50aW5nKCkpCisgICAg ICAgIGNhbnZhcygpLT5jbGVhclByZXNlbnRhdGlvbkNvcHkoKTsKKwogICAgIC8vIFVudGlsIHRo ZSBjYW52YXMgaXMgd3JpdHRlbiB0byBieSB0aGUgYXBwbGljYXRpb24sIHRoZSBjbGVhciB0aGF0 CiAgICAgLy8gaGFwcGVuZWQgYWZ0ZXIgaXQgd2FzIGNvbXBvc2l0ZWQgc2hvdWxkIGJlIGlnbm9y ZWQgYnkgdGhlIGNvbXBvc2l0b3IuCiAgICAgaWYgKG1fY29udGV4dC0+bGF5ZXJDb21wb3NpdGVk KCkgJiYgIW1fYXR0cmlidXRlcy5wcmVzZXJ2ZURyYXdpbmdCdWZmZXIpIHsK