webgl: Add attribute validations and other nits #10224

emilio · 2016-03-27T14:32:00Z

Fixes #9958

Depends on a bunch of prs, and needs a test.

r? @jdm

This change is

highfive · 2016-03-27T14:32:05Z

Heads up! This PR modifies the following files:

@KiChjang: components/script/dom/webglrenderingcontext.rs, components/script/dom/webglshader.rs, components/script_traits/script_msg.rs, components/script_traits/script_msg.rs, components/script/dom/bindings/trace.rs
@emilio: components/script/dom/webglrenderingcontext.rs, components/script/dom/webglshader.rs

highfive · 2016-03-27T14:32:06Z

Warning

These commits modify script code, but no tests are modified. Please consider adding a test!

emilio · 2016-03-27T15:33:37Z

Test added.

btw josh, we don't support HTMLScriptElement.type and that made me bang my head against the wall for a while asking why my shader didn't compile (invalid type, so it didn't recognise gl_FragColor).


        Auto merge of #26 - emilio:webgl-limits, r=glennw

@glennw

Webgl limits Return the GLLimits structure from the `RequestWebGLContext` API message. This allows us to keep a lot of calls that would require validation here asynchronous. See servo/servo#10224 r? @glennw

emilio · 2016-03-28T21:27:52Z

The PRs landed, so this is ready for review.

bors-servo · 2016-03-30T14:29:34Z

☔ The latest upstream changes (presumably #10242) made this pull request unmergeable. Please resolve the merge conflicts.

pcwalton · 2016-03-30T23:54:16Z

Is this ready to go? If so, note that 4 browser.html P1s are blocked on this. If not, I think we should revert the rust-offscreen-rendering-context upgrades in webrender and webrender_traits.

nox · 2016-03-31T00:38:59Z

#10297 unblocks the situation.

bors-servo · 2016-03-31T02:15:34Z

☔ The latest upstream changes (presumably #10297) made this pull request unmergeable. Please resolve the merge conflicts.


        Update webrender to master

I'll rebase servo#10224 against it.


        webgl: Validate shader type parameter to CreateShader.


        webgl: Use early return in DrawArrays


        webgl: Remove unneeded return value.


        webgl: Make the api return the context limits and use them for valida…

…tions This allows keeping the VertexAttrib* calls asynchronous. Another option would be to do the validation in the apply() function, but that'd require us passing an unnecessary channel around and add extra synchronization. The counterpart of this is that it has to be updated when the context changes, but that's less problem.


        webgl: tests: Fix webgl triangle test

It was giving a ton of whitespace errors, and I don't know if it's due to nodeValue or ANGLE...


        webgl: Add test for WebGL vertex attributes out of bounds.

jdm · 2016-04-11T21:42:45Z

@bors-servo: r+
Sorry for sitting on this!

Reviewed 10 of 10 files at r1, 2 of 2 files at r2, 10 of 10 files at r3, 9 of 9 files at r4, 6 of 6 files at r5, 8 of 8 files at r6, 4 of 4 files at r7, 9 of 9 files at r8, 7 of 7 files at r9.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

components/canvas/webgl_paint_thread.rs, line 31 [r3] (raw file):
nit: indent this to match the w on the previous line.

Comments from Reviewable

bors-servo · 2016-04-11T21:42:46Z

📌 Commit 70229b1 has been approved by jdm

bors-servo · 2016-04-11T22:27:51Z

⌛ Testing commit 70229b1 with merge f8d6238...


        Auto merge of #10224 - emilio:shader-type-validations, r=jdm

@jdm

webgl: Add attribute validations and other nits Fixes #9958 Depends on a bunch of prs, and needs a test. r? @jdm  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10224)

bors-servo · 2016-04-11T23:09:36Z

💔 Test failed - linux-rel

emilio · 2016-04-11T23:15:05Z

No problem! I don't mind rebasing as many times as necessary :P

Glad to see you're cleaning up the review queue :)

@bors-servo: retry

#10473

bors-servo · 2016-04-11T23:48:49Z

⌛ Testing commit 70229b1 with merge f0014bd...


        Auto merge of #10224 - emilio:shader-type-validations, r=jdm

@jdm

webgl: Add attribute validations and other nits Fixes #9958 Depends on a bunch of prs, and needs a test. r? @jdm  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10224)

bors-servo · 2016-04-12T00:28:14Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt

emilio added S-blocked-on-external A-content/webgl labels Mar 27, 2016

highfive assigned jdm Mar 27, 2016

highfive added the S-awaiting-review label Mar 27, 2016

emilio force-pushed the emilio:shader-type-validations branch from 99affb4 to 0d5b783 Mar 27, 2016

jdm added the S-needs-tests label Mar 27, 2016

This was referenced Mar 27, 2016

Webgl limits servo/webrender_traits#26

Merged

webgl: Return the GLLimits structure to the caller servo/webrender#250

Closed

emilio force-pushed the emilio:shader-type-validations branch 2 times, most recently from af084d4 to f2ebf29 Mar 27, 2016

emilio removed the S-needs-tests label Mar 27, 2016

emilio force-pushed the emilio:shader-type-validations branch 2 times, most recently from c3871a1 to 713d042 Mar 28, 2016

emilio removed the S-blocked-on-external label Mar 28, 2016

emilio mentioned this pull request Mar 29, 2016

Handle nested transforms and clips when the transform consists solely of a translation. servo/webrender#251

Merged

emilio force-pushed the emilio:shader-type-validations branch from 713d042 to 40a214b Mar 29, 2016

highfive added the S-needs-rebase label Mar 30, 2016

emilio force-pushed the emilio:shader-type-validations branch from 40a214b to db1a86f Mar 30, 2016

emilio removed the S-needs-rebase label Mar 30, 2016

highfive added the S-needs-rebase label Mar 31, 2016

emilio added a commit to emilio/servo that referenced this pull request Mar 31, 2016

Update webrender to master

656f222

I'll rebase servo#10224 against it.

highfive added the S-needs-rebase label Apr 2, 2016

emilio added 6 commits Mar 27, 2016

webgl: Validate shader type parameter to CreateShader.

240ac7c

webgl: Use early return in DrawArrays

466c888

webgl: Remove unneeded return value.

8d7ee15

webgl: tests: Fix webgl triangle test

ee8ecd9

It was giving a ton of whitespace errors, and I don't know if it's due to nodeValue or ANGLE...

webgl: Add test for WebGL vertex attributes out of bounds.

Loading status checks…

70229b1

emilio force-pushed the emilio:shader-type-validations branch from ae7bde9 to 70229b1 Apr 2, 2016

emilio removed the S-needs-rebase label Apr 5, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Apr 11, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Apr 11, 2016

highfive removed the S-tests-failed label Apr 11, 2016

highfive added the S-awaiting-merge label Apr 11, 2016

bors-servo merged commit 70229b1 into servo:master Apr 12, 2016
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Apr 12, 2016

emilio deleted the emilio:shader-type-validations branch Apr 12, 2016

This was referenced Apr 12, 2016

Replace side-effecting unwrap_or_else by if let in constellation. #10345

Merged

Replace use of `debug!` by `warn!` in `constellation.rs` #10489

Merged

jdm mentioned this pull request Aug 25, 2016

Panic loading http://people.mozilla.org/~sicking/webgl/ray.html #9097

Closed

servo / servo

webgl: Add attribute validations and other nits #10224

webgl: Add attribute validations and other nits #10224

emilio commented Mar 27, 2016

highfive commented Mar 27, 2016

highfive commented Mar 27, 2016

emilio commented Mar 27, 2016

emilio commented Mar 28, 2016

bors-servo commented Mar 30, 2016

pcwalton commented Mar 30, 2016

nox commented Mar 31, 2016

bors-servo commented Mar 31, 2016

jdm commented Apr 11, 2016

bors-servo commented Apr 11, 2016

bors-servo commented Apr 11, 2016

bors-servo commented Apr 11, 2016

emilio commented Apr 11, 2016

bors-servo commented Apr 11, 2016

bors-servo commented Apr 12, 2016

servo / servo

Join GitHub today

webgl: Add attribute validations and other nits #10224

webgl: Add attribute validations and other nits #10224

Conversation

emilio commented Mar 27, 2016

highfive commented Mar 27, 2016

highfive commented Mar 27, 2016

emilio commented Mar 27, 2016

emilio commented Mar 28, 2016

bors-servo commented Mar 30, 2016

pcwalton commented Mar 30, 2016

nox commented Mar 31, 2016

bors-servo commented Mar 31, 2016

jdm commented Apr 11, 2016

bors-servo commented Apr 11, 2016

bors-servo commented Apr 11, 2016

bors-servo commented Apr 11, 2016

emilio commented Apr 11, 2016

bors-servo commented Apr 11, 2016

bors-servo commented Apr 12, 2016