[Bf-blender-cvs] [cecda64e2ea] master: GPU: ShaderInterface: Refactor to setup all uniform at creation time

Clément Foucault noreply at git.blender.org
Tue Jun 2 12:13:00 CEST 2020


Commit: cecda64e2ead502a052f9bea5ffde39e4a46bf90
Author: Clément Foucault
Date:   Tue Jun 2 12:11:39 2020 +0200
Branches: master
https://developer.blender.org/rBcecda64e2ead502a052f9bea5ffde39e4a46bf90

GPU: ShaderInterface: Refactor to setup all uniform at creation time

This remove the complexity of queriying the locations at runtime and
allows for more performance and upfront binding specifications.

The benefit of doing everything at creation time is that we can assign binding
points in a predictable order which is going to be somewhat the same for
every similar shader.

This also rewrite GPU_vertformat_from_shader to not use shaderface.

This is to keep the shaderface simple. If it becomes necessary to not query
the shader after creation (i.e: vulkan?) we could just create the vert
format in advance at compilation for PyGPU shaders.

Reviewed By: brecht

Differential Revision: https://developer.blender.org/D7879

===================================================================

M	intern/opencolorio/ocio_impl_glsl.cc
M	source/blender/editors/interface/interface_icons.c
M	source/blender/gpu/GPU_shader_interface.h
M	source/blender/gpu/GPU_vertex_format.h
M	source/blender/gpu/intern/gpu_batch.c
M	source/blender/gpu/intern/gpu_shader_interface.c
M	source/blender/gpu/intern/gpu_vertex_format.c
M	source/blender/python/gpu/gpu_py_shader.c

===================================================================

