47964 2010-10-19 19:00:34 -0700 If WebGL is running on top of a strict version of OpenGL ES it should make sure attribs have buffers assigned at all times 2010-10-27 14:03:55 -0700 1 1 1 Unclassified WebKit WebGL 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 gman zmo abarth cmarrin enne eric kbr webkit.review.bot zmo oldest_to_newest 296599 0 gman 2010-10-19 19:00:34 -0700 Chromium has a strict version of OpenGL ES 2.0 that does all the attrib array bounds checking needed by WebGL so it would be nice if WebGL in webkit didn't redundantly do bounds checking. Unfortunately, when removing the current bounds checking the issue comes up that WebGL can call drawArrays or drawElements when attributes are enabled but no buffer has been assigned. This crashes OpenGL (both the Chromium version and real OpenGL) The suggested solution is to create a dummy buffer, assign it to every attribute at initialization time, If the user deletes a buffer or calls vertexAttribPointer with NULL then instead of putting NULL in the buffer, assign the dummy buffer to the attrib in question This avoids having to check that buffers are assigned at draw time. It add the complication that when querying the state of an attrib you have to shadow its state since if the dummy buffer is assigned you should be telling the user that no buffer is assigned. 296867 1 gman 2010-10-20 10:44:12 -0700 Actually, rethinking this, WebGL still has to verify that attribs that are enabled have buffers assigned to them. The things that need to happen are....If WebGL us running on a strict version of OpenGL ES then 1) bounds do not need to be checked 2) index buffers do not need to be shadowed 296909 2 71315 zmo 2010-10-20 12:05:36 -0700 Created attachment 71315 patch With this patch in, we can turn off bounds checking in WebGLRenderingContext if using command buffer port. This is to avoid bounds checking twice. 299793 3 71315 kbr 2010-10-26 14:21:43 -0700 Comment on attachment 71315 patch The patch looks fine, but is this situation covered by existing layout tests? If not, one is needed (upstream in Khronos as well as pulled down to WebKit). 299843 4 zmo 2010-10-26 15:19:50 -0700 (In reply to comment #3) > (From update of attachment 71315 [details]) > The patch looks fine, but is this situation covered by existing layout tests? If not, one is needed (upstream in Khronos as well as pulled down to WebKit). Yes, it is covered. Without this patch, if we turn off the bounds-checking in WebGLRenderingContext and turn it on in command buffer, some texts will crash. 299847 5 71315 kbr 2010-10-26 15:25:08 -0700 Comment on attachment 71315 patch OK. Looks good to me then. 300317 6 71315 zmo 2010-10-27 10:10:26 -0700 Comment on attachment 71315 patch Clearing flags on attachment: 71315 Committed r70661: <http://trac.webkit.org/changeset/70661> 300318 7 zmo 2010-10-27 10:10:31 -0700 All reviewed patches have been landed. Closing bug. 300543 8 webkit.review.bot 2010-10-27 14:03:55 -0700 http://trac.webkit.org/changeset/70661 might have broken GTK Linux 32-bit Debug 71315 2010-10-20 12:05:36 -0700 2010-10-27 10:10:26 -0700 patch bounds.patch text/plain 3142 zmo SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA3MDE1OCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMTAtMTAtMjAgIFpoZW55YW8gTW8gIDx6bW9AZ29vZ2xlLmNvbT4K KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJZiBXZWJH TCBpcyBydW5uaW5nIG9uIHRvcCBvZiBhIHN0cmljdCB2ZXJzaW9uIG9mIE9wZW5HTCBFUyBpdCBz aG91bGQgbWFrZSBzdXJlIGF0dHJpYnMgaGF2ZSBidWZmZXJzIGFzc2lnbmVkIGF0IGFsbCB0aW1l cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDc5NjQK KworICAgICAgICAqIGh0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5jcHA6CisgICAg ICAgIChXZWJDb3JlOjpXZWJHTFJlbmRlcmluZ0NvbnRleHQ6OnZhbGlkYXRlUmVuZGVyaW5nU3Rh dGUpOiBNaW5pbXVtIGNoZWNraW5nOiBpZiBpbnB1dCA8PSAwLCBvbmx5IGNoZWNrIGlmIGVhY2gg ZW5hYmxlZCB2ZXJ0ZXggYXR0cmlidXRlIGlzIGJvdW5kIHRvIGEgYnVmZmVyLgorICAgICAgICAo V2ViQ29yZTo6V2ViR0xSZW5kZXJpbmdDb250ZXh0OjpkcmF3QXJyYXlzKTogSWYgdW5kZXJseWlu ZyBHTCBwZXJmb3JtcyBib3VuZHMgY2hlY2tpbmcsIHdlIHN0aWxsIG5lZWQgdG8gZG8gdGhlIG1p bmltdW0gY2hlY2tpbmcgdXNpbmcgdmFsaWRhdGVSZW5kZXJpbmdTdGF0ZS4KKyAgICAgICAgKFdl YkNvcmU6OldlYkdMUmVuZGVyaW5nQ29udGV4dDo6ZHJhd0VsZW1lbnRzKTogRGl0dG8uCisgICAg ICAgICogaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdDb250ZXh0Lmg6IEFkZCBhIGNvbW1lbnQg Zm9yIGlucHV0IDw9IDAgaW4gdmFsaWRhdGVSZW5kZXJpbmdTdGF0ZS4KKwogMjAxMC0xMC0yMCAg RGF2aWQgSHlhdHQgIDxoeWF0dEBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgU2lt b24gRnJhc2VyLgpJbmRleDogV2ViQ29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRl eHQuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdD b250ZXh0LmNwcAkocmV2aXNpb24gNzAxNTcpCisrKyBXZWJDb3JlL2h0bWwvY2FudmFzL1dlYkdM UmVuZGVyaW5nQ29udGV4dC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTg3OCw2ICs4NzgsOSBAQCBi b29sIFdlYkdMUmVuZGVyaW5nQ29udGV4dDo6dmFsaWRhdGVSZW5kCiAgICAgICAgICAgICByZXR1 cm4gZmFsc2U7CiAgICAgfQogCisgICAgaWYgKG51bUVsZW1lbnRzUmVxdWlyZWQgPD0gMCkKKyAg ICAgICAgcmV0dXJuIHRydWU7CisKICAgICAvLyBMb29rIGluIGVhY2ggY29uc3VtZWQgdmVydGV4 IGF0dHJpYiAoYnkgdGhlIGN1cnJlbnQgcHJvZ3JhbSkgYW5kIGZpbmQgdGhlIHNtYWxsZXN0IGJ1 ZmZlciBzaXplCiAgICAgbG9uZyBzbWFsbGVzdE51bUVsZW1lbnRzID0gTE9OR19NQVg7CiAgICAg aW50IG51bUFjdGl2ZUF0dHJpYkxvY2F0aW9ucyA9IG1fY3VycmVudFByb2dyYW0tPm51bUFjdGl2 ZUF0dHJpYkxvY2F0aW9ucygpOwpAQCAtOTM5LDYgKzk0MiwxMSBAQCB2b2lkIFdlYkdMUmVuZGVy aW5nQ29udGV4dDo6ZHJhd0FycmF5cyh1CiAgICAgICAgICAgICBtX2NvbnRleHQtPnN5bnRoZXNp emVHTEVycm9yKEdyYXBoaWNzQ29udGV4dDNEOjpJTlZBTElEX09QRVJBVElPTik7CiAgICAgICAg ICAgICByZXR1cm47CiAgICAgICAgIH0KKyAgICB9IGVsc2UgeworICAgICAgICBpZiAoIXZhbGlk YXRlUmVuZGVyaW5nU3RhdGUoMCkpIHsKKyAgICAgICAgICAgIG1fY29udGV4dC0+c3ludGhlc2l6 ZUdMRXJyb3IoR3JhcGhpY3NDb250ZXh0M0Q6OklOVkFMSURfT1BFUkFUSU9OKTsKKyAgICAgICAg ICAgIHJldHVybjsKKyAgICAgICAgfQogICAgIH0KIAogICAgIGJvb2wgdmVydGV4QXR0cmliMFNp bXVsYXRlZCA9IGZhbHNlOwpAQCAtOTkwLDYgKzk5OCwxMSBAQCB2b2lkIFdlYkdMUmVuZGVyaW5n Q29udGV4dDo6ZHJhd0VsZW1lbnRzCiAgICAgICAgICAgICAgICAgcmV0dXJuOwogICAgICAgICAg ICAgfQogICAgICAgICB9CisgICAgfSBlbHNlIHsKKyAgICAgICAgaWYgKCF2YWxpZGF0ZVJlbmRl cmluZ1N0YXRlKDApKSB7CisgICAgICAgICAgICBtX2NvbnRleHQtPnN5bnRoZXNpemVHTEVycm9y KEdyYXBoaWNzQ29udGV4dDNEOjpJTlZBTElEX09QRVJBVElPTik7CisgICAgICAgICAgICByZXR1 cm47CisgICAgICAgIH0KICAgICB9CiAKICAgICBib29sIHZlcnRleEF0dHJpYjBTaW11bGF0ZWQg PSBmYWxzZTsKSW5kZXg6IFdlYkNvcmUvaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdDb250ZXh0 LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRl eHQuaAkocmV2aXNpb24gNzAxNTcpCisrKyBXZWJDb3JlL2h0bWwvY2FudmFzL1dlYkdMUmVuZGVy aW5nQ29udGV4dC5oCSh3b3JraW5nIGNvcHkpCkBAIC0zMjMsNiArMzIzLDcgQEAgcHVibGljOgog CiAgICAgLy8gUHJlY2lzZSBidXQgc2xvdyBpbmRleCB2YWxpZGF0aW9uIC0tIG9ubHkgZG9uZSBp ZiBjb25zZXJ2YXRpdmUgY2hlY2tzIGZhaWwKICAgICBib29sIHZhbGlkYXRlSW5kZXhBcnJheVBy ZWNpc2UodW5zaWduZWQgbG9uZyBjb3VudCwgdW5zaWduZWQgbG9uZyB0eXBlLCBsb25nIG9mZnNl dCwgbG9uZyYgbnVtRWxlbWVudHNSZXF1aXJlZCk7CisgICAgLy8gSWYgbnVtRWxlbWVudHMgPD0g MCwgd2Ugb25seSBjaGVjayBpZiBlYWNoIGVuYWJsZWQgdmVydGV4IGF0dHJpYnV0ZSBpcyBib3Vu ZCB0byBhIGJ1ZmZlci4KICAgICBib29sIHZhbGlkYXRlUmVuZGVyaW5nU3RhdGUobG9uZyBudW1F bGVtZW50cyk7CiAKICAgICBib29sIHZhbGlkYXRlV2ViR0xPYmplY3QoV2ViR0xPYmplY3QqIG9i amVjdCk7Cg==