[Bf-blender-cvs] [deb5416a1a5] master: GPU: Vertex Format: ADd function for safe GLSL attrib name
Clément Foucault
noreply at git.blender.org
Thu Aug 15 00:00:23 CEST 2019
Commit: deb5416a1a50e153cf2f9e3809755a5e82bd8f85
Author: Clément Foucault
Date: Wed Aug 14 22:18:47 2019 +0200
Branches: master
https://developer.blender.org/rBdeb5416a1a50e153cf2f9e3809755a5e82bd8f85
GPU: Vertex Format: ADd function for safe GLSL attrib name
This remove code duplication and use base63 encoding of the hash.
Use mumur hash to have more randomness.
===================================================================
M source/blender/draw/intern/draw_cache_extract_mesh.c
M source/blender/draw/intern/draw_cache_impl_mesh.c
M source/blender/gpu/GPU_shader.h
M source/blender/gpu/GPU_vertex_format.h
M source/blender/gpu/intern/gpu_codegen.c
M source/blender/gpu/intern/gpu_vertex_format.c
===================================================================
diff --git a/source/blender/draw/intern/draw_cache_extract_mesh.c b/source/blender/draw/intern/draw_cache_extract_mesh.c
index f9d6c9ed582..ea1813464c3 100644
--- a/source/blender/draw/intern/draw_cache_extract_mesh.c
+++ b/source/blender/draw/intern/draw_cache_extract_mesh.c
@@ -1570,23 +1570,17 @@ static void *extract_uv_tan_init(const MeshRenderData *mr, void *buf)
bool orco_allocated = false;
const bool use_orco_tan = mr->cache->cd_used.tan_orco != 0;
- /* XXX FIXME XXX */
- /* We use a hash to identify each data layer based on its name.
- * Gawain then search for this name in the current shader and bind if it exists.
- * NOTE : This is prone to hash collision.
- * One solution to hash collision would be to format the cd layer name
- * to a safe glsl var name, but without name clash.
- * NOTE 2 : Replicate changes to code_generate_vertex_new() in gpu_codegen.c */
for (int i = 0; i < MAX_MTFACE; i++) {
if (uv_layers & (1 << i)) {
- char attr_name[32];
+ char attr_name[32], attr_safe_name[GPU_MAX_SAFE_ATTRIB_NAME];
const char *layer_name = CustomData_get_layer_name(cd_ldata, CD_MLOOPUV, i);
- uint hash = BLI_ghashutil_strhash_p(layer_name);
+
+ GPU_vertformat_safe_attrib_name(layer_name, attr_safe_name, GPU_MAX_SAFE_ATTRIB_NAME);
/* UV layer name. */
- BLI_snprintf(attr_name, sizeof(attr_name), "u%u", hash);
+ BLI_snprintf(attr_name, sizeof(attr_name), "u%s", attr_safe_name);
GPU_vertformat_attr_add(&format, attr_name, GPU_COMP_F32, 2, GPU_FETCH_FLOAT);
/* Auto layer name. */
- BLI_snprintf(attr_name, sizeof(attr_name), "a%u", hash);
+ BLI_snprintf(attr_name, sizeof(attr_name), "a%s", attr_safe_name);
GPU_vertformat_alias_add(&format, attr_name);
/* Active render layer name. */
if (i == CustomData_get_render_layer(cd_ldata, CD_MLOOPUV)) {
@@ -1610,11 +1604,11 @@ static void *extract_uv_tan_init(const MeshRenderData *mr, void *buf)
for (int i = 0; i < MAX_MTFACE; i++) {
if (tan_layers & (1 << i)) {
- char attr_name[32];
+ char attr_name[32], attr_safe_name[GPU_MAX_SAFE_ATTRIB_NAME];
const char *layer_name = CustomData_get_layer_name(cd_ldata, CD_MLOOPUV, i);
- uint hash = BLI_ghashutil_strhash_p(layer_name);
+ GPU_vertformat_safe_attrib_name(layer_name, attr_safe_name, GPU_MAX_SAFE_ATTRIB_NAME);
/* Tangent layer name. */
- BLI_snprintf(attr_name, sizeof(attr_name), "t%u", hash);
+ BLI_snprintf(attr_name, sizeof(attr_name), "t%s", attr_safe_name);
GPU_vertformat_attr_add(&format, attr_name, GPU_COMP_F32, 4, GPU_FETCH_FLOAT);
/* Active render layer name. */
if (i == CustomData_get_render_layer(cd_ldata, CD_MLOOPUV)) {
@@ -1687,10 +1681,10 @@ static void *extract_uv_tan_init(const MeshRenderData *mr, void *buf)
}
if (use_orco_tan) {
- char attr_name[32];
+ char attr_name[32], attr_safe_name[GPU_MAX_SAFE_ATTRIB_NAME];
const char *layer_name = CustomData_get_layer_name(cd_ldata, CD_TANGENT, 0);
- uint hash = BLI_ghashutil_strhash_p(layer_name);
- BLI_snprintf(attr_name, sizeof(*attr_name), "t%u", hash);
+ GPU_vertformat_safe_attrib_name(layer_name, attr_safe_name, GPU_MAX_SAFE_ATTRIB_NAME);
+ BLI_snprintf(attr_name, sizeof(*attr_name), "t%s", attr_safe_name);
GPU_vertformat_attr_add(&format, attr_name, GPU_COMP_F32, 4, GPU_FETCH_FLOAT);
GPU_vertformat_alias_add(&format, "t");
GPU_vertformat_alias_add(&format, "at");
@@ -1779,20 +1773,13 @@ static void *extract_vcol_init(const MeshRenderData *mr, void *buf)
CustomData *cd_ldata = &mr->me->ldata;
uint32_t vcol_layers = mr->cache->cd_used.vcol;
- /* XXX FIXME XXX */
- /* We use a hash to identify each data layer based on its name.
- * Gawain then search for this name in the current shader and bind if it exists.
- * NOTE : This is prone to hash collision.
- * One solution to hash collision would be to format the cd layer name
- * to a safe glsl var name, but without name clash.
- * NOTE 2 : Replicate changes to code_generate_vertex_new() in gpu_codegen.c */
for (int i = 0; i < 8; i++) {
if (vcol_layers & (1 << i)) {
- char attr_name[32];
+ char attr_name[32], attr_safe_name[GPU_MAX_SAFE_ATTRIB_NAME];
const char *layer_name = CustomData_get_layer_name(cd_ldata, CD_MLOOPCOL, i);
- uint hash = BLI_ghashutil_strhash_p(layer_name);
+ GPU_vertformat_safe_attrib_name(layer_name, attr_safe_name, GPU_MAX_SAFE_ATTRIB_NAME);
- BLI_snprintf(attr_name, sizeof(attr_name), "c%u", hash);
+ BLI_snprintf(attr_name, sizeof(attr_name), "c%s", attr_safe_name);
GPU_vertformat_attr_add(&format, attr_name, GPU_COMP_U8, 4, GPU_FETCH_INT_TO_FLOAT_UNIT);
if (i == CustomData_get_render_layer(cd_ldata, CD_MLOOPCOL)) {
@@ -1804,7 +1791,7 @@ static void *extract_vcol_init(const MeshRenderData *mr, void *buf)
/* Gather number of auto layers. */
/* We only do vcols that are not overridden by uvs */
if (CustomData_get_named_layer_index(cd_ldata, CD_MLOOPUV, layer_name) == -1) {
- BLI_snprintf(attr_name, sizeof(attr_name), "a%u", hash);
+ BLI_snprintf(attr_name, sizeof(attr_name), "a%s", attr_safe_name);
GPU_vertformat_alias_add(&format, attr_name);
}
}
diff --git a/source/blender/draw/intern/draw_cache_impl_mesh.c b/source/blender/draw/intern/draw_cache_impl_mesh.c
index f498771b596..b23dac838e6 100644
--- a/source/blender/draw/intern/draw_cache_impl_mesh.c
+++ b/source/blender/draw/intern/draw_cache_impl_mesh.c
@@ -244,10 +244,12 @@ static void mesh_cd_extract_auto_layers_names_and_srgb(Mesh *me,
for (int i = 0; i < uv_len; i++) {
if ((cd_used.uv & (1 << i)) != 0) {
const char *name = CustomData_get_layer_name(cd_ldata, CD_MLOOPUV, i);
- uint hash = BLI_ghashutil_strhash_p(name);
+ char safe_name[GPU_MAX_SAFE_ATTRIB_NAME];
+ GPU_vertformat_safe_attrib_name(name, safe_name, GPU_MAX_SAFE_ATTRIB_NAME);
+ auto_ofs += BLI_snprintf_rlen(
+ auto_names + auto_ofs, auto_names_len - auto_ofs, "ba%s", safe_name);
/* +1 to include '\0' terminator. */
- auto_ofs += 1 + BLI_snprintf_rlen(
- auto_names + auto_ofs, auto_names_len - auto_ofs, "ba%u", hash);
+ auto_ofs += 1;
}
}
@@ -257,10 +259,12 @@ static void mesh_cd_extract_auto_layers_names_and_srgb(Mesh *me,
const char *name = CustomData_get_layer_name(cd_ldata, CD_MLOOPCOL, i);
/* We only do vcols that are not overridden by a uv layer with same name. */
if (CustomData_get_named_layer_index(cd_ldata, CD_MLOOPUV, name) == -1) {
- uint hash = BLI_ghashutil_strhash_p(name);
+ char safe_name[GPU_MAX_SAFE_ATTRIB_NAME];
+ GPU_vertformat_safe_attrib_name(name, safe_name, GPU_MAX_SAFE_ATTRIB_NAME);
+ auto_ofs += BLI_snprintf_rlen(
+ auto_names + auto_ofs, auto_names_len - auto_ofs, "ba%s", safe_name);
/* +1 to include '\0' terminator. */
- auto_ofs += 1 + BLI_snprintf_rlen(
- auto_names + auto_ofs, auto_names_len - auto_ofs, "ba%u", hash);
+ auto_ofs += 1;
auto_is_srgb[auto_is_srgb_ofs] = true;
auto_is_srgb_ofs++;
}
diff --git a/source/blender/gpu/GPU_shader.h b/source/blender/gpu/GPU_shader.h
index 124f1f1ff8a..f4a94c7759a 100644
--- a/source/blender/gpu/GPU_shader.h
+++ b/source/blender/gpu/GPU_shader.h
@@ -396,7 +396,9 @@ void GPU_shader_free_builtin_shaders(void);
/* Vertex attributes for shaders */
-#define GPU_MAX_ATTR 32
+/* Hardware limit is 16. Position attribute is always needed so we reduce to 15.
+ * This makes sure the GPUVertexFormat name buffer does not overflow. */
+#define GPU_MAX_ATTR 15
typedef struct GPUVertAttrLayers {
struct {
diff --git a/source/blender/gpu/GPU_vertex_format.h b/source/blender/gpu/GPU_vertex_format.h
index fc468436499..97c1828b593 100644
--- a/source/blender/gpu/GPU_vertex_format.h
+++ b/source/blender/gpu/GPU_vertex_format.h
@@ -32,8 +32,10 @@
#define GPU_VERT_ATTR_MAX_LEN 16
#define GPU_VERT_ATTR_MAX_NAMES 6
-#define GPU_VERT_ATTR_NAME_AVERAGE_LEN 11
-#define GPU_VERT_ATTR_NAMES_BUF_LEN ((GPU_VERT_ATTR_NAME_AVERAGE_LEN + 1) * GPU_VERT_ATTR_MAX_LEN)
+#define GPU_VERT_ATTR_NAMES_BUF_LEN 256
+#define GPU_VERT_FORMAT_MAX_NAMES 63 /* More than enough, actual max is ~30. */
+/* Computed as GPU_VERT_ATTR_NAMES_BUF_LEN / 30 (actual max format name). */
+#define GPU_MAX_SAFE_ATTRIB_NAME 8
typedef enum {
GPU_COMP_I8,
@@ -80,8 +82,8 @@ BLI_STATIC_ASSERT(GPU_VERT_ATTR_NAMES_BUF_LEN <= 256,
typedef struct GPUVertFormat {
/** 0 to 16 (GPU_VERT_ATTR_MAX_LEN). */
uint attr_len : 5;
- /** Total count of active vertex attribute. */
- uint name_len : 5;
+ /** Total count of active vertex attribute names. (max GPU_VERT_FORMAT_MAX_NAMES) */
+ uint name_len : 6;
/** Stride in bytes, 1 to 1024. */
uint stride : 11;
/** Has the format been packed. */
@@ -117,6 +119,8 @@ BLI_INLINE const char *GPU_vertformat_attr_name_get(const GPUVertFormat *format,
return format->names + attr->names[n_idx];
}
+void GPU_vertformat_safe_attrib_name(const char *attrib_name, char *r_safe_name, uint max_len);
+
/* format conversion */
typedef struct GPUPackedNormal {
diff --git a/source/blender/gpu/intern/gpu_codegen.c b/source/blender/gpu/intern/gpu_codegen.c
index 3e635b3198a..0e15fdd000b 100644
--- a/source/blender/gpu/intern/gpu_codegen.c
+++ b/source/blender/gpu/intern/gpu_codegen.c
@@ -46,6 +46,7 @@
#include "GPU_shader.h"
#include "GPU_texture.h"
#include "GPU_uniformbuffer.h"
+#include "GPU_vertex_format.h"
#include "BLI_sys_types.h" /* for intptr_t support */
@@ -1011,19 +1012,24 @@ static char *code_generate_vertex(ListBase *nodes, const char *vert_code, bool u
ds, "#define att%d %s\n", input->attr_id, attr_prefix_get(input->attr_type));
}
else {
- uint hash = BLI_ghashutil_strhash_p(input->attr_name);
+ char attr_safe_name[GPU_MAX_SAFE_ATTRIB_NAME];
+ GPU_vertformat_safe_attrib_name(
+ input->attr_name, attr_safe_name, GPU_MAX_SAFE_ATTRIB_NAME);
BLI_dynstr_app
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list