Auto merge of #24242 - jdm:xr-webgllayer-format, r=asajeffrey,nox

Fix immersive mode panic on three.js rollercoaster on hololens

We have some special logic about texture formats when creating drawing buffers for WebGL that needs to be shared with the code that creates a separate framebuffer for immersive mode.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24083 and fix #20595.
- [x] These changes do not require tests because no CI for hololens; tested manually in the emulator.

<!-- 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/24242)
<!-- Reviewable:end -->

components/script/dom/bindings/trace.rs

@@ -45,8 +45,8 @@ use canvas_traits::canvas::{
     CanvasGradientStop, CanvasId, LinearGradientStyle, RadialGradientStyle,
 };
 use canvas_traits::canvas::{CompositionOrBlending, LineCapStyle, LineJoinStyle, RepetitionStyle};
-use canvas_traits::webgl::GLLimits;
 use canvas_traits::webgl::{ActiveAttribInfo, ActiveUniformInfo, GlType, TexDataType, TexFormat};
+use canvas_traits::webgl::{GLFormats, GLLimits};
 use canvas_traits::webgl::{WebGLBufferId, WebGLChan, WebGLContextShareMode, WebGLError};
 use canvas_traits::webgl::{WebGLFramebufferId, WebGLMsgSender, WebGLPipeline, WebGLProgramId};
 use canvas_traits::webgl::{WebGLReceiver, WebGLRenderbufferId, WebGLSLVersion, WebGLSender};
@@ -437,7 +437,7 @@ unsafe_no_jsmanaged_fields!(StorageType);
 unsafe_no_jsmanaged_fields!(CanvasGradientStop, LinearGradientStyle, RadialGradientStyle);
 unsafe_no_jsmanaged_fields!(LineCapStyle, LineJoinStyle, CompositionOrBlending);
 unsafe_no_jsmanaged_fields!(RepetitionStyle);
-unsafe_no_jsmanaged_fields!(WebGLError, GLLimits, GlType);
+unsafe_no_jsmanaged_fields!(WebGLError, GLFormats, GLLimits, GlType);
 unsafe_no_jsmanaged_fields!(TimeProfilerChan);
 unsafe_no_jsmanaged_fields!(MemProfilerChan);
 unsafe_no_jsmanaged_fields!(PseudoElement);

components/script/dom/webglframebuffer.rs

@@ -59,6 +59,15 @@ impl WebGLFramebufferAttachment {
             },
         }
     }
+
+    fn detach(&self) {
+        match self {
+            WebGLFramebufferAttachment::Renderbuffer(rb) => rb.detach_from_framebuffer(),
+            WebGLFramebufferAttachment::Texture { ref texture, .. } => {
+                texture.detach_from_framebuffer()
+            },
+        }
+    }
 }
 
 #[derive(Clone, JSTraceable, MallocSizeOf)]
@@ -161,7 +170,7 @@ impl WebGLFramebuffer {
         self.size.get()
     }
 
