42852 2010-07-22 15:03:52 -0700 REGRESSION: box shadows on WebGL canvas elements repainted every frame 2010-07-22 17:19:26 -0700 1 1 1 Unclassified WebKit WebGL 528+ (Nightly build) All OS X 10.5 RESOLVED FIXED P2 Normal --- 1 jamesr zmo cmarrin kbr simon.fraser zmo oldest_to_newest 255009 0 jamesr 2010-07-22 15:03:52 -0700 Try adding a large box shadow to the WebGL spinning box example. The framerate drops because the shadow is re-drawn every frame. Only the WebGL's contents should be invalidated, not anything on the canvas itself. 255011 1 simon.fraser 2010-07-22 15:06:49 -0700 Regression from bug 34719. 255012 2 simon.fraser 2010-07-22 15:07:44 -0700 The HTMLCanvasElement::willDraw() call from WebGLRenderingContext::markContextChanged() is causing the canvas's renderer to get redrawn whenever the WebGL does stuff, which is very bad for performance. 255016 3 simon.fraser 2010-07-22 15:12:19 -0700 I don't understand the need for willDraw(), and the "Make sure the canvas's image buffer is allocated." comment. The canvas image buffer should only be created on demand. 255028 4 62351 jamesr 2010-07-22 15:38:34 -0700 Created attachment 62351 Patch 255030 5 jamesr 2010-07-22 15:40:03 -0700 This patch appears to fix the regression and still passes fast/canvas/webgl/canvas-test.html. I'm not sure why we have to call willDraw() at all before doing the paintRenderResultsToCanvas() though so I haven't marked the patch review?. 255032 6 cmarrin 2010-07-22 15:58:12 -0700 The code in WebGLRenderingContext::markContextChanged() looks wrong. In the accelerated compositing case the image buffer should never be allocated and the canvas should not be marked dirty (in the willDraw() call). This could be as simple as adding a #else clause to the HW_COMP test to do this only when not hardware accelerated. 255040 7 62354 jamesr 2010-07-22 16:10:16 -0700 Created attachment 62354 Patch 255072 8 62361 jamesr 2010-07-22 16:56:06 -0700 Created attachment 62361 Patch 255076 9 jamesr 2010-07-22 17:02:46 -0700 This patch reverts WebGLRenderingContext::markContextChanged() mostly to its previous behavior. The changes relative to the pre-r63502 version are: - unconditionally mark m_markedCanvasDirty so if we later need to do a readback we'll know the canvas is dirty - remove the unnecessary call to HTMLCanvasElement::buffer() and incorrect comment The call to ::willDraw() is still needed for the software rendering path. The other change is to add a call to ImageBuffer::clearImage right before calling GraphicsContext3D::paintRenderingResultsToCanvas in the readback path. I don't think this should be needed, but without this change canvas-test.html fails. I think it's better to leave this change in with a FIXME for now and try to figure out what is really going on. I've verified that this passes fast/canvas/webgl/* and renders WebGL content correctly in Safari and Chrome on a Snow Leopard machine. 255077 10 62361 kbr 2010-07-22 17:04:55 -0700 Comment on attachment 62361 Patch Looks good to me. Thanks for fixing this. 255090 11 62361 jamesr 2010-07-22 17:19:21 -0700 Comment on attachment 62361 Patch Clearing flags on attachment: 62361 Committed r63923: <http://trac.webkit.org/changeset/63923> 255091 12 jamesr 2010-07-22 17:19:26 -0700 All reviewed patches have been landed. Closing bug. 62351 2010-07-22 15:38:34 -0700 2010-07-22 16:10:13 -0700 Patch bug-42852-20100722153833.patch text/plain 2031 jamesr ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg YjMxNjVkMTY5ZDdiZDhkNTVlNWY2MjBmNTk0MDNhMTQ1MDk4ZjU3MC4uYzE4ZTU3YzhiMWY2MWUx ZTliN2ZkZjZlMzQwNjU3N2YxNjk5N2ZhNCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOCBAQAorMjAxMC0wNy0yMiAgSmFt ZXMgUm9iaW5zb24gIDxqYW1lc3JAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFJFR1JFU1NJT046IGJveCBzaGFkb3dzIG9uIFdl YkdMIGNhbnZhcyBlbGVtZW50cyByZXBhaW50ZWQgZXZlcnkgZnJhbWUKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQyODUyCisKKyAgICAgICAgSW5zdGVh ZCBvZiBjYWxsaW5nIEhUTUxDYW52YXNFbGVtZW50Ojp3aWxsRHJhdyB3aGVuZXZlciB0aGUgV2Vi R0wgY29udGVudHMKKyAgICAgICAgY2hhbmdlLCBvbmx5IGNhbGwgd2lsbERyYXcgaW1tZWRpYXRl bHkgYmVmb3JlIHBhaW50aW5nIHRoZSBjb250ZW50cyBpbnRvCisgICAgICAgIGFub3RoZXIgY2Fu dmFzLgorCisgICAgICAgICogaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdDb250ZXh0LmNwcDoK KyAgICAgICAgKFdlYkNvcmU6OldlYkdMUmVuZGVyaW5nQ29udGV4dDo6bWFya0NvbnRleHRDaGFu Z2VkKToKKyAgICAgICAgKFdlYkNvcmU6OldlYkdMUmVuZGVyaW5nQ29udGV4dDo6cGFpbnRSZW5k ZXJpbmdSZXN1bHRzVG9DYW52YXMpOgorCiAyMDEwLTA3LTIyICBTYW0gV2VpbmlnICA8c2FtQHdl YmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQW5kZXJzIENhcmxzc29uLgpkaWZmIC0t Z2l0IGEvV2ViQ29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQuY3BwIGIvV2Vi Q29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQuY3BwCmluZGV4IGY1NjdhYzgz Y2YyYmMyYzc2YjliMjlmYjQ0MDI1NTE4N2UxNjJkMWYuLmMyYmQ4ZDNkMDA0ZDE0NjY4OWY2MDc2 MTExODg0ZjFiMjJkMTkyYWMgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvaHRtbC9jYW52YXMvV2ViR0xS ZW5kZXJpbmdDb250ZXh0LmNwcAorKysgYi9XZWJDb3JlL2h0bWwvY2FudmFzL1dlYkdMUmVuZGVy aW5nQ29udGV4dC5jcHAKQEAgLTE0MiwxNyArMTQyLDEzIEBAIHZvaWQgV2ViR0xSZW5kZXJpbmdD b250ZXh0OjptYXJrQ29udGV4dENoYW5nZWQoKQogICAgIGlmIChyZW5kZXJCb3ggJiYgcmVuZGVy Qm94LT5oYXNMYXllcigpICYmIHJlbmRlckJveC0+bGF5ZXIoKS0+aGFzQWNjZWxlcmF0ZWRDb21w b3NpdGluZygpKQogICAgICAgICByZW5kZXJCb3gtPmxheWVyKCktPnJlbmRlcmVyQ29udGVudENo YW5nZWQoKTsKICNlbmRpZgotICAgIGlmICghbV9tYXJrZWRDYW52YXNEaXJ0eSkgewotICAgICAg ICAvLyBNYWtlIHN1cmUgdGhlIGNhbnZhcydzIGltYWdlIGJ1ZmZlciBpcyBhbGxvY2F0ZWQuCi0g ICAgICAgIGNhbnZhcygpLT5idWZmZXIoKTsKLSAgICAgICAgY2FudmFzKCktPndpbGxEcmF3KEZs b2F0UmVjdCgwLCAwLCBjYW52YXMoKS0+d2lkdGgoKSwgY2FudmFzKCktPmhlaWdodCgpKSk7Ci0g ICAgICAgIG1fbWFya2VkQ2FudmFzRGlydHkgPSB0cnVlOwotICAgIH0KKyAgICBtX21hcmtlZENh bnZhc0RpcnR5ID0gdHJ1ZTsKIH0KIAogYm9vbCBXZWJHTFJlbmRlcmluZ0NvbnRleHQ6OnBhaW50 UmVuZGVyaW5nUmVzdWx0c1RvQ2FudmFzKCkKIHsKICAgICBpZiAobV9tYXJrZWRDYW52YXNEaXJ0 eSkgeworICAgICAgICBjYW52YXMoKS0+d2lsbERyYXcoRmxvYXRSZWN0KDAsIDAsIGNhbnZhcygp LT53aWR0aCgpLCBjYW52YXMoKS0+aGVpZ2h0KCkpKTsKICAgICAgICAgbV9tYXJrZWRDYW52YXNE aXJ0eSA9IGZhbHNlOwogICAgICAgICBtX2NvbnRleHQtPnBhaW50UmVuZGVyaW5nUmVzdWx0c1Rv Q2FudmFzKHRoaXMpOwogICAgICAgICByZXR1cm4gdHJ1ZTsK 62354 2010-07-22 16:10:16 -0700 2010-07-22 16:56:03 -0700 Patch bug-42852-20100722161015.patch text/plain 2156 jamesr ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg YjMxNjVkMTY5ZDdiZDhkNTVlNWY2MjBmNTk0MDNhMTQ1MDk4ZjU3MC4uYjEzZDk1MWZjOTE5YTRi MzUxMWM3ZjlmOGM5YWViODFiOTRmMGZjYyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyMSBAQAorMjAxMC0wNy0yMiAgSmFt ZXMgUm9iaW5zb24gIDxqYW1lc3JAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFJFR1JFU1NJT046IGJveCBzaGFkb3dzIG9uIFdl YkdMIGNhbnZhcyBlbGVtZW50cyByZXBhaW50ZWQgZXZlcnkgZnJhbWUKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQyODUyCisKKyAgICAgICAgRWxpbWlu YXRlIGNhbGxzIHRvIEhUTUxDYW52YXNFbGVtZW50Ojp3aWxsRHJhdyBldmVyeSB0aW1lIG1hcmtD b250ZXh0Q2hhbmdlZCgpIGlzIGNhbGxlZC4KKyAgICAgICAgSW5zdGVhZCwgY2xlYXIgb3V0IHRo ZSBIVE1MQ2FudmFzRWxlbWVudCdzIGltYWdlIGJ1ZmZlciBvbmx5IGJlZm9yZSBjYWxsaW5nCisg ICAgICAgIHBhaW50UmVuZGVyaW5nUmVzdWx0c1RvQ2FudmFzKCkuICBUaGlzIGFwcGVhcnMgbmVj ZXNzYXJ5IHRvIHBhc3MgY2FudmFzLXRlc3QuaHRtbCBidXQgc2VlbXMKKyAgICAgICAgd3Jvbmcu CisKKyAgICAgICAgQ292ZXJlZCBieSBmYXN0L2NhbnZhcy93ZWJnbC9jYW52YXMtdGVzdC5odG1s CisKKyAgICAgICAgKiBodG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6V2ViR0xSZW5kZXJpbmdDb250ZXh0OjptYXJrQ29udGV4dENoYW5nZWQp OgorICAgICAgICAoV2ViQ29yZTo6V2ViR0xSZW5kZXJpbmdDb250ZXh0OjpwYWludFJlbmRlcmlu Z1Jlc3VsdHNUb0NhbnZhcyk6CisKIDIwMTAtMDctMjIgIFNhbSBXZWluaWcgIDxzYW1Ad2Via2l0 Lm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBBbmRlcnMgQ2FybHNzb24uCmRpZmYgLS1naXQg YS9XZWJDb3JlL2h0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5jcHAgYi9XZWJDb3Jl L2h0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5jcHAKaW5kZXggZjU2N2FjODNjZjJi YzJjNzZiOWIyOWZiNDQwMjU1MTg3ZTE2MmQxZi4uYTNiNWU4ZmEzOTRlYjliZjhkZjFhNGQxYmVm MjY5YzUwNGYyOTkzYiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRl cmluZ0NvbnRleHQuY3BwCisrKyBiL1dlYkNvcmUvaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdD b250ZXh0LmNwcApAQCAtMTQyLDE3ICsxNDIsMTMgQEAgdm9pZCBXZWJHTFJlbmRlcmluZ0NvbnRl eHQ6Om1hcmtDb250ZXh0Q2hhbmdlZCgpCiAgICAgaWYgKHJlbmRlckJveCAmJiByZW5kZXJCb3gt Pmhhc0xheWVyKCkgJiYgcmVuZGVyQm94LT5sYXllcigpLT5oYXNBY2NlbGVyYXRlZENvbXBvc2l0 aW5nKCkpCiAgICAgICAgIHJlbmRlckJveC0+bGF5ZXIoKS0+cmVuZGVyZXJDb250ZW50Q2hhbmdl ZCgpOwogI2VuZGlmCi0gICAgaWYgKCFtX21hcmtlZENhbnZhc0RpcnR5KSB7Ci0gICAgICAgIC8v IE1ha2Ugc3VyZSB0aGUgY2FudmFzJ3MgaW1hZ2UgYnVmZmVyIGlzIGFsbG9jYXRlZC4KLSAgICAg ICAgY2FudmFzKCktPmJ1ZmZlcigpOwotICAgICAgICBjYW52YXMoKS0+d2lsbERyYXcoRmxvYXRS ZWN0KDAsIDAsIGNhbnZhcygpLT53aWR0aCgpLCBjYW52YXMoKS0+aGVpZ2h0KCkpKTsKLSAgICAg ICAgbV9tYXJrZWRDYW52YXNEaXJ0eSA9IHRydWU7Ci0gICAgfQorICAgIG1fbWFya2VkQ2FudmFz RGlydHkgPSB0cnVlOwogfQogCiBib29sIFdlYkdMUmVuZGVyaW5nQ29udGV4dDo6cGFpbnRSZW5k ZXJpbmdSZXN1bHRzVG9DYW52YXMoKQogewogICAgIGlmIChtX21hcmtlZENhbnZhc0RpcnR5KSB7 CisgICAgICAgIGNhbnZhcygpLT5idWZmZXIoKS0+Y2xlYXJJbWFnZSgpOwogICAgICAgICBtX21h cmtlZENhbnZhc0RpcnR5ID0gZmFsc2U7CiAgICAgICAgIG1fY29udGV4dC0+cGFpbnRSZW5kZXJp bmdSZXN1bHRzVG9DYW52YXModGhpcyk7CiAgICAgICAgIHJldHVybiB0cnVlOwo= 62361 2010-07-22 16:56:06 -0700 2010-07-22 17:19:20 -0700 Patch bug-42852-20100722165605.patch text/plain 2421 jamesr ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg YjMxNjVkMTY5ZDdiZDhkNTVlNWY2MjBmNTk0MDNhMTQ1MDk4ZjU3MC4uZTkyYjdkMzhjZDE3MzQz MDdiZDg0MzA2MzQxMzE4YTczNTI3N2UzMSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyMCBAQAorMjAxMC0wNy0yMiAgSmFt ZXMgUm9iaW5zb24gIDxqYW1lc3JAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFJFR1JFU1NJT046IGJveCBzaGFkb3dzIG9uIFdl YkdMIGNhbnZhcyBlbGVtZW50cyByZXBhaW50ZWQgZXZlcnkgZnJhbWUKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQyODUyCisKKyAgICAgICAgQXZvaWQg Y2FsbGluZyBIVE1MQ2FudmFzRWxlbWVudDo6d2lsbERyYXcoKSBldmVyeSB0aW1lIGEgV2ViR0wg Y2FudmFzJworICAgICAgICBjb250ZW50cyB1cGRhdGUuICBUaGlzIGNhbGwgaXMgb25seSBuZWVk ZWQgaW4gdGhlIG5vbi1jb21wb3NpdGluZworICAgICAgICByZW5kZXJpbmcgcGF0aC4KKworICAg ICAgICBDb3ZlcmVkIGJ5IGZhc3QvY2FudmFzL3dlYmdsL2NhbnZhcy10ZXN0Lmh0bWwKKworICAg ICAgICAqIGh0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5jcHA6CisgICAgICAgIChX ZWJDb3JlOjpXZWJHTFJlbmRlcmluZ0NvbnRleHQ6Om1hcmtDb250ZXh0Q2hhbmdlZCk6CisgICAg ICAgIChXZWJDb3JlOjpXZWJHTFJlbmRlcmluZ0NvbnRleHQ6OnBhaW50UmVuZGVyaW5nUmVzdWx0 c1RvQ2FudmFzKToKKwogMjAxMC0wNy0yMiAgU2FtIFdlaW5pZyAgPHNhbUB3ZWJraXQub3JnPgog CiAgICAgICAgIFJldmlld2VkIGJ5IEFuZGVycyBDYXJsc3Nvbi4KZGlmZiAtLWdpdCBhL1dlYkNv cmUvaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdDb250ZXh0LmNwcCBiL1dlYkNvcmUvaHRtbC9j YW52YXMvV2ViR0xSZW5kZXJpbmdDb250ZXh0LmNwcAppbmRleCBmNTY3YWM4M2NmMmJjMmM3NmI5 YjI5ZmI0NDAyNTUxODdlMTYyZDFmLi5mYjE3ZGI5MjQ2MTExYTc2YTIxNGU0MjQ2ODRlNzU2YjNi MDM3YWFhIDEwMDY0NAotLS0gYS9XZWJDb3JlL2h0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29u dGV4dC5jcHAKKysrIGIvV2ViQ29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQu Y3BwCkBAIC0xNDEsMTggKzE0MSwyMiBAQCB2b2lkIFdlYkdMUmVuZGVyaW5nQ29udGV4dDo6bWFy a0NvbnRleHRDaGFuZ2VkKCkKICAgICBSZW5kZXJCb3gqIHJlbmRlckJveCA9IGNhbnZhcygpLT5y ZW5kZXJCb3goKTsKICAgICBpZiAocmVuZGVyQm94ICYmIHJlbmRlckJveC0+aGFzTGF5ZXIoKSAm JiByZW5kZXJCb3gtPmxheWVyKCktPmhhc0FjY2VsZXJhdGVkQ29tcG9zaXRpbmcoKSkKICAgICAg ICAgcmVuZGVyQm94LT5sYXllcigpLT5yZW5kZXJlckNvbnRlbnRDaGFuZ2VkKCk7CisgICAgZWxz ZSB7CiAjZW5kaWYKLSAgICBpZiAoIW1fbWFya2VkQ2FudmFzRGlydHkpIHsKLSAgICAgICAgLy8g TWFrZSBzdXJlIHRoZSBjYW52YXMncyBpbWFnZSBidWZmZXIgaXMgYWxsb2NhdGVkLgotICAgICAg ICBjYW52YXMoKS0+YnVmZmVyKCk7Ci0gICAgICAgIGNhbnZhcygpLT53aWxsRHJhdyhGbG9hdFJl Y3QoMCwgMCwgY2FudmFzKCktPndpZHRoKCksIGNhbnZhcygpLT5oZWlnaHQoKSkpOwotICAgICAg ICBtX21hcmtlZENhbnZhc0RpcnR5ID0gdHJ1ZTsKKyAgICAgICAgaWYgKCFtX21hcmtlZENhbnZh c0RpcnR5KQorICAgICAgICAgICAgY2FudmFzKCktPndpbGxEcmF3KEZsb2F0UmVjdCgwLCAwLCBj YW52YXMoKS0+d2lkdGgoKSwgY2FudmFzKCktPmhlaWdodCgpKSk7CisjaWYgVVNFKEFDQ0VMRVJB VEVEX0NPTVBPU0lUSU5HKQogICAgIH0KKyNlbmRpZgorICAgIG1fbWFya2VkQ2FudmFzRGlydHkg PSB0cnVlOwogfQogCiBib29sIFdlYkdMUmVuZGVyaW5nQ29udGV4dDo6cGFpbnRSZW5kZXJpbmdS ZXN1bHRzVG9DYW52YXMoKQogewogICAgIGlmIChtX21hcmtlZENhbnZhc0RpcnR5KSB7CisgICAg ICAgIC8vIEZJWE1FOiBJdCBzaG91bGQgbm90IGJlIG5lY2Vzc2FyeSB0byBjbGVhciB0aGUgaW1h Z2UgYmVmb3JlIGRvaW5nIGEgcmVhZGJhY2suCisgICAgICAgIC8vIEludmVzdGlnYXRlIHdoeSB0 aGlzIGlzIG5lZWRlZCBhbmQgcmVtb3ZlIGlmIHBvc3NpYmxlLgorICAgICAgICBjYW52YXMoKS0+ YnVmZmVyKCktPmNsZWFySW1hZ2UoKTsKICAgICAgICAgbV9tYXJrZWRDYW52YXNEaXJ0eSA9IGZh bHNlOwogICAgICAgICBtX2NvbnRleHQtPnBhaW50UmVuZGVyaW5nUmVzdWx0c1RvQ2FudmFzKHRo aXMpOwogICAgICAgICByZXR1cm4gdHJ1ZTsK