RESOLVED FIXED 214116
Implement uniform* and getUniform for WebGL 2 types 
https://bugs.webkit.org/show_bug.cgi?id=214116
Summary Implement uniform* and getUniform for WebGL 2 types
James Darpinian
Reported 2020-07-08 17:59:52 PDT
Implement uniform* and getUniform for WebGL 2 types
Attachments
Patch (50.58 KB, patch)
2020-07-08 18:01 PDT, James Darpinian 		no flags
DetailsFormatted DiffDiff
fix gtk build and rebaseline tests (57.25 KB, patch)
2020-07-09 17:05 PDT, James Darpinian 		no flags
DetailsFormatted DiffDiff
rebase (57.30 KB, patch)
2020-07-10 14:57 PDT, James Darpinian 		no flags
DetailsFormatted DiffDiff
review feedback (61.82 KB, patch)
2020-07-13 17:38 PDT, James Darpinian 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
James Darpinian
Comment 1 2020-07-08 18:01:10 PDT
Created attachment 403830 [details] Patch
James Darpinian
Comment 2 2020-07-09 17:05:39 PDT
Created attachment 403933 [details] fix gtk build and rebaseline tests
James Darpinian
Comment 3 2020-07-10 14:57:55 PDT
Created attachment 404005 [details] rebase
Kenneth Russell
Comment 4 2020-07-10 15:08:07 PDT
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 5 2020-07-10 18:05:27 PDT
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 6 2020-07-13 14:58:34 PDT
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
Comment 7 2020-07-13 17:38:40 PDT
Created attachment 404195 [details] review feedback
James Darpinian
Comment 8 2020-07-13 22:54:11 PDT
Comment on attachment 404195 [details] review feedback All review feedback addressed. Thanks!
EWS
Comment 9 2020-07-14 12:59:05 PDT
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
Comment 10 2020-07-14 13:00:18 PDT
<rdar://problem/65559641>
Note You need to log in before you can comment on or make changes to this bug.