Summary: | [WebGL] Correct uniform input validation for texture sampler uniforms | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||
Component: | WebGL | Assignee: | Brent Fulgham <bfulgham> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bfulgham, commit-queue, darin, dino, esprehn+autocc, gyuyoung.kim, kondapallykalyan, roger_fong, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | All | ||||||||
Bug Depends on: | 122610 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Brent Fulgham
2014-01-10 13:23:02 PST
(In reply to comment #0) > WebGL: INVALID_VALUE: uniform1: invalid texture unit. Typo: I meant to say: WebGL: INVALID_VALUE: uniform1i: invalid texture unit. This stopped working in http://trac.webkit.org/changeset/157271. However, it looks like the new code is doing the right thing, and preventing us from walking past the end of the available texture units. Created attachment 220893 [details]
Patch
This was a bug in the way the Int32Array type was being accessed. The code was attempting to cast the Int32Array* to a GC3DInt*, which resulted in garbage. Also added a test case to help avoid this in the future. Comment on attachment 220893 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220893&action=review r+, but please copy into a new test. > Source/WebCore/ChangeLog:8 > + Revised webgl/1.0.2/resources/webgl_test_files/conformance/uniforms/uniform-samplers-test.html We shouldn't do that. We should try to keep the webgl/1.0.2 files in sync with Khronos. Instead, copy the test into fast/canvas/webgl and make the changes there. > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4205 > + LOG(WebGL, "Calling WebGLRenderingContext::uniform1iv (Int32Array* Variation)"); Should we keep this in here? It might be really noisy. > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4225 > + LOG(WebGL, "Calling WebGLRenderingContext::uniform1iv (GC3Dint* Variation)"); Ditto. (In reply to comment #6) > (From update of attachment 220893 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220893&action=review > > r+, but please copy into a new test. OK! > > Source/WebCore/ChangeLog:8 > > + Revised webgl/1.0.2/resources/webgl_test_files/conformance/uniforms/uniform-samplers-test.html > > We shouldn't do that. We should try to keep the webgl/1.0.2 files in sync with Khronos. Instead, copy the test into fast/canvas/webgl and make the changes there. Done. > > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4205 > > + LOG(WebGL, "Calling WebGLRenderingContext::uniform1iv (Int32Array* Variation)"); > > Should we keep this in here? It might be really noisy. Removed. > > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4225 > > + LOG(WebGL, "Calling WebGLRenderingContext::uniform1iv (GC3Dint* Variation)"); > > Ditto. Removed. Committed r161684: <http://trac.webkit.org/changeset/161684> This patch seems to have added a test with no test results: fast/canvas/webgl/uniform-samplers-test.html Also, the test seems to be failing on my computer. (In reply to comment #9) > This patch seems to have added a test with no test results: fast/canvas/webgl/uniform-samplers-test.html > > Also, the test seems to be failing on my computer. Fixing now. Reopened to add missing test. Created attachment 220951 [details]
Patch
Committed r161794: <http://trac.webkit.org/changeset/161794> |