diff options
author | Clément Foucault <foucault.clem@gmail.com> | 2019-08-14 23:18:47 +0300 |
---|---|---|
committer | Clément Foucault <foucault.clem@gmail.com> | 2019-08-15 00:59:33 +0300 |
commit | deb5416a1a50e153cf2f9e3809755a5e82bd8f85 (patch) | |
tree | 4884568ffb2bac6b78c681569ce18260795d06b3 /source/blender | |
parent | 67f49f9c03c95eb3a62ece38db0ae8feafd4992e (diff) |
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.
Diffstat (limited to 'source/blender')
-rw-r--r-- | source/blender/draw/intern/draw_cache_extract_mesh.c | 43 | ||||
-rw-r--r-- | source/blender/draw/intern/draw_cache_impl_mesh.c | 16 | ||||
-rw-r--r-- | source/blender/gpu/GPU_shader.h | 4 | ||||
-rw-r--r-- | source/blender/gpu/GPU_vertex_format.h | 12 | ||||
-rw-r--r-- | source/blender/gpu/intern/gpu_codegen.c | 20 | ||||
-rw-r--r-- | source/blender/gpu/intern/gpu_vertex_format.c | 63 |
6 files changed, 108 insertions, 50 deletions
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_appendf(ds, - "DEFINE_ATTR(%s, %s%u);\n", + "DEFINE_ATTR(%s, %s%s);\n", GPU_DATATYPE_STR[input->type], attr_prefix_get(input->attr_type), - hash); - BLI_dynstr_appendf( - ds, "#define att%d %s%u\n", input->attr_id, attr_prefix_get(input->attr_type), hash); + attr_safe_name); + BLI_dynstr_appendf(ds, + "#define att%d %s%s\n", + input->attr_id, + attr_prefix_get(input->attr_type), + attr_safe_name); /* Auto attribute can be vertex color byte buffer. * We need to know and convert them to linear space in VS. */ if (input->attr_type == CD_AUTO_FROM_NAME) { - BLI_dynstr_appendf(ds, "uniform bool ba%u;\n", hash); - BLI_dynstr_appendf(ds, "#define att%d_is_srgb ba%u\n", input->attr_id, hash); + BLI_dynstr_appendf(ds, "uniform bool ba%s;\n", attr_safe_name); + BLI_dynstr_appendf(ds, "#define att%d_is_srgb ba%s\n", input->attr_id, attr_safe_name); } } BLI_dynstr_appendf(ds, diff --git a/source/blender/gpu/intern/gpu_vertex_format.c b/source/blender/gpu/intern/gpu_vertex_format.c index f672d350afa..2cfb17b1568 100644 --- a/source/blender/gpu/intern/gpu_vertex_format.c +++ b/source/blender/gpu/intern/gpu_vertex_format.c @@ -31,6 +31,8 @@ #include <string.h> #include "BLI_utildefines.h" +#include "BLI_string.h" +#include "BLI_ghash.h" #define PACK_DEBUG 0 @@ -149,9 +151,9 @@ uint GPU_vertformat_attr_add(GPUVertFormat *format, GPUVertFetchMode fetch_mode) { #if TRUST_NO_ONE - assert(format->name_len < GPU_VERT_ATTR_MAX_LEN); /* there's room for more */ - assert(format->attr_len < GPU_VERT_ATTR_MAX_LEN); /* there's room for more */ - assert(!format->packed); /* packed means frozen/locked */ + assert(format->name_len < GPU_VERT_FORMAT_MAX_NAMES); /* there's room for more */ + assert(format->attr_len < GPU_VERT_ATTR_MAX_LEN); /* there's room for more */ + assert(!format->packed); /* packed means frozen/locked */ assert((comp_len >= 1 && comp_len <= 4) || comp_len == 8 || comp_len == 12 || comp_len == 16); switch (comp_type) { @@ -197,7 +199,7 @@ void GPU_vertformat_alias_add(GPUVertFormat *format, const char *alias) { GPUVertAttr *attr = &format->attrs[format->attr_len - 1]; #if TRUST_NO_ONE - assert(format->name_len < GPU_VERT_ATTR_MAX_LEN); /* there's room for more */ + assert(format->name_len < GPU_VERT_FORMAT_MAX_NAMES); /* there's room for more */ assert(attr->name_len < GPU_VERT_ATTR_MAX_NAMES); #endif format->name_len++; /* multiname support */ @@ -218,6 +220,59 @@ int GPU_vertformat_attr_id_get(const GPUVertFormat *format, const char *name) return -1; } +/* Encode 4 original bytes into 6 safe bytes. */ +static void safe_bytes(char out[6], const char data[4]) +{ + char safe_chars[63] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_"; + + uint32_t in = *(uint32_t *)data; + for (int i = 0; i < 6; i++) { + /* Encoding in base63 */ + out[i] = safe_chars[in % 63u]; + in /= 63u; + } +} + +#if 0 /* For when we can use 11chars names. */ +/* Encode 8 original bytes into 11 safe bytes. */ +static void safe_bytes(char out[11], const char data[8]) +{ + char safe_chars[63] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_"; + + uint64_t in = *(uint64_t *)data; + for (int i = 0; i < 11; i++) { + /* Encoding in base63 */ + out[i] = safe_chars[in % 63lu]; + in /= 63lu; + } +} +#endif + +/* Warning: Always add a prefix to the result of this function as + * the generated string can start with a number and not be a valid attribute name. */ +void GPU_vertformat_safe_attrib_name(const char *attrib_name, + char *r_safe_name, + uint UNUSED(max_len)) +{ + char data[4] = {0}; + /* We use a hash to identify each data layer based on its name. + * NOTE: This is still prone to hash collision but the risks are very low.*/ + *(uint *)data = BLI_ghashutil_strhash_p_murmur(attrib_name); + /* Convert to safe bytes characters. */ + safe_bytes(r_safe_name, data); + /* End the string */ + r_safe_name[6] = '\0'; + + /* TOOD(fclem) When UV and Tangent buffers will be separated, name buffer will have plenty more + * space. In this case, we can think of having ~13 chars for each name which would be enough to + * encode some of the input string along with the hash to reduce colision possibility. */ + + BLI_assert(GPU_MAX_SAFE_ATTRIB_NAME >= 7); +#if 0 /* For debugging */ + printf("%s > %x > %s\n", attrib_name, *(uint32_t *)data, r_safe_name); +#endif +} + /* Make attribute layout non-interleaved. * Warning! This does not change data layout! * Use direct buffer access to fill the data. |