Auto merge of #21313 - jdm:glstuff, r=nox

Framebuffer and renderbuffer fixes

This commits address two separate panics that occur when running the framebuffer-object-attachment.html test. The test still panics due to another framebuffer completion status problem, so the overall test results don't demonstrate any improvement.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #21252
- [x] There are tests for these changes

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

components/script/dom/webgl_extensions/ext/extshadertexturelod.rs

@@ -6,7 +6,7 @@ use canvas_traits::webgl::WebGLVersion;
 use dom::bindings::codegen::Bindings::EXTShaderTextureLodBinding;
 use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object};
 use dom::bindings::root::DomRoot;
-use dom::webglrenderingcontext::WebGLRenderingContext;
+use dom::webglrenderingcontext::{WebGLRenderingContext, is_gles};
 use dom_struct::dom_struct;
 use super::{WebGLExtension, WebGLExtensions, WebGLExtensionSpec};
 
@@ -37,11 +37,8 @@ impl WebGLExtension for EXTShaderTextureLod {
     }
 
     fn is_supported(ext: &WebGLExtensions) -> bool {
-        if cfg!(any(target_os = "android", target_os = "ios")) {
-            return ext.supports_gl_extension("GL_EXT_shader_texture_lod");
-        }
         // This extension is always available on desktop GL.
-        true
+        !is_gles() || ext.supports_gl_extension("GL_EXT_shader_texture_lod")
     }
 
     fn enable(_ext: &WebGLExtensions) {}

components/script/dom/webgl_extensions/ext/oeselementindexuint.rs

@@ -6,7 +6,7 @@ use canvas_traits::webgl::WebGLVersion;
 use dom::bindings::codegen::Bindings::OESElementIndexUintBinding;
 use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object};
 use dom::bindings::root::DomRoot;
-use dom::webglrenderingcontext::WebGLRenderingContext;
+use dom::webglrenderingcontext::{WebGLRenderingContext, is_gles};
 use dom_struct::dom_struct;
 use super::{WebGLExtension, WebGLExtensions, WebGLExtensionSpec};
 
@@ -37,11 +37,8 @@ impl WebGLExtension for OESElementIndexUint {
     }
 
     fn is_supported(ext: &WebGLExtensions) -> bool {
-        if cfg!(any(target_os = "android", target_os = "ios")) {
-            return ext.supports_gl_extension("GL_OES_element_index_uint");
-        }
         // This extension is always available in desktop OpenGL.
-        true
+        !is_gles() || ext.supports_gl_extension("GL_OES_element_index_uint")
     }
 
     fn enable(ext: &WebGLExtensions) {

components/script/dom/webgl_extensions/ext/oesstandardderivatives.rs

@@ -7,7 +7,7 @@ use dom::bindings::codegen::Bindings::OESStandardDerivativesBinding;
 use dom::bindings::codegen::Bindings::OESStandardDerivativesBinding::OESStandardDerivativesConstants;
 use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object};
 use dom::bindings::root::DomRoot;
-use dom::webglrenderingcontext::WebGLRenderingContext;
+use dom::webglrenderingcontext::{WebGLRenderingContext, is_gles};
 use dom_struct::dom_struct;
 use super::{WebGLExtension, WebGLExtensions, WebGLExtensionSpec};
 
