[webkit-reviews] review denied: [Bug 42908] WebGL must enforce restrictions even if running on OpenGL ES 2.0 : [Attachment 65012] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 24 13:00:56 PDT 2010


Kenneth Russell <kbr at google.com> has denied Zhenyao Mo <zmo at google.com>'s
request for review:
Bug 42908: WebGL must enforce restrictions even if running on OpenGL ES 2.0
https://bugs.webkit.org/show_bug.cgi?id=42908

Attachment 65012: patch
https://bugs.webkit.org/attachment.cgi?id=65012&action=review

------- Additional Comments from Kenneth Russell <kbr at google.com>
After looking at these changes I do not think that this is the right direction
to go. Please see below and reply with your thoughts.


WebCore/html/canvas/WebGLRenderingContext.cpp:306
 +	if ((!isGLES2Compliant() || !isGLES2NPOTStrict()) && texture)
This looks like a disaster waiting to happen. If we have multiple guards for
certain of the tracking and verification logic how are we going to keep it
straight? I think it would be better to just always do the tracking logic
regardless of whether we think we are running on OpenGL ES 2.0, because in the
presence of extensions (including ones that are exposed via WebGL) some of this
logic will be enabled once the user enables an extension.


WebCore/html/canvas/WebGLRenderingContext.cpp:542
 +	if (!isGLES2Compliant() || !isGLES2NPOTStrict())
Same comment here.


WebCore/html/canvas/WebGLRenderingContext.cpp:975
 +	if (!isGLES2Compliant() || !isErrorGeneratedOnOutOfBoundsAccesses()) {
Same comment here.


WebCore/html/canvas/WebGLRenderingContext.cpp:1130
 +	if (!isGLES2NPOTStrict() || !isGLES2Compliant()) {
Same here.


WebCore/html/canvas/WebGLRenderingContext.cpp:1137
 +	if (!isGLES2Compliant() || !isGLES2Compliant())
This is a flat out bug.


WebCore/html/canvas/WebGLRenderingContext.cpp:2108
 +	if (!isGLES2Compliant() || !isGLES2NPOTStrict())
Here too.


WebCore/html/canvas/WebGLRenderingContext.cpp:2222
 +	if (!isGLES2ParameterStrict() || !isGLES2NPOTStrict()) {
Here too.


More information about the webkit-reviews mailing list