Skip to content
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

Jump to bottom

Issue #10368: Implemented Uniform4iv and Uniform4i #10369

Merged
merged 1 commit into from Apr 3, 2016
Merged

Issue #10368: Implemented Uniform4iv and Uniform4i #10369

merged 1 commit into from Apr 3, 2016

Conversation

autrilla
Copy link
Contributor

@autrilla autrilla commented Apr 2, 2016

Uniform4iv can make use of Uniform4i, so I implemented both.

This change is Reviewable

@highfive
Copy link

highfive commented Apr 2, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/webglrenderingcontext.rs, components/script/dom/webidls/WebGLRenderingContext.webidl
  • @emilio: components/script/dom/webglrenderingcontext.rs

@highfive
Copy link

highfive commented Apr 2, 2016

warning Warning warning

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

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 2, 2016
@autrilla autrilla mentioned this pull request Apr 2, 2016
Closed
@emilio
Copy link
Member

emilio commented Apr 2, 2016

Looks good, but needs a PR to wr_traits :)

-S-awaiting-review +S-needs-code-changes

Review status: 0 of 5 files reviewed at latest revision, 1 unresolved discussion.

components/script/dom/webglrenderingcontext.rs, line 978 [r1] (raw file):
This variant isn't actually in webrender_traits, right?

Comments from Reviewable

@emilio emilio added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 2, 2016
@emilio emilio assigned emilio and unassigned Ms2ger Apr 2, 2016
@autrilla
Copy link
Contributor Author

autrilla commented Apr 2, 2016

components/script/dom/webglrenderingcontext.rs, line 978 [r1] (raw file):
afaict they are not. Should I issue a PR there too?

Comments from Reviewable

@emilio
Copy link
Member

emilio commented Apr 2, 2016

Review status: 0 of 5 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

components/script/dom/webglrenderingcontext.rs, line 978 [r1] (raw file):
Yes! If not servo won't build :P

Comments from Reviewable

@autrilla autrilla mentioned this pull request Apr 2, 2016
Merged
bors-servo pushed a commit to servo/webrender_traits that referenced this pull request Apr 2, 2016
Auto merge of #32 - autrilla:Uniform4, r=emilio
c578452 
Added Uniform4i

Blocking servo/servo#10369
@emilio
Copy link
Member

emilio commented Apr 2, 2016

Once servo/webrender_traits#32 lands, you can run ./mach cargo-update -p webrender_traits to update to the new webrender_traits, and this'll be ready to land then :)

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Apr 2, 2016
@emilio
Copy link
Member

emilio commented Apr 2, 2016

@bors-servo: r+

@bors-servo
Copy link
Contributor

📌 Commit bd1448a has been approved by emilio

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 2, 2016
bors-servo pushed a commit that referenced this pull request Apr 3, 2016
Auto merge of #10369 - autrilla:Uniform4iv, r=emilio
88d29e5 
Issue #10368: Implemented Uniform4iv and Uniform4i

Uniform4iv can make use of Uniform4i, so I implemented both.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10369)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

⌛ Testing commit bd1448a with merge 88d29e5...

@bors-servo
Copy link
Contributor

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

@bors-servo bors-servo merged commit bd1448a into servo:master Apr 3, 2016
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 3, 2016
@bors-servo bors-servo mentioned this pull request Apr 3, 2016
Merged
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
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@autrilla @highfive @emilio @bors-servo @Ms2ger