Skip to content

Commit dcc5ae9

Browse files
author
bors-servo
authored
Auto merge of #23848 - asajeffrey:webxr-moar-channels, r=Manishearth
Replace use of callbacks in webxr by channels <!-- Please describe your changes on the following line: --> Use senders rather than callbacks in webxr. --- <!-- 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 do not require tests because it's an internal change <!-- 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/23848) <!-- Reviewable:end -->
2 parents f78dd61 + 133a17e commit dcc5ae9

File tree

11 files changed

+109
-190
lines changed

11 files changed

+109
-190
lines changed

‎Cargo.lock‎

Lines changed: 15 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,7 @@ opt-level = 3
2929
mio = { git = "https://github.com/servo/mio.git", branch = "servo" }
3030
iovec = { git = "https://github.com/servo/iovec.git", branch = "servo" }
3131
cmake = { git = "https://github.com/alexcrichton/cmake-rs" }
32+
33+
[patch."https://github.com/servo/webxr"]
34+
webxr = { git = "https://github.com/asajeffrey/webxr", branch = "optional-glsync" }
35+
webxr-api = { git = "https://github.com/asajeffrey/webxr", branch = "optional-glsync" }

‎components/canvas/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,4 @@ servo_config = {path = "../config"}
3737
webrender = {git = "https://github.com/servo/webrender"}
3838
webrender_api = {git = "https://github.com/servo/webrender", features = ["ipc"]}
3939
webrender_traits = {path = "../webrender_traits"}
40+
webxr-api = {git = "https://github.com/servo/webxr", features = ["ipc"]}

