[webkit-reviews] review granted: [Bug 77267] Make WebGL put synthesized errors in the JS console : [Attachment 124415] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has granted Gregg Tavares <gman at google.com>'s
request for review:
Bug 77267: Make WebGL put synthesized errors in the JS console
https://bugs.webkit.org/show_bug.cgi?id=77267

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

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=124415&action=review


Looks good. A few minor issues. Test expectations also need to be updated in
the next version of the patch. r=me

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:979
> +    if (isContextLost() || !validateBlendFuncFactors("blendFunc", srcRGB,
dstRGB) || !validateBlendFuncFactors("blendFunc", srcAlpha, dstAlpha))

The additional call to validateBlendFuncFactors checking srcAlpha and dstAlpha
isn't correct. The restrictions on srcRGB and dstRGB don't apply to the alpha
factors. See the spec and ANGLE's glBlendFuncSeparate in
src/libGLESv2/libGLESv2.cpp.

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2027
> +	   synthesizeGLError(GraphicsContext3D::INVALID_OPERATION,
"generateMipmaps", "level 0 not power of 2 or not all the same size");

generateMipmaps -> generateMipmap

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2077
> +    if (isContextLost() || !validateWebGLObject("getAttachedShader",
program))

getAttachedShader -> getAttachedShaders

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:3136
> +	   synthesizeGLError(GraphicsContext3D::INVALID_OPERATION,
"rendebufferStorage", "no bound renderbuffer");

rendebufferStorage -> renderbufferStorage