-
Notifications
You must be signed in to change notification settings - Fork 23
Add glwindow implementation, that renders to a GlWindow #66
Conversation
The matching servo branch is at https://github.com/asajeffrey/servo/tree/webvr-glwindow |
cc @paulrouget @Manishearth and @MortimerGoro |
This is great! I'll review this soon. |
This looks good to me. r=me for most of the changes. For the blit part, I'd like someone more experienced with GL to look at it. @MortimerGoro can you look at the |
It would be good to get someone used to GL to have a look at this. |
let num_bytes = (width as usize) * (height as usize) * 4; | ||
let mut buffer = self.pool.remove().unwrap_or_else(Vec::new); | ||
buffer.resize(num_bytes, 0); | ||
gl.read_pixels_into_buffer( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we share the data in a more performant way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd need to have GL context sharing, which got added to glutin after this PR was submitted (rust-windowing/glutin#899 (comment)). Context sharing is still not implemented on macOS. This implementation makes all platforms use a slow path, to get other platforms onto the fast path, we'd have to recreate the interface that webrender has for images that are either implemented using shared textures or using readback. I'm not sure how to do that without making rust-webvr
depend on webrender_api
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec issue for this is immersive-web/webxr#528. The XR spec pretty much requires textures to go via main memory in the case that the browser doesn't share GL textures with the VR display.
@MortimerGoro Are you okay landing this and filing an issue about supporting texture sharing? Another issue is that running this on Linux the frame rate is half what you'd expect, my suspicion is that on Linux vsync is always enabled, so both |
@asajeffrey yes, we can work on the texture sharing as a follow-up |
@bors-servo r=paulrouget |
📌 Commit b61b1e5 has been approved by |
☀️ Test successful - checks-travis |
Use a test VRDisplay that renders to a GL window <!-- Please describe your changes on the following line: --> Add a `dom.webvr.test` pref that registers a new VR display, which registers to a GL window. The matching webvr PR is servo/rust-webvr#66. --- <!-- 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 #22795 - [X] These changes do not require tests because we need a followup PR to support reftests which use the VR display <!-- 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. --> <!-- 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/22953) <!-- Reviewable:end -->
Use a test VRDisplay that renders to a GL window <!-- Please describe your changes on the following line: --> Add a `dom.webvr.test` pref that registers a new VR display, which registers to a GL window. The matching webvr PR is servo/rust-webvr#66. --- <!-- 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 #22795 - [X] These changes do not require tests because we need a followup PR to support reftests which use the VR display <!-- 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. --> <!-- 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/22953) <!-- Reviewable:end -->
Use a test VRDisplay that renders to a GL window <!-- Please describe your changes on the following line: --> Add a `dom.webvr.test` pref that registers a new VR display, which registers to a GL window. The matching webvr PR is servo/rust-webvr#66. --- <!-- 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 #22795 - [X] These changes do not require tests because we need a followup PR to support reftests which use the VR display <!-- 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. --> <!-- 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/22953) <!-- Reviewable:end -->
Use a test VRDisplay that renders to a GL window <!-- Please describe your changes on the following line: --> Add a `dom.webvr.test` pref that registers a new VR display, which registers to a GL window. The matching webvr PR is servo/rust-webvr#66. --- <!-- 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 #22795 - [X] These changes do not require tests because we need a followup PR to support reftests which use the VR display <!-- 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. --> <!-- 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/22953) <!-- Reviewable:end -->
Add glwindow implementation, that renders to a GlWindow, mainly for testing, but also this forms a basis for VRDisplays that have code that needs to run in the main thread.
Fixes #52 and #65.