From 4245eaf1b2a6085f90ddb0fd4f68bb0172979d6a Mon Sep 17 00:00:00 2001
From: Alexander Rose <alexander.rose@weirdbyte.de>
Date: Sun, 26 Jun 2022 17:29:21 -0700
Subject: [PATCH] improve use of gl_VertexID when possible

---
 CHANGELOG.md                                       |  1 +
 src/mol-gl/shader-code.ts                          |  3 +++
 .../shader/chunks/common-vert-params.glsl.ts       | 10 ++--------
 src/mol-gl/webgl/compat.ts                         | 14 ++++++++++++++
 src/mol-gl/webgl/extensions.ts                     |  8 +++++++-
 src/mol-gl/webgl/render-item.ts                    |  7 +------
 6 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f3109f8d5..fb08d33af 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@ Note that since we don't clearly distinguish between a public and private interf
 ## [Unreleased]
 
 - Fix superfluous shader varying
+- Improve use of gl_VertexID when possible
 
 ## [v3.10.1] - 2022-06-26
 
diff --git a/src/mol-gl/shader-code.ts b/src/mol-gl/shader-code.ts
index c4a013610..c65f6df5d 100644
--- a/src/mol-gl/shader-code.ts
+++ b/src/mol-gl/shader-code.ts
@@ -309,6 +309,9 @@ function getGlsl300VertPrefix(extensions: WebGLExtensions, shaderExtensions: Sha
             prefix.push('#define requiredDrawBuffers');
         }
     }
+    if (extensions.noNonInstancedActiveAttribs) {
+        prefix.push('#define noNonInstancedActiveAttribs');
+    }
     prefix.push(glsl300VertPrefixCommon);
     return prefix.join('\n') + '\n';
 }
diff --git a/src/mol-gl/shader/chunks/common-vert-params.glsl.ts b/src/mol-gl/shader/chunks/common-vert-params.glsl.ts
index 384e18186..b4e3e2c50 100644
--- a/src/mol-gl/shader/chunks/common-vert-params.glsl.ts
+++ b/src/mol-gl/shader/chunks/common-vert-params.glsl.ts
@@ -43,16 +43,10 @@ uniform int uPickType;
 varying vec3 vModelPosition;
 varying vec3 vViewPosition;
 
-#if __VERSION__ == 100
-    attribute float aVertex;
-    #define VertexID int(aVertex)
+#if defined(noNonInstancedActiveAttribs)
+    #define VertexID gl_VertexID
 #else
-    // not using gl_VertexID but aVertex to ensure there is an active attribute with divisor 0
-    // since FF 85 this is not needed anymore but lets keep it for backwards compatibility
-    // https://bugzilla.mozilla.org/show_bug.cgi?id=1679693
-    // see also note in src/mol-gl/webgl/render-item.ts
     attribute float aVertex;
     #define VertexID int(aVertex)
-    // #define VertexID gl_VertexID
 #endif
 `;
\ No newline at end of file
diff --git a/src/mol-gl/webgl/compat.ts b/src/mol-gl/webgl/compat.ts
index 6c2691911..8fa3aab18 100644
--- a/src/mol-gl/webgl/compat.ts
+++ b/src/mol-gl/webgl/compat.ts
@@ -401,6 +401,20 @@ export function getDisjointTimerQuery(gl: GLRenderingContext): COMPAT_disjoint_t
     }
 }
 
+export function getNoNonInstancedActiveAttribs(gl: GLRenderingContext): boolean {
+    if (!isWebGL2(gl)) return false;
+
+    if (typeof navigator !== 'undefined') {
+        const ffMatch = window.navigator.userAgent.match(/Firefox\/([0-9]+)\./);
+        if (!ffMatch) return true;
+
+        const ffVersion = parseInt(ffMatch[1]);
+        // supported since FF 85 (https://bugzilla.mozilla.org/show_bug.cgi?id=1679693)
+        return ffVersion >= 85;
+    }
+    return false;
+}
+
 //
 
 const TextureTestVertShader = `
diff --git a/src/mol-gl/webgl/extensions.ts b/src/mol-gl/webgl/extensions.ts
index 99cc66b48..0d360170b 100644
--- a/src/mol-gl/webgl/extensions.ts
+++ b/src/mol-gl/webgl/extensions.ts
@@ -4,7 +4,7 @@
  * @author Alexander Rose <alexander.rose@weirdbyte.de>
  */
 
