Skip to content

Commit 503bb10

Browse files
sagudevsagudev
authored
Use surfman with glow bindings (#34328)
* Use glowing surfman Signed-off-by: sagudev <[email protected]> * Port servo/webxr#255 Signed-off-by: sagudev <[email protected]> * fixups rebase Signed-off-by: sagudev <[email protected]> * fmt Signed-off-by: sagudev <[email protected]> * Update surfman Signed-off-by: sagudev <[email protected]> * Fix stale TODO Signed-off-by: sagudev <[email protected]> --------- Signed-off-by: sagudev <[email protected]>
1 parent fdbfecf commit 503bb10

File tree

13 files changed

+70
-97
lines changed

13 files changed

+70
-97
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ style_config = { git = "https://github.com/servo/stylo", branch = "2025-02-03" }
133133
style_dom = { git = "https://github.com/servo/stylo", package = "dom", branch = "2025-02-03" }
134134
style_malloc_size_of = { package = "malloc_size_of", git = "https://github.com/servo/stylo", branch = "2025-02-03", features = ["servo"] }
135135
style_traits = { git = "https://github.com/servo/stylo", branch = "2025-02-03", features = ["servo"] }
136-
surfman = { git = "https://github.com/servo/surfman", rev = "300789ddbda45c89e9165c31118bf1c4c07f89f6", features = ["chains"] }
136+
surfman = { git = "https://github.com/servo/surfman", rev = "a8079ee2708619926d07c5a2088b8c9af99abdb3", features = ["chains"] }
137137
syn = { version = "2", default-features = false, features = ["clone-impls", "derive", "parsing"] }
138138
synstructure = "0.13"
139139
taffy = { version = "0.7.5", default-features = false, features = ["detailed_layout_info", "grid", "serde", "std"] }

‎components/canvas/webgl_thread.rs‎

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ use euclid::default::Size2D;
2626
use fnv::FnvHashMap;
2727
use glow::{
2828
self as gl, bytes_per_type, components_per_format, ActiveTransformFeedback, Context as Gl,
29-
HasContext, NativeFramebuffer, NativeTransformFeedback, NativeUniformLocation,
30-
NativeVertexArray, PixelUnpackData, ShaderPrecisionFormat,
29+
HasContext, NativeTransformFeedback, NativeUniformLocation, NativeVertexArray, PixelUnpackData,
30+
ShaderPrecisionFormat,
3131
};
3232
use half::f16;
3333
use log::{debug, error, trace, warn};
@@ -49,7 +49,6 @@ use crate::webgl_limits::GLLimitsDetect;
4949
#[cfg(feature = "webxr")]
5050
use crate::webxr::{WebXRBridge, WebXRBridgeContexts, WebXRBridgeInit};
5151

52-
type GLuint = u32;
5352
type GLint = i32;
5453

5554
fn native_uniform_location(location: i32) -> Option<NativeUniformLocation> {
@@ -606,10 +605,7 @@ impl WebGLThread {
606605
.framebuffer_object;
607606

608607
unsafe {
609-
gl.bind_framebuffer(
610-
gl::FRAMEBUFFER,
611-
NonZeroU32::new(framebuffer).map(NativeFramebuffer),
612-
);
608+
gl.bind_framebuffer(gl::FRAMEBUFFER, framebuffer);
613609
gl.viewport(0, 0, size.width as i32, size.height as i32);
614610
gl.scissor(0, 0, size.width as i32, size.height as i32);
615611
gl.clear_color(0., 0., 0., !has_alpha as u32 as f32);
@@ -839,7 +835,7 @@ impl WebGLThread {
839835
.unwrap()
840836
.unwrap();
841837
debug!(
842-
"... rebound framebuffer {}, new back buffer surface is {:?}",
838+
"... rebound framebuffer {:?}, new back buffer surface is {:?}",
843839
framebuffer_object, id
844840
);
845841

@@ -2698,14 +2694,13 @@ impl WebGLImpl {
26982694
) {
26992695
let id = match request {
27002696
WebGLFramebufferBindingRequest::Explicit(id) => Some(id.glow()),
2701-
WebGLFramebufferBindingRequest::Default => NonZeroU32::new(
2697+
WebGLFramebufferBindingRequest::Default => {
27022698
device
27032699
.context_surface_info(ctx)
27042700
.unwrap()
27052701
.expect("No surface attached!")
2706-
.framebuffer_object,
2707-
)
2708-
.map(NativeFramebuffer),
2702+
.framebuffer_object
2703+
},
27092704
};
27102705

27112706
debug!("WebGLImpl::bind_framebuffer: {:?}", id);
@@ -3188,9 +3183,8 @@ struct FramebufferRebindingInfo {
31883183
impl FramebufferRebindingInfo {
31893184
fn detect(device: &Device, context: &Context, gl: &Gl) -> FramebufferRebindingInfo {
31903185
unsafe {
3191-
let (mut read_framebuffer, mut draw_framebuffer) = ([0], [0]);
3192-
gl.get_parameter_i32_slice(gl::READ_FRAMEBUFFER_BINDING, &mut read_framebuffer);
3193-
gl.get_parameter_i32_slice(gl::DRAW_FRAMEBUFFER_BINDING, &mut draw_framebuffer);
3186+
let read_framebuffer = gl.get_parameter_framebuffer(gl::READ_FRAMEBUFFER_BINDING);
3187+
let draw_framebuffer = gl.get_parameter_framebuffer(gl::DRAW_FRAMEBUFFER_BINDING);
31943188

31953189
let context_surface_framebuffer = device
31963190
.context_surface_info(context)
@@ -3199,10 +3193,10 @@ impl FramebufferRebindingInfo {
31993193
.framebuffer_object;
32003194

32013195
let mut flags = FramebufferRebindingFlags::empty();
3202-
if context_surface_framebuffer == read_framebuffer[0] as GLuint {
3196+
if context_surface_framebuffer == read_framebuffer {
32033197
flags.insert(FramebufferRebindingFlags::REBIND_READ_FRAMEBUFFER);
32043198
}
3205-
if context_surface_framebuffer == draw_framebuffer[0] as GLuint {
3199+
if context_surface_framebuffer == draw_framebuffer {
32063200
flags.insert(FramebufferRebindingFlags::REBIND_DRAW_FRAMEBUFFER);
32073201
}
32083202

@@ -3227,23 +3221,13 @@ impl FramebufferRebindingInfo {
32273221
.flags
32283222
.contains(FramebufferRebindingFlags::REBIND_READ_FRAMEBUFFER)
32293223
{
3230-
unsafe {
3231-
gl.bind_framebuffer(
3232-
gl::READ_FRAMEBUFFER,
3233-
NonZeroU32::new(context_surface_framebuffer).map(NativeFramebuffer),
3234-
)
3235-
};
3224+
unsafe { gl.bind_framebuffer(gl::READ_FRAMEBUFFER, context_surface_framebuffer) };
32363225
}
32373226
if self
32383227
.flags
32393228
.contains(FramebufferRebindingFlags::REBIND_DRAW_FRAMEBUFFER)
32403229
{
3241-
unsafe {
3242-
gl.bind_framebuffer(
3243-
gl::DRAW_FRAMEBUFFER,
3244-
NonZeroU32::new(context_surface_framebuffer).map(NativeFramebuffer),
3245-
)
3246-
};
3230+
unsafe { gl.bind_framebuffer(gl::DRAW_FRAMEBUFFER, context_surface_framebuffer) };
32473231
}
32483232

32493233
unsafe {

‎components/canvas/webxr.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
44

55
use std::collections::HashMap;
6+
use std::num::NonZeroU32;
67

78
use canvas_traits::webgl::{
89
webgl_channel, WebGLContextId, WebGLMsg, WebGLSender, WebXRCommand, WebXRLayerManagerId,
@@ -25,7 +26,7 @@ use crate::webgl_thread::{GLContextData, WebGLThread};
2526
pub(crate) struct WebXRBridge {
2627
factory_receiver: crossbeam_channel::Receiver<WebXRLayerManagerFactory<WebXRSurfman>>,
2728
managers: HashMap<WebXRLayerManagerId, Box<dyn WebXRLayerManagerAPI<WebXRSurfman>>>,
28-
next_manager_id: u32,
29+
next_manager_id: NonZeroU32,
2930
}
3031

3132
impl WebXRBridge {
@@ -34,7 +35,7 @@ impl WebXRBridge {
3435
factory_receiver, ..
3536
} = init;
3637
let managers = HashMap::new();
37-
let next_manager_id = 1;
38+
let next_manager_id = NonZeroU32::MIN;
3839
WebXRBridge {
3940
factory_receiver,
4041
managers,
@@ -55,8 +56,11 @@ impl WebXRBridge {
5556
.recv()
5657
.map_err(|_| WebXRError::CommunicationError)?;
5758
let manager = factory.build(device, contexts)?;
58-
let manager_id = unsafe { WebXRLayerManagerId::new(self.next_manager_id) };
59-
self.next_manager_id += 1;
59+
let manager_id = WebXRLayerManagerId::new(self.next_manager_id);
60+
self.next_manager_id = self
61+
.next_manager_id
62+
.checked_add(1)
63+
.expect("next_manager_id should not overflow");
6064
self.managers.insert(manager_id, manager);
6165
Ok(manager_id)
6266
}

‎components/script/dom/webxr/xrwebgllayer.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,7 @@ impl XRWebGLLayer {
152152
let sub_images = frame.get_sub_images(self.layer_id()?)?;
153153
let session = self.session();
154154
// TODO: Cache this texture
155-
let color_texture_id =
156-
WebGLTextureId::maybe_new(sub_images.sub_image.as_ref()?.color_texture)?;
155+
let color_texture_id = WebGLTextureId::new(sub_images.sub_image.as_ref()?.color_texture?);
157156
let color_texture = WebGLTexture::new_webxr(context, color_texture_id, session);
158157
let target = self.texture_target();
159158

@@ -187,7 +186,7 @@ impl XRWebGLLayer {
187186
.ok()?;
188187
if let Some(id) = sub_images.sub_image.as_ref()?.depth_stencil_texture {
189188
// TODO: Cache this texture
190-
let depth_stencil_texture_id = WebGLTextureId::maybe_new(id)?;
189+
let depth_stencil_texture_id = WebGLTextureId::new(id);
191190
let depth_stencil_texture =
192191
WebGLTexture::new_webxr(context, depth_stencil_texture_id, session);
193192
framebuffer

‎components/shared/canvas/webgl.rs‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -570,15 +570,9 @@ macro_rules! define_resource_id {
570570
pub struct $name(nonzero_type!($type));
571571

572572
impl $name {
573-
#[allow(unsafe_code)]
574573
#[inline]
575-
/// Create a new $name.
576-
///
577-
/// # Safety
578-
///
579-
/// Using an invalid OpenGL id may result in undefined behavior.
580-
pub unsafe fn new(id: $type) -> Self {
581-
$name(<nonzero_type!($type)>::new_unchecked(id))
574+
pub fn new(id: nonzero_type!($type)) -> Self {
575+
Self(id)
582576
}
583577

584578
#[inline]
@@ -599,10 +593,10 @@ macro_rules! define_resource_id {
599593
D: ::serde::Deserializer<'de>,
600594
{
601595
let id = <$type>::deserialize(deserializer)?;
602-
if id == 0 {
603-
Err(::serde::de::Error::custom("expected a non-zero value"))
596+
if let Some(id) = <nonzero_type!($type)>::new(id) {
597+
Ok($name(id))
604598
} else {
605-
Ok(unsafe { $name::new(id) })
599+
Err(::serde::de::Error::custom("expected a non-zero value"))
606600
}
607601
}
608602
}

‎components/shared/webrender/rendering_context.rs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,8 @@ impl RenderingContext for SurfmanRenderingContext {
135135
fn framebuffer_object(&self) -> u32 {
136136
self.context_surface_info()
137137
.unwrap_or(None)
138-
.map(|info| info.framebuffer_object)
138+
.and_then(|info| info.framebuffer_object)
139+
.map(|fbo| fbo.0.get())
139140
.unwrap_or(0)
140141
}
141142
#[allow(unsafe_code)]
@@ -187,7 +188,11 @@ impl RenderingContext for SurfmanRenderingContext {
187188
debug!("... getting texture for surface {:?}", front_buffer_id);
188189
let surface_texture = device.create_surface_texture(context, surface).unwrap();
189190
let gl_texture = device.surface_texture_object(&surface_texture);
190-
(surface_texture, gl_texture, size)
191+
(
192+
surface_texture,
193+
gl_texture.map(|tex| tex.0.get()).unwrap_or(0),
194+
size,
195+
)
191196
}
192197

193198
fn destroy_texture(&self, surface_texture: SurfaceTexture) -> Surface {
@@ -390,7 +395,10 @@ impl SurfmanRenderingContext {
390395

391396
pub fn surface_texture_object(&self, surface: &SurfaceTexture) -> u32 {
392397
let device = &self.0.device.borrow();
393-
device.surface_texture_object(surface)
398+
device
399+
.surface_texture_object(surface)
400+
.map(|t| t.0.get())
401+
.unwrap_or_default()
394402
}
395403

396404
pub fn get_proc_address(&self, name: &str) -> *const c_void {

‎components/shared/webxr/layer.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
44

55
use std::fmt::Debug;
6+
use std::num::NonZeroU32;
67
use std::sync::atomic::{AtomicUsize, Ordering};
78

89
use euclid::{Rect, Size2D};
@@ -286,9 +287,8 @@ pub struct SubImages {
286287
#[derive(Clone, Debug)]
287288
#[cfg_attr(feature = "ipc", derive(Deserialize, Serialize))]
288289
pub struct SubImage {
289-
pub color_texture: u32,
290-
// TODO: make this Option<NonZeroU32>
291-
pub depth_stencil_texture: Option<u32>,
290+
pub color_texture: Option<NonZeroU32>,
291+
pub depth_stencil_texture: Option<NonZeroU32>,
292292
pub texture_array_index: Option<u32>,
293293
pub viewport: Rect<i32, Viewport>,
294294
}

‎components/webxr/gl_utils.rs‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
44

55
use std::collections::HashMap;
6-
use std::num::NonZero;
76

87
use glow as gl;
98
use glow::{Context as Gl, HasContext};
@@ -12,10 +11,6 @@ use webxr_api::{ContextId, GLContexts, LayerId};
1211

1312
use crate::SurfmanGL;
1413

15-
pub(crate) fn framebuffer(framebuffer: u32) -> Option<gl::NativeFramebuffer> {
16-
NonZero::new(framebuffer).map(gl::NativeFramebuffer)
17-
}
18-
1914
// A utility to clear a color texture and optional depth/stencil texture
2015
pub(crate) struct GlClearer {
2116
fbos: HashMap<
@@ -52,10 +47,9 @@ impl GlClearer {
5247
.entry((layer_id, color, depth_stencil))
5348
.or_insert_with(|| {
5449
// Save the current GL state
55-
let mut bound_fbos = [0, 0];
5650
unsafe {
57-
gl.get_parameter_i32_slice(gl::DRAW_FRAMEBUFFER_BINDING, &mut bound_fbos[0..]);
58-
gl.get_parameter_i32_slice(gl::READ_FRAMEBUFFER_BINDING, &mut bound_fbos[1..]);
51+
let draw_fbo = gl.get_parameter_framebuffer(gl::DRAW_FRAMEBUFFER_BINDING);
52+
let read_fbo = gl.get_parameter_framebuffer(gl::READ_FRAMEBUFFER_BINDING);
5953

6054
// Generate and set attachments of a new FBO
6155
let fbo = gl.create_framebuffer().ok();
@@ -83,8 +77,8 @@ impl GlClearer {
8377
}
8478

8579
// Restore the GL state
86-
gl.bind_framebuffer(gl::DRAW_FRAMEBUFFER, framebuffer(bound_fbos[0] as _));
87-
gl.bind_framebuffer(gl::READ_FRAMEBUFFER, framebuffer(bound_fbos[1] as _));
80+
gl.bind_framebuffer(gl::DRAW_FRAMEBUFFER, draw_fbo);
81+
gl.bind_framebuffer(gl::READ_FRAMEBUFFER, read_fbo);
8882
debug_assert_eq!(gl.get_error(), gl::NO_ERROR);
8983

9084
fbo
@@ -110,16 +104,15 @@ impl GlClearer {
110104
let fbo = self.fbo(gl, layer_id, color, color_target, depth_stencil);
111105
unsafe {
112106
// Save the current GL state
113-
let mut bound_fbos = [0, 0];
114107
let mut clear_color = [0., 0., 0., 0.];
115108
let mut clear_depth = [0.];
116109
let mut clear_stencil = [0];
117110
let mut stencil_mask = [0];
118111
let scissor_enabled = gl.is_enabled(gl::SCISSOR_TEST);
119112
let rasterizer_enabled = gl.is_enabled(gl::RASTERIZER_DISCARD);
120113

121-
gl.get_parameter_i32_slice(gl::DRAW_FRAMEBUFFER_BINDING, &mut bound_fbos[0..]);
122-
gl.get_parameter_i32_slice(gl::READ_FRAMEBUFFER_BINDING, &mut bound_fbos[1..]);
114+
let draw_fbo = gl.get_parameter_framebuffer(gl::DRAW_FRAMEBUFFER_BINDING);
115+
let read_fbo = gl.get_parameter_framebuffer(gl::READ_FRAMEBUFFER_BINDING);
123116
gl.get_parameter_f32_slice(gl::COLOR_CLEAR_VALUE, &mut clear_color[..]);
124117
gl.get_parameter_f32_slice(gl::DEPTH_CLEAR_VALUE, &mut clear_depth[..]);
125118
gl.get_parameter_i32_slice(gl::STENCIL_CLEAR_VALUE, &mut clear_stencil[..]);
@@ -140,8 +133,8 @@ impl GlClearer {
140133
gl.clear(gl::COLOR_BUFFER_BIT | gl::DEPTH_BUFFER_BIT | gl::STENCIL_BUFFER_BIT);
141134

142135
// Restore the GL state
143-
gl.bind_framebuffer(gl::DRAW_FRAMEBUFFER, framebuffer(bound_fbos[0] as _));
144-
gl.bind_framebuffer(gl::READ_FRAMEBUFFER, framebuffer(bound_fbos[1] as _));
136+
gl.bind_framebuffer(gl::DRAW_FRAMEBUFFER, draw_fbo);
137+
gl.bind_framebuffer(gl::READ_FRAMEBUFFER, read_fbo);
145138
gl.clear_color(
146139
clear_color[0],
147140
clear_color[1],

0 commit comments

Comments
 (0)