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

Use surfman for managing GL surfaces #24482

Merged
merged 1 commit into from
Nov 1, 2019
Merged

Conversation

asajeffrey
Copy link
Member

Replaces texture sharing with surfman surface sharing.


@highfive
Copy link

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/servo/command_base.py
  • @cbrewster: components/constellation/Cargo.toml, components/constellation/constellation.rs
  • @KiChjang: components/script/dom/webglframebuffer.rs, components/script/dom/xrwebgllayer.rs, components/script/dom/document.rs, components/script/dom/webgl2renderingcontext.rs, components/script/dom/xrsession.rs and 4 more

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 17, 2019
@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I skimmed a lot of webgl_thread.rs but overall this looks like it's in good shape.

ports/libsimpleservo/api/Cargo.toml Show resolved Hide resolved
ports/libsimpleservo/capi/Cargo.toml Show resolved Hide resolved
python/servo/command_base.py Show resolved Hide resolved
python/servo/command_base.py Show resolved Hide resolved
ports/glutin/Cargo.toml Show resolved Hide resolved
components/script/dom/webglframebuffer.rs Show resolved Hide resolved
components/canvas/webgl_thread.rs Outdated Show resolved Hide resolved
@jdm
Copy link
Member

jdm commented Oct 17, 2019

@bors-servo try

@bors-servo
Copy link
Contributor

🔒 Merge conflict

@jdm jdm 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 Oct 17, 2019
@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 Oct 17, 2019
@jdm
Copy link
Member

jdm commented Oct 17, 2019

@bors-servo try

@bors-servo
Copy link
Contributor

⌛ Trying commit e3943ea with merge a866796...

bors-servo pushed a commit that referenced this pull request Oct 17, 2019
Use surfman for managing GL surfaces

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

Replaces texture sharing with surfman surface sharing.

---
<!-- 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] These changes fix #23509  and #24256
- [x] These changes do not require tests because this is backend gfx

<!-- 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

💔 Test failed - linux-rel-wpt

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-tests-failed The changes caused existing tests to fail. labels Oct 17, 2019
@asajeffrey
Copy link
Member Author

Most comments addressed, the outstanding one is about depth/stencil/alpha.

@asajeffrey
Copy link
Member Author

Build failures are caused by the warnings caused by WebVR not being implemented.

@asajeffrey
Copy link
Member Author

@bors-servo try

@bors-servo
Copy link
Contributor

⌛ Trying commit abbf37f with merge 88c3a8d...

bors-servo pushed a commit that referenced this pull request Oct 17, 2019
Use surfman for managing GL surfaces

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

Replaces texture sharing with surfman surface sharing.

---
<!-- 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] These changes fix #23509  and #24256
- [x] These changes do not require tests because this is backend gfx

<!-- 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

💔 Test failed - linux-rel-css

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Oct 17, 2019
@asajeffrey
Copy link
Member Author

The android build is failing because something is causing a -lGL link flag to be emitted.

@asajeffrey
Copy link
Member Author

@jdm
Copy link
Member

jdm commented Oct 17, 2019

Also be aware: the WPT tests on linux had a lot of failures: https://build.servo.org/builders/linux-rel-css/builds/13780

@jdm
Copy link
Member

jdm commented Oct 17, 2019

They're all crashes of:

 │ Failed to create graphics context!: NoCurrentContext (thread main, at src/libcore/result.rs:1165)

@jdm jdm added this to In progress in UWP port Oct 17, 2019
@asajeffrey
Copy link
Member Author

Squashed. @bors-servo r=jdm

@bors-servo
Copy link
Contributor

📌 Commit a358bca has been approved by jdm

@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. S-needs-squash Some (or all) of the commits in the PR should be combined. labels Nov 1, 2019
@bors-servo
Copy link
Contributor

⌛ Testing commit a358bca with merge 162b2f5...

bors-servo pushed a commit that referenced this pull request Nov 1, 2019
Use surfman for managing GL surfaces

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

Replaces texture sharing with surfman surface sharing.

---
<!-- 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] These changes fix #23509  and #24256
- [x] These changes do not require tests because this is backend gfx

<!-- 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

💔 Test failed - status-taskcluster

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 1, 2019
@jdm
Copy link
Member

jdm commented Nov 1, 2019

@bors-servo retry force

@bors-servo
Copy link
Contributor

💣 Failed to start rebuilding: Unknown error

@bors-servo
Copy link
Contributor

⌛ Testing commit a358bca with merge 4e22889...

bors-servo pushed a commit that referenced this pull request Nov 1, 2019
Use surfman for managing GL surfaces

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

Replaces texture sharing with surfman surface sharing.

---
<!-- 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] These changes fix #23509  and #24256
- [x] These changes do not require tests because this is backend gfx

<!-- 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. -->
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Nov 1, 2019
@bors-servo
Copy link
Contributor

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: jdm
Pushing 4e22889 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
UWP port
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

7 participants