Bug 77267 - Make WebGL put synthesized errors in the JS console
Summary: Make WebGL put synthesized errors in the JS console
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-27 18:14 PST by Gregg Tavares
Modified: 2012-01-31 19:17 PST (History)
4 users (show)

See Also:


Attachments
Patch (141.54 KB, patch)
2012-01-27 18:15 PST, Gregg Tavares
no flags Details | Formatted Diff | Diff
Patch (150.70 KB, patch)
2012-01-31 16:43 PST, Gregg Tavares
no flags Details | Formatted Diff | Diff
Patch (142.30 KB, patch)
2012-01-31 16:53 PST, Gregg Tavares
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gregg Tavares 2012-01-27 18:14:07 PST
Make WebGL put synthesized errors in the JS console
Comment 1 Gregg Tavares 2012-01-27 18:15:02 PST
Created attachment 124415 [details]
Patch
Comment 2 Gregg Tavares 2012-01-27 18:16:58 PST
I don't think adding the function name passing will add that much to the overhead of calling functions. They're static data.
Comment 3 WebKit Review Bot 2012-01-27 18:46:03 PST
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
Comment 4 Kenneth Russell 2012-01-31 10:31:44 PST
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
Comment 5 Gregg Tavares 2012-01-31 16:43:52 PST
Created attachment 124847 [details]
Patch
Comment 6 Gregg Tavares 2012-01-31 16:48:36 PST
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
Comment 7 Gregg Tavares 2012-01-31 16:53:16 PST
Created attachment 124851 [details]
Patch
Comment 8 Kenneth Russell 2012-01-31 16:57:14 PST
Comment on attachment 124851 [details]
Patch

Looks good. r=me
Comment 9 WebKit Review Bot 2012-01-31 19:17:23 PST
Comment on attachment 124851 [details]
Patch

Clearing flags on attachment: 124851

Committed r106421: <http://trac.webkit.org/changeset/106421>
Comment 10 WebKit Review Bot 2012-01-31 19:17:29 PST
All reviewed patches have been landed.  Closing bug.