From 31e25658b055c5244b1d5cb5a15c34fa0066a0a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= <john.kare.alsaker@gmail.com>
Date: Tue, 17 Oct 2023 22:53:47 +0200
Subject: [PATCH] Improve OpenGL 3.1 support

---
 wgpu-hal/src/gles/adapter.rs | 67 +++++++++++++++++++------
 wgpu-hal/src/gles/command.rs | 15 +++---
 wgpu-hal/src/gles/device.rs  | 97 ++++++++++++++++++++++++++++++++----
 wgpu-hal/src/gles/mod.rs     |  4 ++
 wgpu-hal/src/gles/queue.rs   | 70 ++++++++++++++++++--------
 5 files changed, 198 insertions(+), 55 deletions(-)

diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs
index 4b88926cd19..064e2295ce8 100644
--- a/wgpu-hal/src/gles/adapter.rs
+++ b/wgpu-hal/src/gles/adapter.rs
@@ -15,7 +15,7 @@ impl super::Adapter {
     /// resulting in an `Err`.
     /// # Notes
     /// `WebGL 2` version returned as `OpenGL ES 3.0`
-    fn parse_version(mut src: &str) -> Result<(u8, u8), crate::InstanceError> {
+    fn parse_version(mut src: &str) -> Result<((u8, u8), bool), crate::InstanceError> {
         let webgl_sig = "WebGL ";
         // According to the WebGL specification
         // VERSION  WebGL<space>1.0<space><vendor-specific information>
@@ -50,12 +50,15 @@ impl super::Adapter {
         Self::parse_full_version(src).map(|(major, minor)| {
             (
                 // Return WebGL 2.0 version as OpenGL ES 3.0
-                if is_webgl && !is_glsl {
-                    major + 1
-                } else {
-                    major
-                },
-                minor,
+                (
+                    if is_webgl && !is_glsl {
+                        major + 1
+                    } else {
+                        major
+                    },
+                    minor,
+                ),
+                is_webgl,
             )
         })
     }
@@ -218,10 +221,30 @@ impl super::Adapter {
         log::trace!("Version: {}", version);
 
         let full_ver = Self::parse_full_version(&version).ok();
-        let es_ver = full_ver
+        let es_ver_web_gl = full_ver
             .is_none()
             .then_some(())
             .and_then(|_| Self::parse_version(&version).ok());
+        let es_ver = es_ver_web_gl.map(|(v, _)| v);
+        let web_gl = es_ver_web_gl.map(|(_, w)| w).unwrap_or_default();
+
+        if let Some(full_ver) = full_ver {
+            let core_profile = (full_ver >= (3, 2)).then_some(unsafe {
+                gl.get_parameter_i32(glow::CONTEXT_PROFILE_MASK)
+                    & glow::CONTEXT_CORE_PROFILE_BIT as i32
+                    != 0
+            });
+            log::trace!(
+                "Profile: {}",
+                core_profile
+                    .map(|core_profile| if core_profile {
+                        "Core"
+                    } else {
+                        "Compatibility"
+                    })
+                    .unwrap_or("Legacy")
+            );
+        }
 
         if es_ver.is_none() && full_ver.is_none() {
             log::warn!("Unable to parse OpenGL version");
@@ -262,7 +285,7 @@ impl super::Adapter {
                 }
                 naga::back::glsl::Version::Desktop(value)
             } else {
-                let (sl_major, sl_minor) = Self::parse_version(&sl_version).ok()?;
+                let (sl_major, sl_minor) = Self::parse_version(&sl_version).ok().map(|(v, _)| v)?;
                 let value = sl_major as u16 * 100 + sl_minor as u16 * 10;
                 naga::back::glsl::Version::Embedded {
                     version: value,
@@ -584,6 +607,14 @@ impl super::Adapter {
             },
         );
         private_caps.set(super::PrivateCapabilities::QUERY_BUFFERS, query_buffers);
+        private_caps.set(
+            super::PrivateCapabilities::TEXTURE_STORAGE,
+            supported((3, 0), (4, 2)),
+        );
+        private_caps.set(
+            super::PrivateCapabilities::DEBUG_FNS,
+            supported((3, 2), (4, 3)) && !web_gl,
+        );
 
         let max_texture_size = unsafe { gl.get_parameter_i32(glow::MAX_TEXTURE_SIZE) } as u32;
         let max_texture_3d_size = unsafe { gl.get_parameter_i32(glow::MAX_3D_TEXTURE_SIZE) } as u32;
@@ -1169,24 +1200,30 @@ mod tests {
         Adapter::parse_version("1. h3l1o. W0rld").unwrap_err();
         Adapter::parse_version("1.2.3").unwrap_err();
 
-        assert_eq!(Adapter::parse_version("OpenGL ES 3.1").unwrap(), (3, 1));
+        assert_eq!(
+            Adapter::parse_version("OpenGL ES 3.1").unwrap(),
+            ((3, 1), false)
+        );
         assert_eq!(
             Adapter::parse_version("OpenGL ES 2.0 Google Nexus").unwrap(),
-            (2, 0)
+            ((2, 0), false)
+        );
+        assert_eq!(
+            Adapter::parse_version("GLSL ES 1.1").unwrap(),
+            ((1, 1), false)
         );
-        assert_eq!(Adapter::parse_version("GLSL ES 1.1").unwrap(), (1, 1));
         assert_eq!(
             Adapter::parse_version("OpenGL ES GLSL ES 3.20").unwrap(),
-            (3, 2)
+            ((3, 2), false)
         );
         assert_eq!(
             // WebGL 2.0 should parse as OpenGL ES 3.0
             Adapter::parse_version("WebGL 2.0 (OpenGL ES 3.0 Chromium)").unwrap(),
-            (3, 0)
+            ((3, 0), true)
         );
         assert_eq!(
             Adapter::parse_version("WebGL GLSL ES 3.00 (OpenGL ES GLSL ES 3.0 Chromium)").unwrap(),
-            (3, 0)
+            ((3, 0), true)
         );
     }
 }
diff --git a/wgpu-hal/src/gles/command.rs b/wgpu-hal/src/gles/command.rs
index 27a9e11fdc7..73f1ccafea1 100644
--- a/wgpu-hal/src/gles/command.rs
+++ b/wgpu-hal/src/gles/command.rs
@@ -535,13 +535,6 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
                             .push(glow::STENCIL_ATTACHMENT);
                     }
                 }
-
-                if !rendering_to_external_framebuffer {
-                    // set the draw buffers and states
-                    self.cmd_buffer
-                        .commands
-                        .push(C::SetDrawColorBuffers(desc.color_attachments.len() as u8));
-                }
             }
         }
 
