Implement uniform* and getUniform for WebGL 2 types

• Component: WebGL
• Status: RESOLVED FIXED

[James Darpinian]

Implement uniform* and getUniform for WebGL 2 types

[James Darpinian]

Created attachment 403830 [details]
Patch

[James Darpinian]

Created attachment 403933 [details]
fix gtk build and rebaseline tests

[James Darpinian]

Created attachment 404005 [details]
rebase

[Kenneth Russell]

Comment on attachment 404005 [details]
rebase

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

Great work James. LGTM - I don't have reviewer privileges.

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1285
> +    if (location->program() != m_currentProgram) {

Maybe we should add a bool validateUniformLocation(WebGLUniformLocation*, const char* functionName).

[Dean Jackson]

Comment on attachment 404005 [details]
rebase

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

>> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1285
>> +    if (location->program() != m_currentProgram) {
> 
> Maybe we should add a bool validateUniformLocation(WebGLUniformLocation*, const char* functionName).

Agreed.

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1329
> +    m_context->uniform1uiv(location->location(), value.data(), srcOffset, srcLength ? srcLength : (value.length() - srcOffset) / 1);

Maybe remove the "/1"?

Also, do we need to validate the case where srcLength is 0 and srcOffset is > value.length?

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:3409
> +            // Can't handle this type

Nit: End with a .

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:3471
> +            // Can't handle this type

Nit: end with a .

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:6900
> +    GCGLsizei actualSize = size - srcOffset;
> +    if (srcLength > 0) {
> +        if (srcLength > static_cast<GCGLuint>(actualSize)) {
> +            synthesizeGLError(GraphicsContextGL::INVALID_VALUE, functionName, "invalid srcOffset + srcLength");
> +            return false;
> +        }
> +        actualSize = srcLength;

Oh, you did it here :)

> Source/WebCore/platform/graphics/GraphicsContextGL.h:1026
> +    virtual void getUniformuiv(PlatformGLObject program, GCGLint location, GCGLuint* value) = 0;

Huh. I wonder how I missed this.

[Kenneth Russell]

Comment on attachment 404005 [details]
rebase

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

>> Source/WebCore/platform/graphics/GraphicsContextGL.h:1026
>> +    virtual void getUniformuiv(PlatformGLObject program, GCGLint location, GCGLuint* value) = 0;
> 
> Huh. I wonder how I missed this.

Unsigned integer uniforms are new in WebGL 2.0 and this patch is the first one referencing them.

[James Darpinian]

Created attachment 404195 [details]
review feedback

[James Darpinian]

Comment on attachment 404195 [details]
review feedback

All review feedback addressed. Thanks!

[EWS]

Committed r264371: <https://trac.webkit.org/changeset/264371>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404195 [details].

[Radar WebKit Bug Importer]

<rdar://problem/65559641>