-    fn update_status(&self) {
+    pub fn update_status(&self) {
         let c = self.color.borrow();
         let z = self.depth.borrow();
         let s = self.stencil.borrow();
@@ -177,6 +186,7 @@ impl WebGLFramebuffer {
                 constants::RGB5_A1,
                 constants::RGB565,
                 constants::RGBA,
+                constants::RGB,
             ][..],
             &[constants::DEPTH_COMPONENT16][..],
             &[constants::STENCIL_INDEX8][..],
@@ -310,6 +320,7 @@ impl WebGLFramebuffer {
                 }
                 *binding.borrow_mut() =
                     Some(WebGLFramebufferAttachment::Renderbuffer(Dom::from_ref(rb)));
+                rb.attach_to_framebuffer(self);
                 Some(rb.id())
             },
 
@@ -339,6 +350,9 @@ impl WebGLFramebuffer {
         binding: &DomRefCell<Option<WebGLFramebufferAttachment>>,
         attachment: u32,
     ) {
+        if let Some(att) = &*binding.borrow() {
+            att.detach();
+        }
         *binding.borrow_mut() = None;
         if INTERESTING_ATTACHMENT_POINTS.contains(&attachment) {
             self.reattach_depth_stencil();
@@ -363,6 +377,7 @@ impl WebGLFramebuffer {
             let context = self.upcast::<WebGLObject>().context();
             match *attachment {
                 WebGLFramebufferAttachment::Renderbuffer(ref rb) => {
+                    rb.attach_to_framebuffer(self);
                     context.send_command(WebGLCommand::FramebufferRenderbuffer(
                         constants::FRAMEBUFFER,
                         attachment_point,
@@ -371,6 +386,7 @@ impl WebGLFramebuffer {
                     ));
                 },
                 WebGLFramebufferAttachment::Texture { ref texture, level } => {
+                    texture.attach_to_framebuffer(self);
                     context.send_command(WebGLCommand::FramebufferTexture2D(
                         constants::FRAMEBUFFER,
                         attachment_point,
@@ -463,6 +479,7 @@ impl WebGLFramebuffer {
                     texture: Dom::from_ref(texture),
                     level: level,
                 });
+                texture.attach_to_framebuffer(self);
 
                 Some(texture.id())
             },
@@ -550,6 +567,9 @@ impl WebGLFramebuffer {
         let mut depth_or_stencil_updated = false;
         self.with_matching_renderbuffers(rb, |att, name| {
             depth_or_stencil_updated |= INTERESTING_ATTACHMENT_POINTS.contains(&name);
+            if let Some(att) = &*att.borrow() {
+                att.detach();
+            }
             *att.borrow_mut() = None;
             self.update_status();
         });
@@ -563,6 +583,9 @@ impl WebGLFramebuffer {
         let mut depth_or_stencil_updated = false;
         self.with_matching_textures(texture, |att, name| {
             depth_or_stencil_updated |= INTERESTING_ATTACHMENT_POINTS.contains(&name);
+            if let Some(att) = &*att.borrow() {
+                att.detach();
+            }
             *att.borrow_mut() = None;
             self.update_status();
         });

components/script/dom/webglrenderbuffer.rs

@@ -10,7 +10,8 @@ use crate::dom::bindings::codegen::Bindings::WebGLRenderbufferBinding;
 use crate::dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants;
 use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::reflector::{reflect_dom_object, DomObject};
-use crate::dom::bindings::root::DomRoot;
+use crate::dom::bindings::root::{DomRoot, MutNullableDom};
+use crate::dom::webglframebuffer::WebGLFramebuffer;
 use crate::dom::webglobject::WebGLObject;
 use crate::dom::webglrenderingcontext::WebGLRenderingContext;
 use canvas_traits::webgl::{
@@ -28,6 +29,7 @@ pub struct WebGLRenderbuffer {
     size: Cell<Option<(i32, i32)>>,
     internal_format: Cell<Option<u32>>,
     is_initialized: Cell<bool>,
+    attached_framebuffer: MutNullableDom<WebGLFramebuffer>,
 }
 
 impl WebGLRenderbuffer {
@@ -40,6 +42,7 @@ impl WebGLRenderbuffer {
             internal_format: Cell::new(None),
             size: Cell::new(None),
             is_initialized: Cell::new(false),
+            attached_framebuffer: Default::default(),
         }
     }
 
@@ -186,6 +189,10 @@ impl WebGLRenderbuffer {
         self.internal_format.set(Some(internal_format));
         self.is_initialized.set(false);
 
+        if let Some(fb) = self.attached_framebuffer.get() {
+            fb.update_status();
+        }
+
         self.upcast::<WebGLObject>()
             .context()
             .send_command(WebGLCommand::RenderbufferStorage(
@@ -199,6 +206,14 @@ impl WebGLRenderbuffer {
 
         Ok(())
     }
+
+    pub fn attach_to_framebuffer(&self, fb: &WebGLFramebuffer) {
+        self.attached_framebuffer.set(Some(fb));
+    }
+
+    pub fn detach_from_framebuffer(&self) {
+        self.attached_framebuffer.set(None);
+    }
 }
 
 impl Drop for WebGLRenderbuffer {

components/script/dom/webglrenderingcontext.rs

@@ -53,8 +53,8 @@ use crate::script_runtime::JSContext as SafeJSContext;
 use backtrace::Backtrace;
 use canvas_traits::webgl::WebGLError::*;
 use canvas_traits::webgl::{
-    webgl_channel, AlphaTreatment, DOMToTextureCommand, GLContextAttributes, GLLimits, GlType,
-    Parameter, TexDataType, TexFormat, TexParameter, WebGLChan, WebGLCommand,
+    webgl_channel, AlphaTreatment, DOMToTextureCommand, GLContextAttributes, GLFormats, GLLimits,
+    GlType, Parameter, TexDataType, TexFormat, TexParameter, WebGLChan, WebGLCommand,
     WebGLCommandBacktrace, WebGLContextId, WebGLContextShareMode, WebGLError,
     WebGLFramebufferBindingRequest, WebGLMsg, WebGLMsgSender, WebGLProgramId, WebGLResult,
     WebGLSLVersion, WebGLSendResult, WebGLSender, WebGLVersion, WebVRCommand, YAxisTreatment,
@@ -170,6 +170,7 @@ pub struct WebGLRenderingContext {
     current_vao: MutNullableDom<WebGLVertexArrayObjectOES>,
     textures: Textures,
     api_type: GlType,
+    framebuffer_format: GLFormats,
 }
 
 impl WebGLRenderingContext {
@@ -229,6 +230,7 @@ impl WebGLRenderingContext {
                 current_vao: Default::default(),
                 textures: Textures::new(max_combined_texture_image_units),
                 api_type: ctx_data.api_type,
+                framebuffer_format: ctx_data.framebuffer_format,
             }
         })
     }
@@ -1109,6 +1111,10 @@ impl WebGLRenderingContext {
     pub fn extension_manager(&self) -> &WebGLExtensions {
         &self.extension_manager
     }
+
+    pub fn formats(&self) -> &GLFormats {
+        &self.framebuffer_format
+    }
 }
 
 #[cfg(not(feature = "webgl_backtrace"))]
@@ -1241,9 +1247,17 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
             // GL_OES_read_format support (assuming an underlying GLES
             // driver. Desktop is happy to format convert for us).
             constants::IMPLEMENTATION_COLOR_READ_FORMAT => {
+                if self.validate_framebuffer().is_err() {
+                    self.webgl_error(InvalidOperation);
+                    return NullValue();
+                }
                 return Int32Value(constants::RGBA as i32);
             },
             constants::IMPLEMENTATION_COLOR_READ_TYPE => {
+                if self.validate_framebuffer().is_err() {
+                    self.webgl_error(InvalidOperation);
+                    return NullValue();
+                }
                 return Int32Value(constants::UNSIGNED_BYTE as i32);
             },
             constants::COMPRESSED_TEXTURE_FORMATS => unsafe {
@@ -1920,6 +1934,47 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
             Err(_) => return,
         };
 
+        let framebuffer_format = match self.bound_framebuffer.get() {
+            Some(fb) => match fb.attachment(constants::COLOR_ATTACHMENT0) {
+                Some(WebGLFramebufferAttachmentRoot::Renderbuffer(rb)) => {
+                    TexFormat::from_gl_constant(rb.internal_format())
+                },
+                Some(WebGLFramebufferAttachmentRoot::Texture(texture)) => {
+                    texture.image_info_for_target(&target, 0).internal_format()
+                },
+                None => None,
+            },
+            None => {
+                let attrs = self.GetContextAttributes().unwrap();
+                Some(if attrs.alpha {
+                    TexFormat::RGBA
+                } else {
+                    TexFormat::RGB
+                })
+            },
+        };
+
+        let framebuffer_format = match framebuffer_format {
+            Some(f) => f,
+            None => {
+                self.webgl_error(InvalidOperation);
+                return;
+            },
+        };
+
+        match (framebuffer_format, internal_format) {
+            (a, b) if a == b => (),
+            (TexFormat::RGBA, TexFormat::RGB) => (),
+            (TexFormat::RGBA, TexFormat::Alpha) => (),
+            (TexFormat::RGBA, TexFormat::Luminance) => (),
+            (TexFormat::RGBA, TexFormat::LuminanceAlpha) => (),
+            (TexFormat::RGB, TexFormat::Luminance) => (),
+            _ => {
+                self.webgl_error(InvalidOperation);
+                return;
+            },
+        }
+
         // NB: TexImage2D depth is always equal to 1
         handle_potential_webgl_error!(
             self,
@@ -2819,6 +2874,8 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
         pixel_type: u32,
         mut pixels: CustomAutoRooterGuard<Option<ArrayBufferView>>,
     ) {
+        handle_potential_webgl_error!(self, self.validate_framebuffer(), return);
+
         let pixels =
             handle_potential_webgl_error!(self, pixels.as_mut().ok_or(InvalidValue), return);
 
@@ -2834,7 +2891,6 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
             return self.webgl_error(InvalidOperation);
         }
 
-        handle_potential_webgl_error!(self, self.validate_framebuffer(), return);
         let (fb_width, fb_height) = handle_potential_webgl_error!(
             self,
             self.get_current_framebuffer_size().ok_or(InvalidOperation),

components/script/dom/webgltexture.rs

@@ -10,8 +10,9 @@ use crate::dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGL
 use crate::dom::bindings::codegen::Bindings::WebGLTextureBinding;
 use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::reflector::{reflect_dom_object, DomObject};
-use crate::dom::bindings::root::DomRoot;
+use crate::dom::bindings::root::{DomRoot, MutNullableDom};
 use crate::dom::webgl_validations::types::TexImageTarget;
+use crate::dom::webglframebuffer::WebGLFramebuffer;
 use crate::dom::webglobject::WebGLObject;
 use crate::dom::webglrenderingcontext::WebGLRenderingContext;
 use canvas_traits::webgl::{webgl_channel, TexDataType, TexFormat, WebGLResult, WebGLTextureId};
@@ -48,6 +49,8 @@ pub struct WebGLTexture {
     mag_filter: Cell<u32>,
     /// True if this texture is used for the DOMToTexture feature.
     attached_to_dom: Cell<bool>,
+    /// Framebuffer that this texture is attached to.
+    attached_framebuffer: MutNullableDom<WebGLFramebuffer>,
 }
 
 impl WebGLTexture {
@@ -63,6 +66,7 @@ impl WebGLTexture {
             mag_filter: Cell::new(constants::LINEAR),
             image_info_array: DomRefCell::new([ImageInfo::new(); MAX_LEVEL_COUNT * MAX_FACE_COUNT]),
             attached_to_dom: Cell::new(false),
+            attached_framebuffer: Default::default(),
         }
     }
 
@@ -138,6 +142,11 @@ impl WebGLTexture {
 
         let face_index = self.face_index_for_target(&target);
         self.set_image_infos_at_level_and_face(level, face_index, image_info);
+
+        if let Some(fb) = self.attached_framebuffer.get() {
+            fb.update_status();
+        }
+
         Ok(())
     }
 
@@ -405,6 +414,14 @@ impl WebGLTexture {
     pub fn set_attached_to_dom(&self) {
         self.attached_to_dom.set(true);
     }
+
+    pub fn attach_to_framebuffer(&self, fb: &WebGLFramebuffer) {
+        self.attached_framebuffer.set(Some(fb));
+    }
+
+    pub fn detach_from_framebuffer(&self) {
+        self.attached_framebuffer.set(None);
+    }
 }
 
 impl Drop for WebGLTexture {

components/script/dom/xrwebgllayer.rs

@@ -110,16 +110,18 @@ impl XRWebGLLayer {
         let mut pixels = CustomAutoRooter::new(None);
         let mut clear_bits = constants::COLOR_BUFFER_BIT;
 
+        let formats = context.formats();
+
         context.BindTexture(constants::TEXTURE_2D, Some(&texture));
         let sc = context.TexImage2D(
             constants::TEXTURE_2D,
             0,
-            constants::RGBA,
+            formats.texture_format,
             resolution.width,
             resolution.height,
             0,
-            constants::RGBA,
-            constants::UNSIGNED_BYTE,
+            formats.texture_format,
+            formats.texture_type,
             pixels.root(*cx),
         );