DRW: Resource Handle: Use manual bitmasks and bitsiftstmp-drw-callbatching

This is in order to avoid implementation dependant behavior.

4 files changed, 119 insertions, 79 deletions

diff --git a/source/blender/draw/intern/draw_manager.c b/source/blender/draw/intern/draw_manager.c
index 66defdb6c89..6940ae6a7cb 100644
--- a/source/blender/draw/intern/draw_manager.c
+++ b/source/blender/draw/intern/draw_manager.c
@@ -612,7 +612,7 @@ static void draw_unit_state_create(void)
   culling->bsphere.radius = -1.0f;
   culling->user_data = NULL;
 
-  INCREMENT_RESOURCE_HANDLE(DST.resource_handle);
+  DRW_handle_increment(&DST.resource_handle);
 }
 
 /* It also stores viewport variable to an immutable place: DST
@@ -675,8 +675,8 @@ static void drw_viewport_var_init(void)
       DST.vmempool->images = BLI_memblock_create(sizeof(GPUTexture *));
     }
 
-    DST.resource_handle.value = 0;
-    DST.pass_handle.value = 0;
+    DST.resource_handle = 0;
+    DST.pass_handle = 0;
 
     draw_unit_state_create();
 
@@ -1121,7 +1121,7 @@ static void drw_engines_world_update(Scene *scene)
 
 static void drw_engines_cache_populate(Object *ob)
 {
-  DST.ob_handle.value = 0;
+  DST.ob_handle = 0;
 
   /* HACK: DrawData is copied by COW from the duplicated object.
    * This is valid for IDs that cannot be instantiated but this
@@ -2085,7 +2085,7 @@ void DRW_render_object_iter(
     if ((object_type_exclude_viewport & (1 << ob->type)) == 0) {
       DST.dupli_parent = data_.dupli_parent;
       DST.dupli_source = data_.dupli_object_current;
-      DST.ob_handle.value = 0;
+      DST.ob_handle = 0;
       drw_duplidata_load(DST.dupli_source);
 
       if (!DST.dupli_source) {
diff --git a/source/blender/draw/intern/draw_manager.h b/source/blender/draw/intern/draw_manager.h
index 154b35077a0..051d6a4549a 100644
--- a/source/blender/draw/intern/draw_manager.h
+++ b/source/blender/draw/intern/draw_manager.h
@@ -31,6 +31,7 @@
 #include "BLI_assert.h"
 #include "BLI_linklist.h"
 #include "BLI_threads.h"
+#include "BLI_memblock.h"
 
 #include "GPU_batch.h"
 #include "GPU_context.h"
@@ -103,35 +104,60 @@ typedef struct DRWCullingState {
   void *user_data;
 } DRWCullingState;
 
-/* We count on the fact that unsigned integers wrap around whe overflowing. */
-#define INCREMENT_RESOURCE_HANDLE(handle) \
-  do { \
-    if ((handle).id++ == 511) { \
-      (handle).chunk++; \
-    } \
-  } while (0)
-
 /* Minimum max UBO size is 64KiB. We take the largest
  * UBO struct and alloc the max number.
  * ((1 << 16) / sizeof(DRWObjectMatrix)) = 512
  * Keep in sync with common_view_lib.glsl */
 #define DRW_RESOURCE_CHUNK_LEN 512
 
