Bug 77267 – Make WebGL put synthesized errors in the JS console

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Gregg Tavares]

Make WebGL put synthesized errors in the JS console

[Gregg Tavares]

Created attachment 124415 [details]
Patch

[Gregg Tavares]

I don't think adding the function name passing will add that much to the overhead of calling functions. They're static data.

[WebKit Review Bot]

Comment on attachment 124415 [details]
Patch

Attachment 124415 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11349945

New failing tests:
fast/canvas/webgl/error-reporting.html
http/tests/inspector/inspect-element.html
fast/canvas/webgl/buffer-data-array-buffer.html
fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
fast/canvas/webgl/gl-enable-enum-test.html
fast/canvas/webgl/gl-uniformmatrix4fv.html
fast/canvas/webgl/get-active-test.html
fast/canvas/webgl/context-lost-restored.html
fast/canvas/webgl/compressed-tex-image.html
fast/canvas/webgl/read-pixels-pack-alignment.html
fast/canvas/webgl/buffer-bind-test.html
fast/canvas/webgl/null-object-behaviour.html
fast/canvas/webgl/WebGLContextEvent.html
fast/canvas/webgl/program-test.html
fast/canvas/webgl/tex-image-and-sub-image-2d-with-image.html
fast/canvas/webgl/draw-arrays-out-of-bounds.html
fast/canvas/webgl/framebuffer-test.html
fast/canvas/webgl/object-deletion-behaviour.html
fast/canvas/webgl/context-lost.html
fast/canvas/webgl/attrib-location-length-limits.html
fast/canvas/webgl/bad-arguments-test.html
fast/canvas/webgl/incorrect-context-object-behaviour.html
fast/canvas/webgl/index-validation.html
fast/canvas/webgl/gl-enum-tests.html
fast/canvas/webgl/draw-elements-out-of-bounds.html
fast/canvas/webgl/glsl-conformance.html
fast/canvas/webgl/shader-deleted-by-accessor.html
fast/canvas/webgl/invalid-passed-params.html
fast/canvas/webgl/gl-vertexattribpointer.html
fast/canvas/webgl/context-destroyed-crash.html

[Kenneth Russell]

Comment on attachment 124415 [details]
Patch

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

[Gregg Tavares]

Created attachment 124847 [details]
Patch

[Gregg Tavares]

Comment on attachment 124415 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=124415&action=review

I fixed the typos and the blendFuncSeparate issue. I also added a flag m_synthesizedErrorsToConsole which defaults to false. I'll use another CL to turn it on through Settings.h for Chrome only. That way hopefully DRT won't need new expectations

>> 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.

done

>> 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

done

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

done

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

done

[Gregg Tavares]

Created attachment 124851 [details]
Patch

[Kenneth Russell]

Comment on attachment 124851 [details]
Patch

Looks good. r=me

[WebKit Review Bot]

Comment on attachment 124851 [details]
Patch

Clearing flags on attachment: 124851

Committed r106421: <http://trac.webkit.org/changeset/106421>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.