@@ -37,11 +37,8 @@ impl WebGLExtension for OESStandardDerivatives {
     }
 
     fn is_supported(ext: &WebGLExtensions) -> bool {
-        if cfg!(any(target_os = "android", target_os = "ios")) {
-            return ext.supports_any_gl_extension(&["GL_OES_standard_derivatives"]);
-        }
         // The standard derivatives are always available in desktop OpenGL.
-        true
+        !is_gles() || ext.supports_any_gl_extension(&["GL_OES_standard_derivatives"])
     }
 
     fn enable(ext: &WebGLExtensions) {

components/script/dom/webglframebuffer.rs

@@ -175,7 +175,11 @@ impl WebGLFramebuffer {
         self.size.set(fb_size);
 
         if has_c || has_z || has_zs || has_s {
-            self.status.set(constants::FRAMEBUFFER_COMPLETE);
+            if self.size.get().map_or(false, |(w, h)| w != 0 && h != 0) {
+                self.status.set(constants::FRAMEBUFFER_COMPLETE);
+            } else {
+                self.status.set(constants::FRAMEBUFFER_INCOMPLETE_ATTACHMENT);
+            }
         } else {
             self.status.set(constants::FRAMEBUFFER_UNSUPPORTED);
         }

components/script/dom/webglrenderbuffer.rs

@@ -4,13 +4,14 @@
 
 // https://www.khronos.org/registry/webgl/specs/latest/1.0/webgl.idl
 use canvas_traits::webgl::{webgl_channel, WebGLCommand, WebGLError, WebGLRenderbufferId, WebGLResult};
+use dom::bindings::codegen::Bindings::WebGL2RenderingContextBinding::WebGL2RenderingContextConstants as WebGl2Constants;
 use dom::bindings::codegen::Bindings::WebGLRenderbufferBinding;
 use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants;
 use dom::bindings::inheritance::Castable;
 use dom::bindings::reflector::{DomObject, reflect_dom_object};
 use dom::bindings::root::DomRoot;
 use dom::webglobject::WebGLObject;
-use dom::webglrenderingcontext::WebGLRenderingContext;
+use dom::webglrenderingcontext::{WebGLRenderingContext, is_gles};
 use dom_struct::dom_struct;
 use std::cell::Cell;
 
@@ -61,6 +62,10 @@ impl WebGLRenderbuffer {
         self.size.get()
     }
 
+    pub fn internal_format(&self) -> u32 {
+        self.internal_format.get().unwrap_or(constants::RGBA4)
+    }
+
     pub fn bind(&self, target: u32) {
         self.ever_bound.set(true);
         self.upcast::<WebGLObject>()
@@ -88,25 +93,39 @@ impl WebGLRenderbuffer {
     pub fn storage(&self, internal_format: u32, width: i32, height: i32) -> WebGLResult<()> {
         // Validate the internal_format, and save it for completeness
         // validation.
-        match internal_format {
+        let actual_format = match internal_format {
             constants::RGBA4 |
-            constants::RGB565 |
-            constants::RGB5_A1 |
             constants::DEPTH_COMPONENT16 |
             constants::STENCIL_INDEX8 |
             // https://www.khronos.org/registry/webgl/specs/latest/1.0/#6.7
-            constants::DEPTH_STENCIL => {
+            constants::DEPTH_STENCIL => internal_format,
-                self.internal_format.set(Some(internal_format))
+            constants::RGB5_A1 => {
+                // 16-bit RGBA formats are not supported on desktop GL.
+                if is_gles() {
+                    constants::RGB5_A1
+                } else {
+                    WebGl2Constants::RGBA8
+                }
+            }
+            constants::RGB565 => {
+                // RGB565 is not supported on desktop GL.
+                if is_gles() {
+                    constants::RGB565
+                } else {
+                    WebGl2Constants::RGB8
+                }
             }
             _ => return Err(WebGLError::InvalidEnum),
         };
 
+        self.internal_format.set(Some(internal_format));
+
         // FIXME: Invalidate completeness after the call
 
         self.upcast::<WebGLObject>().context().send_command(
             WebGLCommand::RenderbufferStorage(
                 constants::RENDERBUFFER,
-                internal_format,
+                actual_format,
                 width,
                 height,
             )

components/script/dom/webglrenderingcontext.rs

@@ -71,6 +71,12 @@ use std::cmp;
 use std::ptr::{self, NonNull};
 use webrender_api;
 
+pub fn is_gles() -> bool {
+    // TODO: align this with the actual kind of graphics context in use, rather than
+    // making assumptions based on platform
+    cfg!(any(target_os = "android", target_os = "ios"))
+}
+
 type ImagePixelResult = Result<(Vec<u8>, Size2D<i32>, bool), ()>;
 pub const MAX_UNIFORM_AND_ATTRIBUTE_LEN: usize = 256;
 
@@ -2702,10 +2708,17 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
             return NullValue();
         }
 
-        let (sender, receiver) = webgl_channel().unwrap();
+        let result = if pname == constants::RENDERBUFFER_INTERNAL_FORMAT {
-        self.send_command(WebGLCommand::GetRenderbufferParameter(target, pname, sender));
+            let rb = self.bound_renderbuffer.get().unwrap();
+            rb.internal_format() as i32
+        } else {
+            let (sender, receiver) = webgl_channel().unwrap();
+            self.send_command(WebGLCommand::GetRenderbufferParameter(target, pname, sender));
+            receiver.recv().unwrap()
+        };
 
-        Int32Value(receiver.recv().unwrap())
+
+        Int32Value(result)
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9

tests/wpt/mozilla/meta/webgl/conformance-2.0.0/conformance2/extensions/ext-color-buffer-float.html.ini

@@ -2,48 +2,24 @@
   [WebGL test #2: getError expected: NO_ERROR. Was INVALID_ENUM : floating-point texture allocation should succeed]
     expected: FAIL
 
-  [WebGL test #3: floating-point R16F render target should not be supported without enabling EXT_color_buffer_float]
-    expected: FAIL
-
   [WebGL test #5: getError expected: NO_ERROR. Was INVALID_ENUM : floating-point texture allocation should succeed]
     expected: FAIL
 
-  [WebGL test #6: floating-point RG16F render target should not be supported without enabling EXT_color_buffer_float]
-    expected: FAIL
-
   [WebGL test #8: getError expected: NO_ERROR. Was INVALID_ENUM : floating-point texture allocation should succeed]
     expected: FAIL
 
-  [WebGL test #9: floating-point RGBA16F render target should not be supported without enabling EXT_color_buffer_float]
-    expected: FAIL
-
   [WebGL test #11: getError expected: NO_ERROR. Was INVALID_ENUM : floating-point texture allocation should succeed]
     expected: FAIL
 
-  [WebGL test #12: floating-point R32F render target should not be supported without enabling EXT_color_buffer_float]
-    expected: FAIL
-
   [WebGL test #14: getError expected: NO_ERROR. Was INVALID_ENUM : floating-point texture allocation should succeed]
     expected: FAIL
 
-  [WebGL test #15: floating-point RG32F render target should not be supported without enabling EXT_color_buffer_float]
-    expected: FAIL
-
   [WebGL test #17: getError expected: NO_ERROR. Was INVALID_ENUM : floating-point texture allocation should succeed]
     expected: FAIL
 
-  [WebGL test #18: floating-point RGBA32F render target should not be supported without enabling EXT_color_buffer_float]
-    expected: FAIL
-
   [WebGL test #20: getError expected: NO_ERROR. Was INVALID_ENUM : floating-point texture allocation should succeed]
     expected: FAIL
 
-  [WebGL test #21: floating-point R11F_G11F_B10F render target should not be supported without enabling EXT_color_buffer_float]
-    expected: FAIL
-
   [WebGL test #30: getError expected: NO_ERROR. Was INVALID_ENUM : RGB16F texture allocation should succeed]
     expected: FAIL
 
-  [WebGL test #31: RGB16F render target should not be supported with or without enabling EXT_color_buffer_float]
-    expected: FAIL
-

tests/wpt/mozilla/meta/webgl/conformance-2.0.0/conformance2/reading/read-pixels-from-fbo-test.html.ini

@@ -1,2 +1,97 @@
 [read-pixels-from-fbo-test.html]
-  expected: CRASH
+  [WebGL test #0: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #1: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #2: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #3: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #4: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #5: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #6: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #7: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #8: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #9: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #10: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #11: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #12: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #13: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #14: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #15: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #16: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #17: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #18: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #19: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #20: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #21: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #22: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #23: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #24: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #25: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #26: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #27: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #28: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #29: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #30: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+
+  [WebGL test #31: getError expected: NO_ERROR. Was INVALID_ENUM : Setting up fbo should generate no error]
+    expected: FAIL
+

tests/wpt/mozilla/meta/webgl/conformance-2.0.0/conformance2/reading/read-pixels-from-rgb8-into-pbo-bug.html.ini

@@ -1,2 +1,7 @@
 [read-pixels-from-rgb8-into-pbo-bug.html]
-  expected: CRASH
+  [WebGL test #1: framebuffer with RGB8 color buffer is incomplete]
+    expected: FAIL
+
+  [WebGL test #2: getError expected: NO_ERROR. Was INVALID_ENUM : Tests should complete without gl errors]
+    expected: FAIL
+

tests/wpt/mozilla/meta/webgl/conformance-2.0.0/conformance2/renderbuffers/multisampled-renderbuffer-initialization.html.ini

@@ -1,2 +1,14 @@
 [multisampled-renderbuffer-initialization.html]
-  expected: CRASH
+  expected: ERROR
+  [WebGL test #1: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.]
+    expected: FAIL
+
+  [WebGL test #2: getError expected: NO_ERROR. Was INVALID_ENUM : should be no errors]
+    expected: FAIL
+
+  [WebGL test #3: user buffer has been cleared to green\nat (0, 0) expected: 0,255,0,255 was 0,0,0,0]
+    expected: FAIL
+
+  [WebGL test #4: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).]
+    expected: FAIL
+

tests/wpt/mozilla/meta/webgl/conformance-2.0.0/conformance2/rendering/clear-srgb-color-buffer.html.ini

@@ -1,2 +1,11 @@
 [clear-srgb-color-buffer.html]
-  expected: CRASH
+  expected: ERROR
+  [WebGL test #1: Framebuffer incomplete.]
+    expected: FAIL
+
+  [WebGL test #2: should be 124,193,222,255\nat (0, 0) expected: 124,193,222,255 was 0,0,0,0]
+    expected: FAIL
+
+  [WebGL test #3: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).]
+    expected: FAIL
+

tests/wpt/mozilla/meta/webgl/conformance-2.0.0/conformance2/rendering/framebuffer-completeness-unaffected.html.ini

@@ -3,3 +3,6 @@
   [WebGL test #2: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).]
     expected: FAIL
 
+  [WebGL test #1: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.]
+    expected: FAIL
+