-typedef struct DRWResourceHandle {
-  union {
-    /* TODO order correctly and make sure
-     * endianness does not change anything. */
-    struct {
-      uint32_t negative_scale : 1;
-      uint32_t id : 9;
-      uint32_t chunk : 22;
-    };
-    /** Use this to read the whole handle value as one 32bit uint.
-     *  Useful for sorting and test.
-     */
-    uint32_t value;
-  };
-} DRWResourceHandle;
+/**
+ * Identifier used to sort similar drawcalls together.
+ * Also used to reference elements inside memory blocks.
+ *
+ * From MSB to LSB
+ * 1 bit for negative scale.
+ * 22 bits for chunk id.
+ * 9 bits for resource id inside the chunk. (can go up to 511)
+ * |-|----------------------|---------|
+ *
+ * Use manual bitsift and mask instead of bitfields to avoid
+ * compiler dependant behavior that would mess the ordering of
+ * the members thus changing the sorting order.
+ */
+typedef uint32_t DRWResourceHandle;
+
+BLI_INLINE uint32_t DRW_handle_negative_scale_get(const DRWResourceHandle *handle)
+{
+  return (*handle & 0x80000000) != 0;
+}
+
+BLI_INLINE uint32_t DRW_handle_chunk_get(const DRWResourceHandle *handle)
+{
+  return (*handle & 0x7FFFFFFF) >> 9;
+}
+
+BLI_INLINE uint32_t DRW_handle_id_get(const DRWResourceHandle *handle)
+{
+  return (*handle & 0x000001FF);
+}
+
+BLI_INLINE void DRW_handle_increment(DRWResourceHandle *handle)
+{
+  *handle += 1;
+}
+
+BLI_INLINE void DRW_handle_negative_scale_enable(DRWResourceHandle *handle)
+{
+  *handle |= 0x80000000;
+}
+
+BLI_INLINE void *DRW_memblock_elem_from_handle(struct BLI_memblock *memblock,
+                                               const DRWResourceHandle *handle)
+{
+  int elem = DRW_handle_id_get(handle);
+  int chunk = DRW_handle_chunk_get(handle);
+  return BLI_memblock_elem_get(memblock, chunk, elem);
+}
 
 typedef struct DRWObjectMatrix {
   float model[4][4];
diff --git a/source/blender/draw/intern/draw_manager_data.c b/source/blender/draw/intern/draw_manager_data.c
index 301d2993bf2..6c91d8321a2 100644
--- a/source/blender/draw/intern/draw_manager_data.c
+++ b/source/blender/draw/intern/draw_manager_data.c
@@ -98,7 +98,9 @@ void DRW_uniformbuffer_free(GPUUniformBuffer *ubo)
 
 void drw_resource_buffer_finish(ViewportMemoryPool *vmempool)
 {
-  int ubo_len = 1 + DST.resource_handle.chunk - ((DST.resource_handle.id == 0) ? 1 : 0);
+  int chunk_id = DRW_handle_chunk_get(&DST.resource_handle);
+  int elem_id = DRW_handle_id_get(&DST.resource_handle);
+  int ubo_len = 1 + chunk_id - ((elem_id == 0) ? 1 : 0);
   size_t list_size = sizeof(GPUUniformBuffer *) * ubo_len;
 
   /* TODO find a better system. currently a lot of obinfos UBO are going to be unused
@@ -538,9 +540,11 @@ static DRWResourceHandle drw_resource_handle_new(float (*obmat)[4], Object *ob)
   UNUSED_VARS(ob_infos);
 
   DRWResourceHandle handle = DST.resource_handle;
-  INCREMENT_RESOURCE_HANDLE(DST.resource_handle);
+  DRW_handle_increment(&DST.resource_handle);
 
-  handle.negative_scale = (ob && (ob->transflag & OB_NEG_SCALE)) ? 1 : 0;
+  if (ob && (ob->transflag & OB_NEG_SCALE)) {
+    DRW_handle_negative_scale_enable(&handle);
+  }
 
   drw_call_matrix_init(ob_mats, ob, obmat);
   drw_call_culling_init(culling, ob);
@@ -555,7 +559,7 @@ static DRWResourceHandle drw_resource_handle(DRWShadingGroup *shgroup,
 {
   if (ob == NULL) {
     if (obmat == NULL) {
-      DRWResourceHandle handle = {.value = 0};
+      DRWResourceHandle handle = 0;
       return handle;
     }
     else {
@@ -563,7 +567,7 @@ static DRWResourceHandle drw_resource_handle(DRWShadingGroup *shgroup,
     }
   }
   else {
-    if (DST.ob_handle.value == 0) {
+    if (DST.ob_handle == 0) {
       DST.ob_handle = drw_resource_handle_new(obmat, ob);
       DST.ob_state_obinfo_init = false;
     }
@@ -571,9 +575,8 @@ static DRWResourceHandle drw_resource_handle(DRWShadingGroup *shgroup,
     if (shgroup->objectinfo) {
       if (!DST.ob_state_obinfo_init) {
         DST.ob_state_obinfo_init = true;
-
-        DRWObjectInfos *ob_infos = BLI_memblock_elem_get(
-            DST.vmempool->obinfos, DST.ob_handle.chunk, DST.ob_handle.id);
+        DRWObjectInfos *ob_infos = DRW_memblock_elem_from_handle(DST.vmempool->obinfos,
+                                                                 &DST.ob_handle);
 
         drw_call_obinfos_init(ob_infos, ob);
       }
@@ -700,8 +703,8 @@ void DRW_shgroup_call_ex(DRWShadingGroup *shgroup,
 
   /* Culling data. */
   if (user_data || bypass_culling) {
-    DRWCullingState *culling = BLI_memblock_elem_get(
-        DST.vmempool->cullstates, handle.chunk, handle.id);
+    DRWCullingState *culling = DRW_memblock_elem_from_handle(DST.vmempool->cullstates,
+                                                             &DST.ob_handle);
 
     if (user_data) {
       culling->user_data = user_data;
@@ -1248,7 +1251,7 @@ DRWShadingGroup *DRW_shgroup_get_next(DRWShadingGroup *shgroup)
 
 /* This is a workaround function waiting for the clearing operation to be available inside the
  * shgroups. */
-uint DRW_shgroup_stencil_mask_get(DRWShadingGroup *shgroup)
+uint DRW_shgroup_stencil_mask_get(DRWShadingGroup *UNUSED(shgroup))
 {
   /* TODO remove. This is broken. */
   return 0;
@@ -1263,8 +1266,8 @@ DRWShadingGroup *DRW_shgroup_create_sub(DRWShadingGroup *shgroup)
   shgroup_new->cmd.first = NULL;
   shgroup_new->cmd.last = NULL;
 
-  DRWPass *parent_pass = BLI_memblock_elem_get(
-      DST.vmempool->passes, shgroup->pass_handle.chunk, shgroup->pass_handle.id);
+  DRWPass *parent_pass = DRW_memblock_elem_from_handle(DST.vmempool->passes,
+                                                       &shgroup->pass_handle);
 
   BLI_LINKS_INSERT_AFTER(&parent_pass->shgroups, shgroup, shgroup_new);
 
@@ -1762,7 +1765,7 @@ DRWPass *DRW_pass_create(const char *name, DRWState state)
   pass->shgroups.first = NULL;
   pass->shgroups.last = NULL;
   pass->handle = DST.pass_handle;
-  INCREMENT_RESOURCE_HANDLE(DST.pass_handle);
+  DRW_handle_increment(&DST.pass_handle);
 
   return pass;
 }
@@ -1854,20 +1857,20 @@ void DRW_pass_sort_shgroup_z(DRWPass *pass)
   uint index = 0;
   DRWShadingGroup *shgroup = pass->shgroups.first;
   do {
-    DRWResourceHandle handle = {.value = 0};
+    DRWResourceHandle handle = 0;
     /* Find first DRWCommandDraw. */
     DRWCommandChunk *cmd_chunk = shgroup->cmd.first;
-    for (; cmd_chunk && handle.value == 0; cmd_chunk = cmd_chunk->next) {
-      for (int i = 0; i < cmd_chunk->command_used && handle.value == 0; i++) {
+    for (; cmd_chunk && handle == 0; cmd_chunk = cmd_chunk->next) {
+      for (int i = 0; i < cmd_chunk->command_used && handle == 0; i++) {
         if (DRW_CMD_DRAW == command_type_get(cmd_chunk->command_type, i)) {
           handle = cmd_chunk->commands[i].draw.handle;
         }
       }
     }
     /* To be sorted a shgroup needs to have at least one draw command.  */
-    BLI_assert(handle.value != 0);
+    BLI_assert(handle != 0);
 
-    DRWObjectMatrix *obmats = BLI_memblock_elem_get(DST.vmempool->obmats, handle.chunk, handle.id);
+    DRWObjectMatrix *obmats = DRW_memblock_elem_from_handle(DST.vmempool->obmats, &handle);
 
     /* Compute distance to camera. */
     float tmp[3];
diff --git a/source/blender/draw/intern/draw_manager_exec.c b/source/blender/draw/intern/draw_manager_exec.c
index 34f45cfe13d..508768b4613 100644
--- a/source/blender/draw/intern/draw_manager_exec.c
+++ b/source/blender/draw/intern/draw_manager_exec.c
@@ -416,10 +416,9 @@ void DRW_state_reset(void)
 /** \name Culling (DRW_culling)
  * \{ */
 
-static bool draw_call_is_culled(DRWResourceHandle handle, DRWView *view)
+static bool draw_call_is_culled(const DRWResourceHandle *handle, DRWView *view)
 {
-  DRWCullingState *culling = BLI_memblock_elem_get(
-      DST.vmempool->cullstates, handle.chunk, handle.id);
+  DRWCullingState *culling = DRW_memblock_elem_from_handle(DST.vmempool->cullstates, handle);
   return (culling->mask & view->culling_mask) != 0;
 }
 
@@ -600,13 +599,13 @@ static void draw_compute_culling(DRWView *view)
  * \{ */
 
 BLI_INLINE void draw_legacy_matrix_update(DRWShadingGroup *shgroup,
-                                          DRWResourceHandle handle,
+                                          DRWResourceHandle *handle,
                                           float obmat_loc,
                                           float obinv_loc,
                                           float mvp_loc)
 {
   /* Still supported for compatibility with gpu_shader_* but should be forbidden. */
-  DRWObjectMatrix *ob_mats = BLI_memblock_elem_get(DST.vmempool->obmats, handle.chunk, handle.id);
+  DRWObjectMatrix *ob_mats = DRW_memblock_elem_from_handle(DST.vmempool->obmats, handle);
   if (obmat_loc != -1) {
     GPU_shader_uniform_vector(shgroup->shader, obmat_loc, 16, 1, (float *)ob_mats->model);
   }
@@ -833,8 +832,8 @@ static bool ubo_bindings_validate(DRWShadingGroup *shgroup)
         valid = false;
       }
 
-      DRWPass *parent_pass = BLI_memblock_elem_get(
-          DST.vmempool->passes, shgroup->pass_handle.chunk, shgroup->pass_handle.id);
+      DRWPass *parent_pass = DRW_memblock_elem_from_handle(DST.vmempool->passes,
+                                                           &shgroup->pass_handle);
 
       printf("Pass : %s, Shader : %s, Block : %s\n",
              parent_pass->name,
@@ -972,7 +971,7 @@ static void draw_update_uniforms(DRWShadingGroup *shgroup,
 BLI_INLINE void draw_select_buffer(DRWShadingGroup *shgroup,
                                    DRWCommandsState *state,
                                    GPUBatch *batch,
-                                   DRWResourceHandle handle)
+                                   const DRWResourceHandle *handle)
 {
   const bool is_instancing = (batch->inst != NULL);
   int start = 0;
@@ -998,7 +997,8 @@ BLI_INLINE void draw_select_buffer(DRWShadingGroup *shgroup,
       draw_geometry_execute(shgroup, batch, 0, 0, start, count, state->baseinst_loc);
     }
     else {
-      draw_geometry_execute(shgroup, batch, start, count, handle.id, 0, state->baseinst_loc);
+      draw_geometry_execute(
+          shgroup, batch, start, count, DRW_handle_id_get(handle), 0, state->baseinst_loc);
     }
     start += count;
   }
@@ -1032,27 +1032,29 @@ static DRWCommand *draw_command_iter_step(DRWCommandIterator *iter, eDRWCommandT
   return NULL;
 }
 
-static void draw_call_resource_bind(DRWCommandsState *state, DRWResourceHandle handle)
+static void draw_call_resource_bind(DRWCommandsState *state, const DRWResourceHandle *handle)
 {
   /* Front face is not a resource but it is inside the resource handle. */
-  if (handle.negative_scale != state->neg_scale) {
-    glFrontFace((handle.negative_scale) ? GL_CW : GL_CCW);
-    state->neg_scale = handle.negative_scale;
+  bool neg_scale = DRW_handle_negative_scale_get(handle);
+  if (neg_scale != state->neg_scale) {
+    glFrontFace((neg_scale) ? GL_CW : GL_CCW);
+    state->neg_scale = neg_scale;
   }
 
-  if (state->resource_chunk != handle.chunk) {
+  int chunk = DRW_handle_chunk_get(handle);
+  if (state->resource_chunk != chunk) {
     if (state->chunkid_loc != -1) {
-      GPU_shader_uniform_int(NULL, state->chunkid_loc, handle.chunk);
+      GPU_shader_uniform_int(NULL, state->chunkid_loc, chunk);
     }
     if (state->obmats_loc != -1) {
       GPU_uniformbuffer_unbind(DST.vmempool->matrices_ubo[state->resource_chunk]);
-      GPU_uniformbuffer_bind(DST.vmempool->matrices_ubo[handle.chunk], 0);
+      GPU_uniformbuffer_bind(DST.vmempool->matrices_ubo[chunk], 0);
     }
     if (state->obinfos_loc != -1) {
       GPU_uniformbuffer_unbind(DST.vmempool->obinfos_ubo[state->resource_chunk]);
-      GPU_uniformbuffer_bind(DST.vmempool->obinfos_ubo[handle.chunk], 1);
+      GPU_uniformbuffer_bind(DST.vmempool->obinfos_ubo[chunk], 1);
     }
-    state->resource_chunk = handle.chunk;
+    state->resource_chunk = chunk;
   }
 }
 
@@ -1076,17 +1078,18 @@ static void draw_call_single_do(DRWShadingGroup *shgroup,
 {
   draw_call_batching_flush(shgroup, state);
 
-  draw_call_resource_bind(state, handle);
+  draw_call_resource_bind(state, &handle);
 
   /* TODO This is Legacy. Need to be removed. */
   if (state->obmats_loc == -1 &&
       (state->obmat_loc != -1 || state->obinv_loc != -1 || state->mvp_loc != -1)) {
-    draw_legacy_matrix_update(shgroup, handle, state->obmat_loc, state->obinv_loc, state->mvp_loc);
+    draw_legacy_matrix_update(
+        shgroup, &handle, state->obmat_loc, state->obinv_loc, state->mvp_loc);
   }
 
   if (G.f & G_FLAG_PICKSEL) {
     if (state->select_buf != NULL) {
-      draw_select_buffer(shgroup, state, batch, handle);
+      draw_select_buffer(shgroup, state, batch, &handle);
       return;
     }
     else {
@@ -1094,8 +1097,13 @@ static void draw_call_single_do(DRWShadingGroup *shgroup,
     }
   }
 
-  draw_geometry_execute(
-      shgroup, batch, vert_first, vert_count, handle.id, inst_count, state->baseinst_loc);
+  draw_geometry_execute(shgroup,
+                        batch,
+                        vert_first,
+                        vert_count,
+                        DRW_handle_id_get(&handle),
+                        inst_count,
+                        state->baseinst_loc);
 }
 
 static void draw_call_batching_start(DRWCommandsState *state)
@@ -1117,9 +1125,12 @@ static void draw_call_batching_do(DRWShadingGroup *shgroup,
                                   DRWCommandDraw *call)
 {
   /* If any condition requires to interupt the merging. */
-  if ((call->handle.negative_scale != state->neg_scale) || /* Need to change state. */
-      (call->handle.chunk != state->resource_chunk) ||     /* Need to change UBOs. */
-      (call->batch != state->batch)                        /* Need to change VAO. */
+  bool neg_scale = DRW_handle_negative_scale_get(&call->handle);
+  int chunk = DRW_handle_chunk_get(&call->handle);
+  int id = DRW_handle_id_get(&call->handle);
+  if ((state->neg_scale != neg_scale) ||  /* Need to change state. */
+      (state->resource_chunk != chunk) || /* Need to change UBOs. */
+      (state->batch != call->batch)       /* Need to change VAO. */
   ) {
     draw_call_batching_flush(shgroup, state);
 
@@ -1128,18 +1139,18 @@ static void draw_call_batching_do(DRWShadingGroup *shgroup,
     state->v_count = (call->batch->elem) ? call->batch->elem->index_len :
                                            call->batch->verts[0]->vertex_len;
     state->inst_count = 1;
-    state->base_inst = call->handle.id;
+    state->base_inst = id;
 
-    draw_call_resource_bind(state, call->handle);
+    draw_call_resource_bind(state, &call->handle);
 
     GPU_draw_list_init(DST.draw_list, state->batch);
   }
   /* Is the id consecutive? */
-  else if (call->handle.id != state->base_inst + state->inst_count) {
+  else if (id != state->base_inst + state->inst_count) {
     /* We need to add a draw command for the pending instances. */
     draw_indirect_call(shgroup, state);
     state->inst_count = 1;
-    state->base_inst = call->handle.id;
+    state->base_inst = id;
   }
   /* We avoid a drawcall by merging with the precedent
    * drawcall using instancing. */
@@ -1224,7 +1235,7 @@ static void draw_shgroup(DRWShadingGroup *shgroup, DRWState pass_state)
         case DRW_CMD_DRAW:
         case DRW_CMD_DRAW_PROCEDURAL:
         case DRW_CMD_DRAW_INSTANCE:
-          if (draw_call_is_culled(cmd->instance.handle, DST.view_active)) {
+          if (draw_call_is_culled(&cmd->instance.handle, DST.view_active)) {
             continue;
           }
           break;
@@ -1275,7 +1286,7 @@ static void draw_shgroup(DRWShadingGroup *shgroup, DRWState pass_state)
           draw_call_single_do(shgroup,
                               &state,
                               cmd->range.batch,
-                              (DRWResourceHandle){.value = 0},
+                              (DRWResourceHandle)0,
                               cmd->range.vert_first,
                               cmd->range.vert_count,
                               1);