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

6 files changed, 124 insertions, 11 deletions

diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs
index 91b4c119999..3e7486ed8ca 100644
--- a/components/script/dom/bindings/trace.rs
+++ b/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);
diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs
index 114ede38b01..3a31e99be33 100644
--- a/components/script/dom/webglframebuffer.rs
+++ b/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();
         });
diff --git a/components/script/dom/webglrenderbuffer.rs b/components/script/dom/webglrenderbuffer.rs
index f210b579e88..07afdbccb44 100644
--- a/components/script/dom/webglrenderbuffer.rs
+++ b/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 {
diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs
index e50d70cb916..33aaaf603d4 100644
--- a/components/script/dom/webglrenderingcontext.rs
+++ b/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),
diff --git a/components/script/dom/webgltexture.rs b/components/script/dom/webgltexture.rs
index a7e548dcdef..e4a18b519a3 100644
--- a/components/script/dom/webgltexture.rs
+++ b/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 {
diff --git a/components/script/dom/xrwebgllayer.rs b/components/script/dom/xrwebgllayer.rs
index a1e912252e9..dabff8fc947 100644
--- a/components/script/dom/xrwebgllayer.rs
+++ b/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),
         );