Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use real values for GL maximums when compiling shaders. #21026

Closed
wants to merge 1 commit into from
Closed

Use real values for GL maximums when compiling shaders. #21026

wants to merge 1 commit into from

Conversation

@jdm
Copy link
Member

jdm commented Jun 8, 2018 •
edited by SimonSapin

This matches the equivalent Gecko code and makes a lot of three.js examples render as expected.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #20928.
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable
@jdm
Use real values for GL maximums when compiling shaders.
Loading status checks…
9204f6c
@highfive
Copy link

highfive commented Jun 8, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webglrenderingcontext.rs, components/script/dom/webglshader.rs
  • @fitzgen: components/script/dom/webglrenderingcontext.rs, components/script/dom/webglshader.rs
  • @KiChjang: components/script/dom/webglrenderingcontext.rs, components/script/dom/webglshader.rs
@highfive
Copy link

highfive commented Jun 8, 2018

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@jdm
Copy link
Member Author

jdm commented Jun 8, 2018

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Jun 8, 2018

⌛ Trying commit 9204f6c with merge ad0565b...
bors-servo added a commit that referenced this pull request Jun 8, 2018
@bors-servo
Auto merge of #21026 - jdm:shader-limits, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
ad0565b 
Use real values for GL maximums when compiling shaders.

This matches [the equivalent Gecko code](https://searchfox.org/mozilla-central/rev/c621276fbdd9591f52009042d959b9e19b66d49f/dom/canvas/WebGLShaderValidator.cpp#150-156) and makes a lot of three.js examples render as expected.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #20928.
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____
@bors-servo
Copy link
Contributor

bors-servo commented Jun 8, 2018

💔 Test failed - linux-rel-css
@jdm
Copy link
Member Author

jdm commented Jun 8, 2018

I'm going to need to learn some things about webGL in order to write a test for this.
@jdm jdm added S-needs-tests and removed S-tests-failed labels Jun 8, 2018
@jdm jdm mentioned this pull request Jun 19, 2018
Closed
5 of 6 tasks complete
@jdm jdm added this to In progress in WebGL content Jun 19, 2018
This was referenced Jun 19, 2018
Closed
Closed
@bors-servo
Copy link
Contributor

bors-servo commented Jul 8, 2018

☔ The latest upstream changes (presumably #21129) made this pull request unmergeable. Please resolve the merge conflicts.
@nox
Copy link
Member

nox commented Jul 9, 2018

I did this already in 0e93f06, sorry for that.
@nox nox closed this Jul 9, 2018
@jdm jdm moved this from In progress to Done in WebGL content Jul 9, 2018
@atouchet atouchet removed this from Done in WebGL content Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@emilio emilio

Labels
S-awaiting-review S-needs-rebase S-needs-tests
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

three.js clipping example has shader with too many uniforms
5 participants
@jdm @highfive @bors-servo @nox @emilio
You can’t perform that action at this time.