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

Add support for WebGL2 framebuffer invalidation #25798

Merged

Conversation

@mmatyas
Copy link
Contributor

mmatyas commented Feb 19, 2020

Adds support for the invalidateFramebuffer and invalideSubFramebuffer WebGL2 calls.

See: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.4

Depends on #25785.

cc @jdm @zakorgy


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes
@highfive
Copy link

highfive commented Feb 19, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webglrenderingcontext.rs, components/script/dom/webidls/WebGL2RenderingContext.webidl, components/script/dom/webglframebuffer.rs, components/script/dom/webgl2renderingcontext.rs, components/script/dom/webglrenderbuffer.rs
  • @jgraham: tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-test.html.ini, tests/wpt/webgl/meta/conformance2/context/methods-2.html.ini, tests/wpt/webgl/meta/conformance2/rendering/framebuffer-texture-level1.html.ini, tests/wpt/webgl/meta/conformance2/rendering/framebuffer-completeness-unaffected.html.ini, tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-object-attachment.html.ini and 3 more
  • @KiChjang: components/script/dom/webglrenderingcontext.rs, components/script/dom/webidls/WebGL2RenderingContext.webidl, components/script/dom/webglframebuffer.rs, components/script/dom/webgl2renderingcontext.rs, components/script/dom/webglrenderbuffer.rs
@highfive
Copy link

highfive commented Feb 19, 2020

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@jdm
jdm approved these changes Feb 19, 2020
@jdm jdm assigned jdm and unassigned paulrouget Feb 19, 2020
@mmatyas mmatyas mentioned this pull request Feb 20, 2020
3 of 3 tasks complete
@bors-servo
Copy link
Contributor

bors-servo commented Feb 21, 2020

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

@jdm
Copy link
Member

jdm commented Feb 21, 2020

This can now merge after a rebase.

@mmatyas mmatyas force-pushed the szeged:mmatyas__webgl_fns_framebuf_invalidate branch from 8ca2c42 to 7d44c86 Feb 25, 2020
@mmatyas
Copy link
Contributor Author

mmatyas commented Feb 25, 2020

Rebased and updated the tests.

@jdm
Copy link
Member

jdm commented Feb 25, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Feb 25, 2020

📌 Commit 7d44c86 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Feb 25, 2020

Testing commit 7d44c86 with merge e6a82e4...

bors-servo added a commit that referenced this pull request Feb 25, 2020
… r=jdm

Add support for WebGL2 framebuffer invalidation

Adds support for the `invalidateFramebuffer` and `invalideSubFramebuffer` WebGL2 calls.

See: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.4

<!-- Please describe your changes on the following line: -->

Depends on #25785.

cc @jdm @zakorgy

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 25, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Feb 25, 2020

  ▶ Unexpected subtest result in /_webgl/conformance2/state/gl-object-get-calls.html:
  │ FAIL [expected PASS] WebGL test #182: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).
  │   → assert_true: successfullyParsed should be true (of type boolean). Was undefined (of type undefined). expected true got false
  │ 
  │ reportTestResultsToHarness/<@http://web-platform.test:8000/_webgl/js/js-test-pre.js:123:9
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1867:25
  │ test@http://web-platform.test:8000/resources/testharness.js:548:30
  │ reportTestResultsToHarness@http://web-platform.test:8000/_webgl/js/js-test-pre.js:122:7
  │ testFailed@http://web-platform.test:8000/_webgl/js/js-test-pre.js:249:5
  │ shouldBe@http://web-platform.test:8000/_webgl/js/js-test-pre.js:406:9
  │ shouldBeTrue@http://web-platform.test:8000/_webgl/js/js-test-pre.js:432:29
  └ @http://web-platform.test:8000/_webgl/js/js-test-post.js:24:1
@mmatyas
Copy link
Contributor Author

mmatyas commented Feb 27, 2020

Hmm I've tried to rebuild and regenerate the file, but it produces the same results for me every time.