‎components/canvas/webgl_mode/inprocess.rs‎

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
/* This Source Code Form is subject to the terms of the Mozilla Public
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
4-
54
use crate::gl_context::GLContextFactory;
65
use crate::webgl_thread::{WebGLMainThread, WebGLThread, WebGLThreadInit};
76
use canvas_traits::webgl::webgl_channel;
@@ -17,6 +16,7 @@ use std::default::Default;
1716
use std::rc::Rc;
1817
use std::sync::{Arc, Mutex};
1918
use webrender_traits::{WebrenderExternalImageApi, WebrenderExternalImageRegistry};
19+
use webxr_api::WebGLExternalImageApi;
2020

2121
/// WebGL Threading API entry point that lives in the constellation.
2222
pub struct WebGLThreads(WebGLSender<WebGLMsg>);
@@ -38,6 +38,7 @@ impl WebGLThreads {
3838
) -> (
3939
WebGLThreads,
4040
Option<Rc<WebGLMainThread>>,
41+
Box<dyn webxr_api::WebGLExternalImageApi>,
4142
Box<dyn WebrenderExternalImageApi>,
4243
Option<Box<dyn webrender::OutputImageHandler>>,
4344
) {
@@ -77,6 +78,7 @@ impl WebGLThreads {
7778
(
7879
WebGLThreads(sender),
7980
webgl_thread,
81+
external.sendable.clone_box(),
8082
Box::new(external),
8183
output_handler.map(|b| b as Box<_>),
8284
)
@@ -96,9 +98,8 @@ impl WebGLThreads {
9698
}
9799
}
98100

99-
/// Bridge between the webrender::ExternalImage callbacks and the WebGLThreads.
100-
struct WebGLExternalImages {
101-
webrender_gl: Rc<dyn gl::Gl>,
101+
/// Bridge between the webxr_api::ExternalImage callbacks and the WebGLThreads.
102+
struct SendableWebGLExternalImages {
102103
webgl_channel: WebGLSender<WebGLMsg>,
103104
// Used to avoid creating a new channel on each received WebRender request.
104105
lock_channel: (
@@ -107,24 +108,26 @@ struct WebGLExternalImages {
107108
),
108109
}
109110

110-
impl WebGLExternalImages {
111-
fn new(webrender_gl: Rc<dyn gl::Gl>, channel: WebGLSender<WebGLMsg>) -> Self {
112-
WebGLExternalImages {
113-
webrender_gl,
111+
impl SendableWebGLExternalImages {
112+
fn new(channel: WebGLSender<WebGLMsg>) -> Self {
113+
Self {
114114
webgl_channel: channel,
115115
lock_channel: webgl_channel().unwrap(),
116116
}
117117
}
118118
}
119119

120-
impl WebrenderExternalImageApi for WebGLExternalImages {
121-
fn lock(&mut self, id: u64) -> (u32, Size2D<i32>) {
120+
impl webxr_api::WebGLExternalImageApi for SendableWebGLExternalImages {
121+
fn lock(&self, id: usize) -> (u32, Size2D<i32>, Option<gl::GLsync>) {
122122
if let Some(main_thread) = WebGLMainThread::on_current_thread() {
123123
// If we're on the same thread as WebGL, we can get the data directly
124-
main_thread
124+
let (image_id, size) = main_thread
125125
.thread_data
126126
.borrow_mut()
127-
.handle_lock_unsync(WebGLContextId(id as usize))
127+
.handle_lock_unsync(WebGLContextId(id as usize));
128+
// We don't need a GLsync object if we're running on the main thread
129+
// Might be better to return an option?
130+
(image_id, size, None)
128131
} else {
129132
// WebGL Thread has it's own GL command queue that we need to synchronize with the WR GL command queue.
130133
// The WebGLMsg::Lock message inserts a fence in the WebGL command queue.
@@ -135,16 +138,11 @@ impl WebrenderExternalImageApi for WebGLExternalImages {
135138
))
136139
.unwrap();
137140
let (image_id, size, gl_sync) = self.lock_channel.1.recv().unwrap();
138-
// The next glWaitSync call is run on the WR thread and it's used to synchronize the two
139-
// flows of OpenGL commands in order to avoid WR using a semi-ready WebGL texture.
140-
// glWaitSync doesn't block WR thread, it affects only internal OpenGL subsystem.
141-
self.webrender_gl
142-
.wait_sync(gl_sync as gl::GLsync, 0, gl::TIMEOUT_IGNORED);
143-
(image_id, size)
141+
(image_id, size, Some(gl_sync as gl::GLsync))
144142
}
145143
}
146144

147-
fn unlock(&mut self, id: u64) {
145+
fn unlock(&self, id: usize) {
148146
if let Some(main_thread) = WebGLMainThread::on_current_thread() {
149147
// If we're on the same thread as WebGL, we can unlock directly
150148
main_thread
@@ -157,6 +155,42 @@ impl WebrenderExternalImageApi for WebGLExternalImages {
157155
.unwrap()
158156
}
159157
}
158+
159+
fn clone_box(&self) -> Box<dyn webxr_api::WebGLExternalImageApi> {
160+
Box::new(Self::new(self.webgl_channel.clone()))
161+
}
162+
}
163+
164+
/// Bridge between the webrender::ExternalImage callbacks and the WebGLThreads.
165+
struct WebGLExternalImages {
166+
webrender_gl: Rc<dyn gl::Gl>,
167+
sendable: SendableWebGLExternalImages,
168+
}
169+
170+
impl WebGLExternalImages {
171+
fn new(webrender_gl: Rc<dyn gl::Gl>, channel: WebGLSender<WebGLMsg>) -> Self {
172+
Self {
173+
webrender_gl,
174+
sendable: SendableWebGLExternalImages::new(channel),
175+
}
176+
}
177+
}
178+
179+
impl WebrenderExternalImageApi for WebGLExternalImages {
180+
fn lock(&mut self, id: u64) -> (u32, Size2D<i32>) {
181+
let (image_id, size, gl_sync) = self.sendable.lock(id as usize);
182+
// The next glWaitSync call is run on the WR thread and it's used to synchronize the two
183+
// flows of OpenGL commands in order to avoid WR using a semi-ready WebGL texture.
184+
// glWaitSync doesn't block WR thread, it affects only internal OpenGL subsystem.
185+
if let Some(gl_sync) = gl_sync {
186+
self.webrender_gl.wait_sync(gl_sync, 0, gl::TIMEOUT_IGNORED);
187+
}
188+
(image_id, size)
189+
}
190+
191+
fn unlock(&mut self, id: u64) {
192+
self.sendable.unlock(id as usize);
193+
}
160194
}
161195

162196
/// struct used to implement DOMToTexture feature and webrender::OutputImageHandler trait.

‎components/canvas/webgl_thread.rs‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use euclid::default::Size2D;
1010
use fnv::FnvHashMap;
1111
use gleam::gl;
1212
use half::f16;
13-
use ipc_channel::ipc::{self, IpcSender, OpaqueIpcMessage};
13+
use ipc_channel::ipc::{self, OpaqueIpcMessage};
1414
use ipc_channel::router::ROUTER;
1515
use offscreen_gl_context::{DrawBuffer, GLContext, NativeGLContextMethods};
1616
use pixels::{self, PixelFormat};
@@ -284,9 +284,6 @@ impl WebGLThread {
284284
WebGLMsg::Lock(ctx_id, sender) => {
285285
self.handle_lock(ctx_id, sender);
286286
},
287-
WebGLMsg::LockIPC(ctx_id, sender) => {
288-
self.handle_lock_ipc(ctx_id, sender);
289-
},
290287
WebGLMsg::Unlock(ctx_id) => {
291288
self.handle_unlock(ctx_id);
292289
},
@@ -343,21 +340,6 @@ impl WebGLThread {
343340
context_id: WebGLContextId,
344341
sender: WebGLSender<(u32, Size2D<i32>, usize)>,
345342
) {
346-
sender.send(self.handle_lock_sync(context_id)).unwrap();
347-
}
348-
349-
/// handle_lock, but unconditionally IPC (used by webxr)
350-
fn handle_lock_ipc(
351-
&mut self,
352-
context_id: WebGLContextId,
353-
sender: IpcSender<(u32, Size2D<i32>, usize)>,
354-
) {
355-
sender.send(self.handle_lock_sync(context_id)).unwrap();
356-
}
357-
358-
/// Shared code between handle_lock and handle_lock_ipc, does the actual syncing/flushing
359-
/// but the caller must send the response back
360-
fn handle_lock_sync(&mut self, context_id: WebGLContextId) -> (u32, Size2D<i32>, usize) {
361343
let data =
362344
Self::make_current_if_needed(context_id, &self.contexts, &mut self.bound_context_id)
363345
.expect("WebGLContext not found in a WebGLMsg::Lock message");
@@ -374,7 +356,7 @@ impl WebGLThread {
374356
data.ctx.gl().flush();
375357
debug_assert!(data.ctx.gl().get_error() == gl::NO_ERROR);
376358

377-
(info.texture_id, info.size, gl_sync as usize)
359+
let _ = sender.send((info.texture_id, info.size, gl_sync as usize));
378360
}
379361

380362
/// A version of locking that doesn't return a GLsync object,

‎components/canvas_traits/webgl.rs‎

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@
44

55
use euclid::default::{Rect, Size2D};
66
use gleam::gl;
7-
use gleam::gl::GLsync;
8-
use gleam::gl::GLuint;
97
use gleam::gl::Gl;
10-
use ipc_channel::ipc::{self, IpcBytesReceiver, IpcBytesSender, IpcSender, IpcSharedMemory};
8+
use ipc_channel::ipc::{IpcBytesReceiver, IpcBytesSender, IpcSharedMemory};
119
use pixels::PixelFormat;
1210
use std::borrow::Cow;
1311
use std::fmt;
@@ -61,8 +59,6 @@ pub enum WebGLMsg {
6159
/// The WR client should not change the shared texture content until the Unlock call.
6260
/// Currently OpenGL Sync Objects are used to implement the synchronization mechanism.
6361
Lock(WebGLContextId, WebGLSender<(u32, Size2D<i32>, usize)>),
64-
/// Lock(), but unconditionally IPC (used by webxr)
65-
LockIPC(WebGLContextId, IpcSender<(u32, Size2D<i32>, usize)>),
6662
/// Unlocks a specific WebGLContext. Unlock messages are used for a correct synchronization
6763
/// with WebRender external image API.
6864
/// The WR unlocks a context when it finished reading the shared texture contents.
@@ -185,39 +181,6 @@ impl WebGLMsgSender {
185181
pub fn send_dom_to_texture(&self, command: DOMToTextureCommand) -> WebGLSendResult {
186182
self.sender.send(WebGLMsg::DOMToTextureCommand(command))
187183
}
188-
189-
pub fn webxr_external_image_api(&self) -> impl webxr_api::WebGLExternalImageApi {
190-
SerializableWebGLMsgSender {
191-
ctx_id: self.ctx_id,
192-
sender: self.sender.to_ipc(),
193-
}
194-
}
195-
}
196-
197-
// WegGLMsgSender isn't actually serializable, despite what it claims.
198-
#[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)]
199-
struct SerializableWebGLMsgSender {
200-
ctx_id: WebGLContextId,
201-
#[ignore_malloc_size_of = "channels are hard"]
202-
sender: IpcSender<WebGLMsg>,
203-
}
204-
205-
#[typetag::serde]
206-
impl webxr_api::WebGLExternalImageApi for SerializableWebGLMsgSender {
207-
fn lock(&self) -> Result<(GLuint, Size2D<i32>, GLsync), webxr_api::Error> {
208-
let (sender, receiver) = ipc::channel().or(Err(webxr_api::Error::CommunicationError))?;
209-
self.sender
210-
.send(WebGLMsg::LockIPC(self.ctx_id, sender))
211-
.or(Err(webxr_api::Error::CommunicationError))?;
212-
let (texture, size, sync) = receiver
213-
.recv()
214-
.or(Err(webxr_api::Error::CommunicationError))?;
215-
Ok((texture, size, sync as GLsync))
216-
}
217-
218-
fn unlock(&self) {
219-
let _ = self.sender.send(WebGLMsg::Unlock(self.ctx_id));
220-
}
221184
}
222185

223186
#[derive(Deserialize, Serialize)]

0 commit comments

Comments
 (0)