diff --git a/intern/opencolorio/ocio_impl_glsl.cc b/intern/opencolorio/ocio_impl_glsl.cc
index df6adc8f34b..87769a647f6 100644
--- a/intern/opencolorio/ocio_impl_glsl.cc
+++ b/intern/opencolorio/ocio_impl_glsl.cc
@@ -263,11 +263,6 @@ static void updateGLSLShader(OCIO_GLSLShader *shader,
     shader->curve_mapping_loc = glGetUniformLocation(shader->program, "curve_mapping");
 
     glUseProgram(shader->program);
-    /* Set texture bind point uniform once. This is saved by the shader. */
-    glUniform1i(glGetUniformLocation(shader->program, "image_texture"), 0);
-    glUniform1i(glGetUniformLocation(shader->program, "lut3d_texture"), 2);
-    glUniform1i(glGetUniformLocation(shader->program, "lut3d_display_texture"), 3);
-    glUniform1i(glGetUniformLocation(shader->program, "curve_mapping_texture"), 4);
 
     /* Set UBO binding location. */
     GLuint index = glGetUniformBlockIndex(shader->program, "OCIO_GLSLCurveMappingParameters");
@@ -276,6 +271,12 @@ static void updateGLSLShader(OCIO_GLSLShader *shader,
     /* TODO(fclem) Remove this. Make caller always assume viewport space and
      * specify texco via vertex attribs. */
     shader->interface = GPU_shaderinterface_create(shader->program);
+
+    /* Set texture bind point uniform once. This is saved by the shader. */
+    glUniform1i(glGetUniformLocation(shader->program, "image_texture"), 0);
+    glUniform1i(glGetUniformLocation(shader->program, "lut3d_texture"), 2);
+    glUniform1i(glGetUniformLocation(shader->program, "lut3d_display_texture"), 3);
+    glUniform1i(glGetUniformLocation(shader->program, "curve_mapping_texture"), 4);
   }
 
   shader->cacheId = cache_id;
diff --git a/source/blender/editors/interface/interface_icons.c b/source/blender/editors/interface/interface_icons.c
index c94a95890c0..0f22c83d945 100644
--- a/source/blender/editors/interface/interface_icons.c
+++ b/source/blender/editors/interface/interface_icons.c
@@ -1604,7 +1604,7 @@ static void icon_draw_cache_texture_flush_ex(GLuint texture,
   GPU_shader_bind(shader);
 
   int img_loc = GPU_shader_get_uniform_ensure(shader, "image");
-  int data_loc = GPU_shader_get_uniform_ensure(shader, "calls_data[0]");
+  int data_loc = GPU_shader_get_uniform_ensure(shader, "calls_data");
 
   glUniform1i(img_loc, 0);
   glUniform4fv(data_loc, ICON_DRAW_CACHE_SIZE * 3, (float *)texture_draw_calls->drawcall_cache);
diff --git a/source/blender/gpu/GPU_shader_interface.h b/source/blender/gpu/GPU_shader_interface.h
index 3e7bad409a3..91064719995 100644
--- a/source/blender/gpu/GPU_shader_interface.h
+++ b/source/blender/gpu/GPU_shader_interface.h
@@ -64,33 +64,34 @@ typedef enum {
 } GPUUniformBuiltin;
 
 typedef struct GPUShaderInput {
-  struct GPUShaderInput *next;
   uint32_t name_offset;
-  uint name_hash;
-  /** Only for uniform inputs. */
-  GPUUniformBuiltin builtin_type;
-  /** Only for attribute inputs. */
-  uint32_t gl_type;
-  /** Only for attribute inputs. */
-  int32_t size;
+  uint32_t name_hash;
   int32_t location;
+  /** Defined at interface creation or in shader. Only for Samplers, UBOs and Vertex Attribs. */
+  int32_t binding;
 } GPUShaderInput;
 
-#define GPU_NUM_SHADERINTERFACE_BUCKETS 257
 #define GPU_SHADERINTERFACE_REF_ALLOC_COUNT 16
 
 typedef struct GPUShaderInterface {
-  int32_t program;
-  uint32_t name_buffer_offset;
-  GPUShaderInput *attr_buckets[GPU_NUM_SHADERINTERFACE_BUCKETS];
-  GPUShaderInput *uniform_buckets[GPU_NUM_SHADERINTERFACE_BUCKETS];
-  GPUShaderInput *ubo_buckets[GPU_NUM_SHADERINTERFACE_BUCKETS];
-  GPUShaderInput *builtin_uniforms[GPU_NUM_UNIFORMS];
+  /** Buffer containing all inputs names separated by '\0'. */
   char *name_buffer;
-  struct GPUBatch **batches; /* references to batches using this interface */
+  /** Reference to GPUBatches using this interface */
+  struct GPUBatch **batches;
   uint batches_len;
-  /** All enabled attributes in this shader. Used to set default values for unbound attributes. */
+  /** Input counts. */
+  uint attribute_len;
+  uint ubo_len;
+  uint uniform_len;
+  /** Enabled bindpoints that needs to be fed with data. */
   uint16_t enabled_attr_mask;
+  uint16_t enabled_ubo_mask;
+  uint64_t enabled_tex_mask;
+  /** Opengl Location of builtin uniforms. Fast access, no lookup needed. */
+  /* TODO replace by location only array. */
+  GPUShaderInput builtins[GPU_NUM_UNIFORMS];
+  /** Flat array. In this order: Attributes, Ubos, Uniforms. */
+  GPUShaderInput inputs[0];
 } GPUShaderInterface;
 
 GPUShaderInterface *GPU_shaderinterface_create(int32_t program_id);
diff --git a/source/blender/gpu/GPU_vertex_format.h b/source/blender/gpu/GPU_vertex_format.h
index 7adad2ff831..61b14a4c5c0 100644
--- a/source/blender/gpu/GPU_vertex_format.h
+++ b/source/blender/gpu/GPU_vertex_format.h
@@ -101,12 +101,11 @@ typedef struct GPUVertFormat {
   char names[GPU_VERT_ATTR_NAMES_BUF_LEN];
 } GPUVertFormat;
 
-struct GPUShaderInterface;
+struct GPUShader;
 
 void GPU_vertformat_clear(GPUVertFormat *);
 void GPU_vertformat_copy(GPUVertFormat *dest, const GPUVertFormat *src);
-void GPU_vertformat_from_interface(GPUVertFormat *format,
-                                   const struct GPUShaderInterface *shaderface);
+void GPU_vertformat_from_shader(GPUVertFormat *format, const struct GPUShader *shader);
 
 uint GPU_vertformat_attr_add(
     GPUVertFormat *, const char *name, GPUVertCompType, uint comp_len, GPUVertFetchMode);
diff --git a/source/blender/gpu/intern/gpu_batch.c b/source/blender/gpu/intern/gpu_batch.c
index 3e0a1e57664..3dd43431393 100644
--- a/source/blender/gpu/intern/gpu_batch.c
+++ b/source/blender/gpu/intern/gpu_batch.c
@@ -375,7 +375,7 @@ void GPU_batch_program_set_no_use(GPUBatch *batch,
                                   const GPUShaderInterface *shaderface)
 {
 #if TRUST_NO_ONE
-  assert(glIsProgram(shaderface->program));
+  assert(glIsProgram(program));
   assert(batch->program_in_use == 0);
 #endif
   batch->interface = shaderface;
diff --git a/source/blender/gpu/intern/gpu_shader_interface.c b/source/blender/gpu/intern/gpu_shader_interface.c
index 3218d12bc0d..5471333a4c8 100644
--- a/source/blender/gpu/intern/gpu_shader_interface.c
+++ b/source/blender/gpu/intern/gpu_shader_interface.c
@@ -24,6 +24,9 @@
  */
 
 #include "BKE_global.h"
+
+#include "BLI_math_base.h"
+
 #include "MEM_guardedalloc.h"
 
 #include "GPU_shader_interface.h"
@@ -91,131 +94,132 @@ GPU_INLINE uint hash_string(const char *str)
   return i;
 }
 
-GPU_INLINE void set_input_name(GPUShaderInterface *shaderface,
-                               GPUShaderInput *input,
-                               const char *name,
-                               uint32_t name_len)
+GPU_INLINE uint32_t set_input_name(GPUShaderInterface *shaderface,
+                                   GPUShaderInput *input,
+                                   char *name,
+                                   uint32_t name_len)
 {
-  input->name_offset = shaderface->name_buffer_offset;
-  input->name_hash = hash_string(name);
-  shaderface->name_buffer_offset += name_len + 1; /* include NULL terminator */
-}
+  /* remove "[0]" from array name */
+  if (name[name_len - 1] == ']') {
+    name[name_len - 3] = '\0';
+    name_len -= 3;
+  }
 
-GPU_INLINE void shader_input_to_bucket(GPUShaderInput *input,
-                                       GPUShaderInput *buckets[GPU_NUM_SHADERINTERFACE_BUCKETS])
-{
-  const uint bucket_index = input->name_hash % GPU_NUM_SHADERINTERFACE_BUCKETS;
-  input->next = buckets[bucket_index];
-  buckets[bucket_index] = input;
+  input->name_offset = (uint32_t)(name - shaderface->name_buffer);
+  input->name_hash = hash_string(name);
+  return name_len + 1; /* include NULL terminator */
 }
 
-GPU_INLINE const GPUShaderInput *buckets_lookup(
-    GPUShaderInput *const buckets[GPU_NUM_SHADERINTERFACE_BUCKETS],
-    const char *name_buffer,
-    const char *name)
+GPU_INLINE const GPUShaderInput *input_lookup(const GPUShaderInterface *shaderface,
+                                              const GPUShaderInput *const inputs,
+                                              const uint inputs_len,
+                                              const char *name)
 {
   const uint name_hash = hash_string(name);
-  const uint bucket_index = name_hash % GPU_NUM_SHADERINTERFACE_BUCKETS;
-  const GPUShaderInput *input = buckets[bucket_index];
-  if (input == NULL) {
-    /* Requested uniform is not found at all. */
-    return NULL;
-  }
-  /* Optimization bit: if there is no hash collision detected when constructing shader interface
-   * it means we can only request the single possible uniform. Surely, it's possible we request
-   * uniform which causes hash collision, but that will be detected in debug builds. */
-  if (input->next == NULL) {
-    if (name_hash == input->name_hash) {
-#if TRUST_NO_ONE
-      assert(match(name_buffer + input->name_offset, name));
-#endif
-      return input;
-    }
-    return NULL;
-  }
-  /* Work through possible collisions. */
-  const GPUShaderInput *next = input;
-  while (next != NULL) {
-    input = next;
-    next = input->next;
-    if (input->name_hash != name_hash) {
-      continue;
-    }
-    if (match(name_buffer + input->name_offset, name)) {
-      return input;
+  /* Simple linear search for now. */
+  for (int i = inputs_len - 1; i >= 0; i--) {
+    if (inputs[i].name_hash == name_hash) {
+      if ((i > 0) && UNLIKELY(inputs[i - 1].name_hash == name_hash)) {
+        /* Hash colision resolve. */
+        for (; i >= 0 && inputs[i].name_hash == name_hash; i--) {
+          if (match(name, shaderface->name_buffer + inputs[i].name_offset)) {
+            return inputs + i; /* not found */
+          }
+        }
+        return NULL; /* not found */
+      }
+      else {
+        /* This is a bit dangerous since we could have a hash collision.
+         * where the asked uniform that does not exist has the same hash
+         * as a real uniform. */
+        BLI_assert(match(name, shaderface->name_buffer + inputs[i].name_offset));
+        return inputs + i;
+      }
     }
   }
   return NULL; /* not found */
 }
 
-GPU_INLINE void buckets_free(GPUShaderInput *buckets[GPU_NUM_SHADERINTERFACE_BUCKETS])
+/* Note that this modify the src arr

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list