@jdm
Copy link
Member

jdm commented Feb 27, 2020

Have you run the test with --headless?

@bors-servo
Copy link
Contributor

bors-servo commented Mar 2, 2020

Trying commit 143013b with merge 2755c3e...

bors-servo added a commit that referenced this pull request Mar 2, 2020
… r=<try>

Add support for WebGL2 framebuffer invalidation

Adds support for the `invalidateFramebuffer` and `invalideSubFramebuffer` WebGL2 calls.

See: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.4

<!-- Please describe your changes on the following line: -->

Depends on #25785.

cc @jdm @zakorgy

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 2, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Mar 2, 2020

  ▶ Unexpected subtest result in /_webgl/conformance2/state/gl-object-get-calls.html:
  │ FAIL [expected PASS] WebGL test #182: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).
  │   → assert_true: successfullyParsed should be true (of type boolean). Was undefined (of type undefined). expected true got false
  │ 
  │ reportTestResultsToHarness/<@http://web-platform.test:8000/_webgl/js/js-test-pre.js:123:9
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1926:25
  │ test@http://web-platform.test:8000/resources/testharness.js:548:30
  │ reportTestResultsToHarness@http://web-platform.test:8000/_webgl/js/js-test-pre.js:122:7
  │ testFailed@http://web-platform.test:8000/_webgl/js/js-test-pre.js:249:5
  │ shouldBe@http://web-platform.test:8000/_webgl/js/js-test-pre.js:406:9
  │ shouldBeTrue@http://web-platform.test:8000/_webgl/js/js-test-pre.js:432:29
  └ @http://web-platform.test:8000/_webgl/js/js-test-post.js:24:1

Weird.

@mmatyas
Copy link
Contributor Author

mmatyas commented Mar 3, 2020

Hm. I wonder if not touching that file at all would fix the issue?

@jdm
Copy link
Member

jdm commented Mar 3, 2020

Doubtful. Just add the missing failure by hand so it can merge?

Adds support for the `invalidateFramebuffer` and `invalideSubFramebuffer`
WebGL2 calls.

See: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.4
@mmatyas mmatyas force-pushed the szeged:mmatyas__webgl_fns_framebuf_invalidate branch from 143013b to cc07d93 Mar 3, 2020
@highfive highfive removed the S-tests-failed label Mar 3, 2020
@mmatyas
Copy link
Contributor Author

mmatyas commented Mar 3, 2020

Ok, this should work then I guess.

@jdm
Copy link
Member

jdm commented Mar 3, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Mar 3, 2020

📌 Commit cc07d93 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Mar 3, 2020

Testing commit cc07d93 with merge e0d8b5f...

bors-servo added a commit that referenced this pull request Mar 3, 2020
… r=jdm

Add support for WebGL2 framebuffer invalidation

Adds support for the `invalidateFramebuffer` and `invalideSubFramebuffer` WebGL2 calls.

See: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.4

<!-- Please describe your changes on the following line: -->

Depends on #25785.

cc @jdm @zakorgy

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 3, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Mar 3, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Mar 3, 2020

Testing commit cc07d93 with merge 56b5d9d...

@bors-servo
Copy link
Contributor

bors-servo commented Mar 3, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 56b5d9d to master...

@bors-servo bors-servo merged commit 56b5d9d into servo:master Mar 3, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
bors-servo added a commit that referenced this pull request Mar 4, 2020
…=jdm

Add support for WebGL2 FramebufferTextureLayer

Adds support for `FramebufferTextureLayer` WebGL2 call.

See: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.4

<!-- Please describe your changes on the following line: -->

Depends on #25798.

cc @jdm @zakorgy

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this pull request Mar 4, 2020
…=jdm

Add support for WebGL2 FramebufferTextureLayer

Adds support for `FramebufferTextureLayer` WebGL2 call.

See: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.4

<!-- Please describe your changes on the following line: -->

Depends on #25798.

cc @jdm @zakorgy

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
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.

None yet

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