@@ -586,6 +579,14 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
                 );
             }
         }
+
+        if !rendering_to_external_framebuffer {
+            // set the draw buffers and states
+            self.cmd_buffer
+                .commands
+                .push(C::SetDrawColorBuffers(desc.color_attachments.len() as u8));
+        }
+
         if let Some(ref dsat) = desc.depth_stencil_attachment {
             let clear_depth = !dsat.depth_ops.contains(crate::AttachmentOps::LOAD);
             let clear_stencil = !dsat.stencil_ops.contains(crate::AttachmentOps::LOAD);
diff --git a/wgpu-hal/src/gles/device.rs b/wgpu-hal/src/gles/device.rs
index fe664c3cfeb..b27d406859e 100644
--- a/wgpu-hal/src/gles/device.rs
+++ b/wgpu-hal/src/gles/device.rs
@@ -1,7 +1,8 @@
-use super::conv;
+use super::{conv, PrivateCapabilities};
 use crate::auxil::map_naga_stage;
 use glow::HasContext;
 use std::{
+    cmp::max,
     convert::TryInto,
     ptr,
     sync::{Arc, Mutex},
@@ -28,9 +29,12 @@ struct CompilationContext<'a> {
 impl CompilationContext<'_> {
     fn consume_reflection(
         self,
+        gl: &glow::Context,
         module: &naga::Module,
         ep_info: &naga::valid::FunctionInfo,
         reflection_info: naga::back::glsl::ReflectionInfo,
+        naga_stage: naga::ShaderStage,
+        program: glow::Program,
     ) {
         for (handle, var) in module.global_variables.iter() {
             if ep_info[handle].is_empty() {
@@ -83,6 +87,20 @@ impl CompilationContext<'_> {
                 self.sampler_map[texture_linear_index as usize] = Some(sampler_linear_index);
             }
         }
+
+        for (name, location) in reflection_info.varying {
+            match naga_stage {
+                naga::ShaderStage::Vertex => {
+                    assert_eq!(location.index, 0);
+                    unsafe { gl.bind_attrib_location(program, location.location, &name) }
+                }
+                naga::ShaderStage::Fragment => {
+                    assert_eq!(location.index, 0);
+                    unsafe { gl.bind_frag_data_location(program, location.location, &name) }
+                }
+                naga::ShaderStage::Compute => {}
+            }
+        }
     }
 }
 
@@ -176,6 +194,8 @@ impl super::Device {
             }
             Ok(raw)
         } else {
+            log::error!("\tShader compilation failed: {}", msg);
+            unsafe { gl.delete_shader(raw) };
             Err(crate::PipelineError::Linkage(
                 map_naga_stage(naga_stage),
                 msg,
@@ -188,6 +208,7 @@ impl super::Device {
         naga_stage: naga::ShaderStage,
         stage: &crate::ProgrammableStage<super::Api>,
         context: CompilationContext,
+        program: glow::Program,
     ) -> Result<glow::Shader, crate::PipelineError> {
         use naga::back::glsl;
         let pipeline_options = glsl::PipelineOptions {
@@ -207,7 +228,7 @@ impl super::Device {
         use naga::proc::BoundsCheckPolicy;
         // The image bounds checks require the TEXTURE_LEVELS feature available in GL core 1.3+.
         let version = gl.version();
-        let image_check = if !version.is_embedded && (version.major, version.minor) >= (1, 3) {
+        let image_check = if !version.is_embedded && (version.major, version.minor) >= (4, 3) {
             BoundsCheckPolicy::ReadZeroSkipWrite
         } else {
             BoundsCheckPolicy::Unchecked
@@ -244,9 +265,12 @@ impl super::Device {
         log::debug!("Naga generated shader:\n{}", output);
 
         context.consume_reflection(
+            gl,
             &shader.module,
             shader.info.get_entry_point(entry_point_index),
             reflection_info,
+            naga_stage,
+            program,
         );
 
         unsafe { Self::compile_shader(gl, &output, naga_stage, stage.module.label.as_deref()) }
@@ -308,7 +332,7 @@ impl super::Device {
         #[cfg_attr(target_arch = "wasm32", allow(unused))] label: Option<&str>,
         multiview: Option<std::num::NonZeroU32>,
         glsl_version: naga::back::glsl::Version,
-        private_caps: super::PrivateCapabilities,
+        private_caps: PrivateCapabilities,
     ) -> Result<Arc<super::PipelineInner>, crate::PipelineError> {
         let glsl_version = match glsl_version {
             naga::back::glsl::Version::Embedded { version, .. } => format!("{version} es"),
@@ -337,7 +361,7 @@ impl super::Device {
                 multiview,
             };
 
-            let shader = Self::create_shader(gl, naga_stage, stage, context)?;
+            let shader = Self::create_shader(gl, naga_stage, stage, context, program)?;
             shaders_to_delete.push(shader);
         }
 
@@ -748,13 +772,64 @@ impl crate::Device<super::Api> for super::Device {
                 };
             } else {
                 unsafe {
-                    gl.tex_storage_2d(
-                        target,
-                        desc.mip_level_count as i32,
-                        format_desc.internal,
-                        desc.size.width as i32,
-                        desc.size.height as i32,
-                    )
+                    if self
+                        .shared
+                        .private_caps
+                        .contains(PrivateCapabilities::TEXTURE_STORAGE)
+                    {
+                        gl.tex_storage_2d(
+                            target,
+                            desc.mip_level_count as i32,
+                            format_desc.internal,
+                            desc.size.width as i32,
+                            desc.size.height as i32,
+                        )
+                    } else if target == glow::TEXTURE_CUBE_MAP {
+                        let mut width = desc.size.width;
+                        let mut height = desc.size.width;
+                        for i in 0..desc.mip_level_count {
+                            for face in [
+                                glow::TEXTURE_CUBE_MAP_POSITIVE_X,
+                                glow::TEXTURE_CUBE_MAP_NEGATIVE_X,
+                                glow::TEXTURE_CUBE_MAP_POSITIVE_Y,
+                                glow::TEXTURE_CUBE_MAP_NEGATIVE_Y,
+                                glow::TEXTURE_CUBE_MAP_POSITIVE_Z,
+                                glow::TEXTURE_CUBE_MAP_NEGATIVE_Z,
+                            ] {
+                                gl.tex_image_2d(
+                                    face,
+                                    i as i32,
+                                    format_desc.internal as i32,
+                                    width as i32,
+                                    height as i32,
+                                    0,
+                                    format_desc.external,
+                                    format_desc.data_type,
+                                    None,
+                                );
+                            }
+                            width = max(1, width / 2);
+                            height = max(1, height / 2);
+                        }
+                    } else {
+                        let mut width = desc.size.width;
+                        let mut height = desc.size.width;
+                        for i in 0..desc.mip_level_count {
+                            gl.tex_image_2d(
+                                target,
+                                i as i32,
+                                format_desc.internal as i32,
+                                width as i32,
+                                height as i32,
+                                0,
+                                format_desc.external,
+                                format_desc.data_type,
+                                None,
+                            );
+                            width = max(1, width / 2);
+                            height = max(1, height / 2);
+                        }
+                    }
                 };
             }
 
diff --git a/wgpu-hal/src/gles/mod.rs b/wgpu-hal/src/gles/mod.rs
index c46dabb96f4..72fb037a631 100644
--- a/wgpu-hal/src/gles/mod.rs
+++ b/wgpu-hal/src/gles/mod.rs
@@ -163,6 +163,10 @@ bitflags::bitflags! {
         const TEXTURE_FLOAT_LINEAR = 1 << 10;
         /// Supports query buffer objects.
         const QUERY_BUFFERS = 1 << 11;
+        /// Supports `glTexStorage2D`, etc.
+        const TEXTURE_STORAGE = 1 << 12;
+        /// Supports `push_debug_group`, `pop_debug_group` and `debug_message_insert`.
+        const DEBUG_FNS = 1 << 13;
     }
 }
 
diff --git a/wgpu-hal/src/gles/queue.rs b/wgpu-hal/src/gles/queue.rs
index 2c57c1f52c4..479ebe27207 100644
--- a/wgpu-hal/src/gles/queue.rs
+++ b/wgpu-hal/src/gles/queue.rs
@@ -3,10 +3,8 @@ use arrayvec::ArrayVec;
 use glow::HasContext;
 use std::{mem, slice, sync::Arc};
 
-#[cfg(not(target_arch = "wasm32"))]
 const DEBUG_ID: u32 = 0;
 
-#[cfg(not(target_arch = "wasm32"))]
 fn extract_marker<'a>(data: &'a [u8], range: &std::ops::Range<u32>) -> &'a str {
     std::str::from_utf8(&data[range.start as usize..range.end as usize]).unwrap()
 }
@@ -992,6 +990,14 @@ impl super::Queue {
                     && is_srgb
                 {
                     unsafe { self.perform_shader_clear(gl, draw_buffer, *color) };
+                } else if draw_buffer < 32 {
+                    // Prefer `clear_color` as `clear_buffer_f32_slice` crashes on Sandy Bridge
+                    // on Windows.
+                    unsafe {
+                        gl.draw_buffers(&[glow::COLOR_ATTACHMENT0 + draw_buffer]);
+                        gl.clear_color(color[0], color[1], color[2], color[3]);
+                        gl.clear(glow::COLOR_BUFFER_BIT);
+                    }
                 } else {
                     unsafe { gl.clear_buffer_f32_slice(glow::COLOR, draw_buffer, color) };
                 }
@@ -1364,34 +1370,45 @@ impl super::Queue {
                     )
                 };
             }
-            #[cfg(not(target_arch = "wasm32"))]
             C::InsertDebugMarker(ref range) => {
                 let marker = extract_marker(data_bytes, range);
                 unsafe {
-                    gl.debug_message_insert(
-                        glow::DEBUG_SOURCE_APPLICATION,
-                        glow::DEBUG_TYPE_MARKER,
-                        DEBUG_ID,
-                        glow::DEBUG_SEVERITY_NOTIFICATION,
-                        marker,
-                    )
+                    if self
+                        .shared
+                        .private_caps
+                        .contains(PrivateCapabilities::DEBUG_FNS)
+                    {
+                        gl.debug_message_insert(
+                            glow::DEBUG_SOURCE_APPLICATION,
+                            glow::DEBUG_TYPE_MARKER,
+                            DEBUG_ID,
+                            glow::DEBUG_SEVERITY_NOTIFICATION,
+                            marker,
+                        )
+                    }
                 };
             }
-            #[cfg(target_arch = "wasm32")]
-            C::InsertDebugMarker(_) => (),
-            #[cfg_attr(target_arch = "wasm32", allow(unused))]
             C::PushDebugGroup(ref range) => {
-                #[cfg(not(target_arch = "wasm32"))]
                 let marker = extract_marker(data_bytes, range);
-                #[cfg(not(target_arch = "wasm32"))]
                 unsafe {
-                    gl.push_debug_group(glow::DEBUG_SOURCE_APPLICATION, DEBUG_ID, marker)
+                    if self
+                        .shared
+                        .private_caps
+                        .contains(PrivateCapabilities::DEBUG_FNS)
+                    {
+                        gl.push_debug_group(glow::DEBUG_SOURCE_APPLICATION, DEBUG_ID, marker)
+                    }
                 };
             }
             C::PopDebugGroup => {
-                #[cfg(not(target_arch = "wasm32"))]
                 unsafe {
-                    gl.pop_debug_group()
+                    if self
+                        .shared
+                        .private_caps
+                        .contains(PrivateCapabilities::DEBUG_FNS)
+                    {
+                        gl.pop_debug_group()
+                    }
                 };
             }
             C::SetPushConstants {
@@ -1476,17 +1493,26 @@ impl crate::Queue<super::Api> for super::Queue {
             // this at the beginning of the loop in case something outside of wgpu modified
             // this state prior to commit.
             unsafe { self.reset_state(gl) };
-            #[cfg(not(target_arch = "wasm32"))]
             if let Some(ref label) = cmd_buf.label {
-                unsafe { gl.push_debug_group(glow::DEBUG_SOURCE_APPLICATION, DEBUG_ID, label) };
+                if self
+                    .shared
+                    .private_caps
+                    .contains(PrivateCapabilities::DEBUG_FNS)
+                {
+                    unsafe { gl.push_debug_group(glow::DEBUG_SOURCE_APPLICATION, DEBUG_ID, label) };
+                }
             }
 
             for command in cmd_buf.commands.iter() {
                 unsafe { self.process(gl, command, &cmd_buf.data_bytes, &cmd_buf.queries) };
             }
 
-            #[cfg(not(target_arch = "wasm32"))]
-            if cmd_buf.label.is_some() {
+            if cmd_buf.label.is_some()
+                && self
+                    .shared
+                    .private_caps
+                    .contains(PrivateCapabilities::DEBUG_FNS)
+            {
                 unsafe { gl.pop_debug_group() };
             }
         }