-import { GLRenderingContext, COMPAT_instanced_arrays, COMPAT_standard_derivatives, COMPAT_vertex_array_object, getInstancedArrays, getStandardDerivatives, COMPAT_element_index_uint, getElementIndexUint, COMPAT_texture_float, getTextureFloat, COMPAT_texture_float_linear, getTextureFloatLinear, COMPAT_blend_minmax, getBlendMinMax, getFragDepth, COMPAT_frag_depth, COMPAT_color_buffer_float, getColorBufferFloat, COMPAT_draw_buffers, getDrawBuffers, getShaderTextureLod, COMPAT_shader_texture_lod, getDepthTexture, COMPAT_depth_texture, COMPAT_sRGB, getSRGB, getTextureHalfFloat, getTextureHalfFloatLinear, COMPAT_texture_half_float, COMPAT_texture_half_float_linear, COMPAT_color_buffer_half_float, getColorBufferHalfFloat, getVertexArrayObject, getDisjointTimerQuery, COMPAT_disjoint_timer_query } from './compat';
+import { GLRenderingContext, COMPAT_instanced_arrays, COMPAT_standard_derivatives, COMPAT_vertex_array_object, getInstancedArrays, getStandardDerivatives, COMPAT_element_index_uint, getElementIndexUint, COMPAT_texture_float, getTextureFloat, COMPAT_texture_float_linear, getTextureFloatLinear, COMPAT_blend_minmax, getBlendMinMax, getFragDepth, COMPAT_frag_depth, COMPAT_color_buffer_float, getColorBufferFloat, COMPAT_draw_buffers, getDrawBuffers, getShaderTextureLod, COMPAT_shader_texture_lod, getDepthTexture, COMPAT_depth_texture, COMPAT_sRGB, getSRGB, getTextureHalfFloat, getTextureHalfFloatLinear, COMPAT_texture_half_float, COMPAT_texture_half_float_linear, COMPAT_color_buffer_half_float, getColorBufferHalfFloat, getVertexArrayObject, getDisjointTimerQuery, COMPAT_disjoint_timer_query, getNoNonInstancedActiveAttribs } from './compat';
 import { isDebugMode } from '../../mol-util/debug';
 
 export type WebGLExtensions = {
@@ -26,6 +26,8 @@ export type WebGLExtensions = {
     shaderTextureLod: COMPAT_shader_texture_lod | null
     sRGB: COMPAT_sRGB | null
     disjointTimerQuery: COMPAT_disjoint_timer_query | null
+
+    noNonInstancedActiveAttribs: boolean
 }
 
 export function createExtensions(gl: GLRenderingContext): WebGLExtensions {
@@ -105,6 +107,8 @@ export function createExtensions(gl: GLRenderingContext): WebGLExtensions {
         console.log('Could not find support for "disjoint_timer_query"');
     }
 
+    const noNonInstancedActiveAttribs = getNoNonInstancedActiveAttribs(gl);
+
     return {
         instancedArrays,
         standardDerivatives,
@@ -124,5 +128,7 @@ export function createExtensions(gl: GLRenderingContext): WebGLExtensions {
         shaderTextureLod,
         sRGB,
         disjointTimerQuery,
+
+        noNonInstancedActiveAttribs,
     };
 }
\ No newline at end of file
diff --git a/src/mol-gl/webgl/render-item.ts b/src/mol-gl/webgl/render-item.ts
index 69af3847c..f25090291 100644
--- a/src/mol-gl/webgl/render-item.ts
+++ b/src/mol-gl/webgl/render-item.ts
@@ -112,12 +112,7 @@ export function createRenderItem<T extends string>(ctx: WebGLContext, drawMode:
     const { instancedArrays, vertexArrayObject } = ctx.extensions;
 
     // emulate gl_VertexID when needed
-    // if (!ctx.isWebGL2 && values.uVertexCount) {
-    // not using gl_VertexID in WebGL2 but aVertex to ensure there is an active attribute with divisor 0
-    // since FF 85 this is not needed anymore but lets keep it for backwards compatibility
-    // https://bugzilla.mozilla.org/show_bug.cgi?id=1679693
-    // see also note in src/mol-gl/shader/chunks/common-vert-params.glsl.ts
-    if (values.uVertexCount) {
+    if (values.uVertexCount && !ctx.extensions.noNonInstancedActiveAttribs) {
         const vertexCount = values.uVertexCount.ref.value;
         (values as any).aVertex = ValueCell.create(fillSerial(new Float32Array(vertexCount)));
         (schema as any).aVertex = AttributeSpec('float32', 1, 0);
-- 
GitLab