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

Issue #20593: Implement proper checks in WebGLRenderingContext's getFramebufferAttachmentParameter(). #20699

Merged
merged 1 commit into from Jun 18, 2018

Conversation

@simartin
Copy link
Contributor

simartin commented Apr 27, 2018

Add missing input checks.


  • ./mach build -d does not report any errors
  • ./mach build-geckolib does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #20593
  • There are tests for these changes

This change is Reviewable

@highfive
Copy link

highfive commented Apr 27, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webglrenderingcontext.rs
  • @fitzgen: components/script/dom/webglrenderingcontext.rs
  • @KiChjang: components/script/dom/webglrenderingcontext.rs
@gootorov
Copy link
Contributor

gootorov commented Apr 28, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Apr 28, 2018

Trying commit f3625b8 with merge 2f46c6c...

bors-servo added a commit that referenced this pull request Apr 28, 2018
Issue #20593: Implement proper checks in WebGLRenderingContext's getFramebufferAttachmentParameter().

Add missing input checks.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach build-geckolib` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #20593
- [X] There are tests for these changes

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

bors-servo commented Apr 28, 2018

💔 Test failed - linux-rel-wpt

@simartin
Copy link
Contributor Author

simartin commented May 10, 2018

Looking into those failures, they seem to be due to a spurious change that rejected DEPTH_STENCIL_ATTACHMENT. PR amended accordingly.

@simartin simartin force-pushed the simartin:issue_20593 branch from f3625b8 to c3ccd5a May 10, 2018
@simartin simartin force-pushed the simartin:issue_20593 branch from c3ccd5a to a90fc1b May 10, 2018
@jdm jdm assigned jdm and unassigned glennw May 19, 2018
@simartin
Copy link
Contributor Author

simartin commented May 26, 2018

Is there anything outstanding from me to get this one integrated? Thanks

@jdm
Copy link
Member

jdm commented May 26, 2018

No. I'm behind in my review queue, sorry.
r? @nox

@highfive highfive assigned nox and unassigned jdm May 26, 2018
@@ -0,0 +1,114 @@
<!doctype html>

This comment has been minimized.

@nox

nox May 28, 2018

Member

Nit: spaces before the doctype.

This comment has been minimized.

@simartin

simartin May 30, 2018

Author Contributor

Indeed, thanks. Fixing in next iteration.

@@ -0,0 +1,114 @@
<!doctype html>
<meta charset="utf-8">
<title>getFramebufferAttachmentParameter input type check (issue #20593)</title>

This comment has been minimized.

@nox

nox May 28, 2018

Member

@emilio What do we do for additional WebGL tests?

This comment has been minimized.

@jdm

jdm May 28, 2018

Member

What do you mean?

This comment has been minimized.

@emilio

emilio May 28, 2018

Member

You can upstream them to https://github.com/KhronosGroup/WebGL if you think they're useful.

This comment has been minimized.

@simartin

simartin May 30, 2018

Author Contributor

I don't know enough to decide whether they're useful there. Any opinion?

@nox
Copy link
Member

nox commented May 28, 2018

bors-servo added a commit that referenced this pull request May 28, 2018
Issue #20593: Implement proper checks in WebGLRenderingContext's getFramebufferAttachmentParameter().

Add missing input checks.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach build-geckolib` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #20593
- [X] There are tests for these changes

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

bors-servo commented May 28, 2018

Trying commit a90fc1b with merge da172eb...

@bors-servo
Copy link
Contributor

bors-servo commented May 28, 2018

💔 Test failed - mac-dev-unit

@simartin simartin force-pushed the simartin:issue_20593 branch from a90fc1b to 27bf459 May 30, 2018
@jdm
Copy link
Member

jdm commented May 30, 2018

@bors-servo
Copy link
Contributor

bors-servo commented May 30, 2018

Trying commit 27bf459 with merge 4151922...

bors-servo added a commit that referenced this pull request May 30, 2018
Issue #20593: Implement proper checks in WebGLRenderingContext's getFramebufferAttachmentParameter().

Add missing input checks.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach build-geckolib` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #20593
- [X] There are tests for these changes

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

jdm commented May 30, 2018

You will need to run ./mach update-manifest to ensure the manifest reflects the most recent changes to the test file.

…ramebufferAttachmentParameter().
@simartin simartin force-pushed the simartin:issue_20593 branch from 27bf459 to 08b193c May 31, 2018
@nox
Copy link
Member

nox commented Jun 4, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2018

Trying commit 08b193c with merge 5eb99b7...

bors-servo added a commit that referenced this pull request Jun 4, 2018
Issue #20593: Implement proper checks in WebGLRenderingContext's getFramebufferAttachmentParameter().

Add missing input checks.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach build-geckolib` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #20593
- [X] There are tests for these changes

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

bors-servo commented Jun 4, 2018

@simartin
Copy link
Contributor Author

simartin commented Jun 16, 2018

Hi! Is there anything outstanding from me to get this one integrated? Thanks

@jdm
Copy link
Member

jdm commented Jun 16, 2018

Review ping @nox.

@nox
Copy link
Member

nox commented Jun 18, 2018

Still the same issue as before, what do we do about the Khronos tests we change locally?

#20699 (comment)

@jdm
Copy link
Member

jdm commented Jun 18, 2018

@nox We're not changing any tests locally. Modifying the root webgl directory is fine.

@nox
Copy link
Member

nox commented Jun 18, 2018

Ok.

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Jun 18, 2018

📌 Commit 08b193c has been approved by nox

@bors-servo
Copy link
Contributor

bors-servo commented Jun 18, 2018

Testing commit 08b193c with merge ff943a3...

bors-servo added a commit that referenced this pull request Jun 18, 2018
Issue #20593: Implement proper checks in WebGLRenderingContext's getFramebufferAttachmentParameter().

Add missing input checks.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach build-geckolib` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #20593
- [X] There are tests for these changes

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

bors-servo commented Jun 18, 2018

💔 Test failed - linux-rel-wpt

@jdm
Copy link
Member

jdm commented Jun 18, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jun 18, 2018

Testing commit 08b193c with merge 1f562af...

bors-servo added a commit that referenced this pull request Jun 18, 2018
Issue #20593: Implement proper checks in WebGLRenderingContext's getFramebufferAttachmentParameter().

Add missing input checks.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach build-geckolib` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #20593
- [X] There are tests for these changes

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

bors-servo commented Jun 18, 2018

@bors-servo bors-servo merged commit 08b193c into servo:master Jun 18, 2018
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
@simartin simartin deleted the simartin:issue_20593 branch Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

8 participants
You can’t perform that action at this time.