56414 2011-03-15 14:04:29 -0700 texImage2D gets old contents of canvas 2011-03-16 22:55:13 -0700 1 1 1 Unclassified WebKit WebGL 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 1 jbauman webkit-unassigned cmarrin commit-queue hyatt jamesr jbauman kbr senorblanco zmo oldest_to_newest 367934 0 jbauman 2011-03-15 14:04:29 -0700 Currently we don't seem to be marking the copied image of a canvas as dirty when we're doing accelerated rendering, causing a texImage2D on the canvas to possibly get old data. 368018 1 jbauman 2011-03-15 16:13:01 -0700 Looks like we're checking paintsIntoCanvasBuffer to see if we should do paintRenderingResultsToCanvas to update the image. This is true on chromium with accelerated compositing disabled (like in this test), so the paint doesn't happen. I'm not sure what exactly paintsIntoCanvasBuffer is supposed to mean, but removing the check for it in HTMLCanvasElement::copiedImage seems to work. 368058 2 kbr 2011-03-15 17:11:00 -0700 Do you happen to know whether it's broken when passing a 2D-rendered Canvas to WebGL via texImage2D, when passing a WebGL-rendered Canvas to WebGL via texImage2D, or both? For the WebGL case, https://bugs.webkit.org/show_bug.cgi?id=56431 will affect the behavior here, though we can fix this bug independently. 368064 3 jbauman 2011-03-15 17:16:51 -0700 I haven't tested reading from 2D-rendered Canvas, but I think that may be broken when the canvas is GPU-accelerated. An unaccelerated canvas should work fine. 368582 4 85986 jbauman 2011-03-16 15:23:37 -0700 Created attachment 85986 Patch 368590 5 85986 jamesr 2011-03-16 15:30:50 -0700 Comment on attachment 85986 Patch This seems correct to me. Should we have a more direct test for this? fast/canvas/webgl/premultiplyalpha-test.html seems to test a bunch of other stuff as well. 368591 6 jamesr 2011-03-16 15:31:23 -0700 It looks like this will never change behavior for the Mac port since paintsIntoCanvasBuffer() always returns false on mac for WebGL content. 368595 7 jbauman 2011-03-16 15:37:28 -0700 Right, the test already passes on Macs. The patch might change something on Qt, but hopefully it will change it to do the right thing. 368640 8 senorblanco 2011-03-16 16:48:09 -0700 Is it possible this fix would break (or slow down) accelerated canvas2D -> canvas2D draws? (the amazon shelf demo used to be a good test of this, but unfortunately it's not available anymore). This seems to end up here: bool GraphicsContext3DInternal::paintsIntoCanvasBuffer() const { // If the gpu compositor is on then skip the readback and software rendering path. return !m_webViewImpl->isAcceleratedCompositingActive(); } 368650 9 jbauman 2011-03-16 16:54:04 -0700 HTMLCanvasElement::paint still checks paintsIntoCanvasBuffer, so that won't be affected. canvas2D->canvas2D drawImages go down a different path (not using copiedImage, but instead manually doing makeRenderingResultsAvailable if the canvas isn't accelerated), so they shouldn't change. 368658 10 jamesr 2011-03-16 17:05:05 -0700 canvas2d->canvas2d on chromium/skia uses this: http://trac.webkit.org/browser/trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp#L1267 which does the readback if needed and then does this: http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/skia/ImageBufferSkia.cpp#L118 which should still do the right thing 368669 11 senorblanco 2011-03-16 17:20:45 -0700 (In reply to comment #10) > canvas2d->canvas2d on chromium/skia uses this: > http://trac.webkit.org/browser/trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp#L1267 > > which does the readback if needed and then does this: > http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/skia/ImageBufferSkia.cpp#L118 > > which should still do the right thing Right, thanks. This change makes more sense to me now. 368795 12 85986 commit-queue 2011-03-16 20:56:28 -0700 Comment on attachment 85986 Patch Clearing flags on attachment: 85986 Committed r81317: <http://trac.webkit.org/changeset/81317> 368796 13 commit-queue 2011-03-16 20:56:34 -0700 All reviewed patches have been landed. Closing bug. 368842 14 commit-queue 2011-03-16 22:55:13 -0700 The commit-queue encountered the following flaky tests while processing attachment 85986: fast/workers/storage/use-same-database-in-page-and-workers.html bug 50995 (author: dumi@chromium.org) The commit-queue is continuing to process your patch. 85986 2011-03-16 15:23:37 -0700 2011-03-16 20:56:28 -0700 Patch bug-56414-20110316152336.patch text/plain 3158 jbauman U3VidmVyc2lvbiBSZXZpc2lvbjogODEyMTMKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5n ZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwppbmRleCBlNzkxNzIxMTAyMDkyNmI1MWI4Njc2 NWQzNjgzMDIxN2M4YjcyZDlhLi4yZjg0NzVjMDEwYjE1NDg1OTNiOWEyNmU0YmQ0ZjEyYzk0ZTIx ZGNjIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMv Q2hhbmdlTG9nCkBAIC0xLDMgKzEsMTIgQEAKKzIwMTEtMDMtMTYgIEpvaG4gQmF1bWFuICA8amJh dW1hbkBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgdGV4SW1hZ2UyRCBnZXRzIG9sZCBjb250ZW50cyBvZiBjYW52YXMKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTU2NDE0CisKKyAgICAg ICAgKiBwbGF0Zm9ybS9jaHJvbWl1bS90ZXN0X2V4cGVjdGF0aW9ucy50eHQ6IHByZW11bHRpcGx5 LWFscGhhIHRlc3Qgbm93IHdvcmtzCisKIDIwMTEtMDMtMTUgIEpvaG4gQmF1bWFuICA8amJhdW1h bkBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgS2VubmV0aCBSdXNzZWxsLgpk aWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvcGxhdGZvcm0vY2hyb21pdW0vdGVzdF9leHBlY3RhdGlv bnMudHh0IGIvTGF5b3V0VGVzdHMvcGxhdGZvcm0vY2hyb21pdW0vdGVzdF9leHBlY3RhdGlvbnMu dHh0CmluZGV4IGFiZWQxZWYwYjI5NjI3NDAzMDZiMzc0MmFkYTU2ZGVhNjMxZDlkYWEuLmY5N2Qx MGEzNTMzNzFkNTk1YmYwNTZhYzg3NjA5ZGQwOTcwNzg5MTUgMTAwNzU1Ci0tLSBhL0xheW91dFRl c3RzL3BsYXRmb3JtL2Nocm9taXVtL3Rlc3RfZXhwZWN0YXRpb25zLnR4dAorKysgYi9MYXlvdXRU ZXN0cy9wbGF0Zm9ybS9jaHJvbWl1bS90ZXN0X2V4cGVjdGF0aW9ucy50eHQKQEAgLTIzNTMsOCAr MjM1Myw2IEBAIEJVR1dFQkdMIE1BQyBMSU5VWCA6IGZhc3QvY2FudmFzL3dlYmdsL2dsc2wtY29u Zm9ybWFuY2UuaHRtbCA9IENSQVNICiBCVUdXRUJHTCBHUFUgV0lOIERFQlVHIFNMT1cgOiBmYXN0 L2NhbnZhcy93ZWJnbC9jb250ZXh0LWF0dHJpYnV0ZXMtYWxwaGEtZGVwdGgtc3RlbmNpbC1hbnRp YWxpYXMuaHRtbCA9IFBBU1MKIEJVR1dFQkdMIEdQVSBXSU4gREVCVUcgU0xPVyA6IGZhc3QvY2Fu dmFzL3dlYmdsL2luZGV4LXZhbGlkYXRpb24td2l0aC1yZXNpemVkLWJ1ZmZlci5odG1sID0gUEFT UwogCi1CVUdXSzU2NDE0IDogZmFzdC9jYW52YXMvd2ViZ2wvcHJlbXVsdGlwbHlhbHBoYS10ZXN0 Lmh0bWwgPSBURVhUCi0KIC8vIFRoZXNlIHdlcmUgbm90IGZpeGVkIGJ5IHRoZSBNZXNhIDcuOSB1 cGdyYWRlLiBOZWVkIHRvIGludmVzdGlnYXRlLgogQlVHQ1I2MDY1MSBXSU4gOiBmYXN0L2NhbnZh cy93ZWJnbC9nbC1vYmplY3QtZ2V0LWNhbGxzLmh0bWwgPSBUSU1FT1VUIFBBU1MgVEVYVAogLy8g QlVHQ1I2MDY1MSBXSU4gOiBmYXN0L2NhbnZhcy93ZWJnbC9nbHNsLWNvbmZvcm1hbmNlLmh0bWwg PSBUSU1FT1VUIEZBSUwKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBjMDQ2MTFhY2ViOGU3NTc1NGZjNGExNzRmODJj NjExMzI4MGY3ZjUyLi42OTg5Nzk3MTIzMzgyOTM1MWQyNWY3MjQ0MmYzYTg4ODJmODU1MDEwIDEw MDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUv Q2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDMtMTYgIEpvaG4gQmF1bWFuICA8amJh dW1hbkBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgdGV4SW1hZ2UyRCBnZXRzIG9sZCBjb250ZW50cyBvZiBjYW52YXMKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTU2NDE0CisKKyAgICAg ICAgQWx3YXlzIHVwZGF0ZSB0aGUgY2FudmFzIGNvbnRlbnRzIGluIGNvcGllZEltYWdlLCBhcyB0 aGVyZSdzIG5vIHJlYXNvbgorICAgICAgICB0byBhc2sgZm9yIGFuIG91dC1vZi1kYXRlIGltYWdl LgorCisgICAgICAgICogaHRtbC9IVE1MQ2FudmFzRWxlbWVudC5jcHA6CisgICAgICAgIChXZWJD b3JlOjpIVE1MQ2FudmFzRWxlbWVudDo6Y29waWVkSW1hZ2UpOgorCiAyMDExLTAzLTE1ICBKb2hu IEJhdW1hbiAgPGpiYXVtYW5AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEtl bm5ldGggUnVzc2VsbC4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTENhbnZh c0VsZW1lbnQuY3BwIGIvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MQ2FudmFzRWxlbWVudC5jcHAK aW5kZXggNmQ0MWU1NTUxNzY2NTZkOWY0ZGVkMzZlZWMwNmY2YmFhNDhjNDk3NC4uNjQ2MWRiNmI0 YzViMjgyOTYyMDVlNzQ2MDViYTJhOTk1YWU5NzAwOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNv cmUvaHRtbC9IVE1MQ2FudmFzRWxlbWVudC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvaHRtbC9I VE1MQ2FudmFzRWxlbWVudC5jcHAKQEAgLTQ2MSwxMSArNDYxLDggQEAgSW1hZ2VCdWZmZXIqIEhU TUxDYW52YXNFbGVtZW50OjpidWZmZXIoKSBjb25zdAogSW1hZ2UqIEhUTUxDYW52YXNFbGVtZW50 Ojpjb3BpZWRJbWFnZSgpIGNvbnN0CiB7CiAgICAgaWYgKCFtX2NvcGllZEltYWdlICYmIGJ1ZmZl cigpKSB7Ci0gICAgICAgIGlmIChtX2NvbnRleHQpIHsKLSAgICAgICAgICAgIC8vIElmIHdlJ3Jl IG5vdCByZW5kZXJpbmcgdG8gdGhlIEltYWdlQnVmZmVyLCBjb3B5IHRoZSByZW5kZXJpbmcgcmVz dWx0cyB0byBpdC4KLSAgICAgICAgICAgIGlmICghbV9jb250ZXh0LT5wYWludHNJbnRvQ2FudmFz QnVmZmVyKCkpCi0gICAgICAgICAgICAgICAgbV9jb250ZXh0LT5wYWludFJlbmRlcmluZ1Jlc3Vs dHNUb0NhbnZhcygpOwotICAgICAgICB9CisgICAgICAgIGlmIChtX2NvbnRleHQpCisgICAgICAg ICAgICBtX2NvbnRleHQtPnBhaW50UmVuZGVyaW5nUmVzdWx0c1RvQ2FudmFzKCk7CiAgICAgICAg IG1fY29waWVkSW1hZ2UgPSBidWZmZXIoKS0+Y29weUltYWdlKCk7CiAgICAgfQogICAgIHJldHVy biBtX2NvcGllZEltYWdlLmdldCgpOwo=