From ca37654b63277985a4f050772ddc412d8158a529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Foucault?= Date: Thu, 31 Mar 2022 13:36:04 +0200 Subject: Fix T96920 DRW: Regression: Hair strands are drawn in wrong place This was caused by the recent changes made to the way we handle matrix copies. The matrix copy assumed that the uniform iteration was the same as creation order. But this was far from true. The reality was that the iterator was reverse for `unichunk` but not for `unichunk->uniforms` so this was recreating wrong matrix. I rewrote this part to always use reverse iteration and fix the copy destination. Also I simplified the code making the assumption this won't be used for anything else than mat4. --- source/blender/draw/intern/draw_manager_exec.c | 72 +++++++++----------------- 1 file changed, 24 insertions(+), 48 deletions(-) (limited to 'source/blender/draw/intern/draw_manager_exec.c') diff --git a/source/blender/draw/intern/draw_manager_exec.c b/source/blender/draw/intern/draw_manager_exec.c index 8ac2f61ad26..fbff3b29d24 100644 --- a/source/blender/draw/intern/draw_manager_exec.c +++ b/source/blender/draw/intern/draw_manager_exec.c @@ -587,59 +587,35 @@ static void draw_update_uniforms(DRWShadingGroup *shgroup, #define MAX_UNIFORM_STACK_SIZE 64 /* Uniform array elements stored as separate entries. We need to batch these together */ - int current_uniform_array_loc = -1; - unsigned int current_array_index = 0; - static union { - int istack[MAX_UNIFORM_STACK_SIZE]; - float fstack[MAX_UNIFORM_STACK_SIZE]; - } uniform_stack; - - /* Loop through uniforms. */ - for (DRWUniformChunk *unichunk = shgroup->uniforms; unichunk; unichunk = unichunk->next) { - DRWUniform *uni = unichunk->uniforms; + int array_uniform_loc = -1; + int array_index = 0; + float mat4_stack[4 * 4]; - for (int i = 0; i < unichunk->uniform_used; i++, uni++) { + /* Loop through uniforms in reverse order. */ + for (DRWUniformChunk *unichunk = shgroup->uniforms; unichunk; unichunk = unichunk->next) { + DRWUniform *uni = unichunk->uniforms + unichunk->uniform_used - 1; + for (int i = 0; i < unichunk->uniform_used; i++, uni--) { /* For uniform array copies, copy per-array-element data into local buffer before upload. */ - if (uni->arraysize > 1 && - (uni->type == DRW_UNIFORM_INT_COPY || uni->type == DRW_UNIFORM_FLOAT_COPY)) { - + if (uni->arraysize > 1 && uni->type == DRW_UNIFORM_FLOAT_COPY) { + /* Only written for mat4 copy for now and is not meant to become generalized. */ + /* TODO(@fclem): Use UBOs/SSBOs instead of inline mat4 copies. */ + BLI_assert(uni->arraysize == 4 && uni->length == 4); /* Begin copying uniform array. */ - if (current_array_index == 0) { - current_uniform_array_loc = uni->location; + if (array_uniform_loc == -1) { + array_uniform_loc = uni->location; + array_index = uni->arraysize * uni->length; } - /* Debug check same array loc. */ - BLI_assert(current_uniform_array_loc > -1); - BLI_assert(current_uniform_array_loc == uni->location); - + BLI_assert(array_uniform_loc > -1 && array_uniform_loc == uni->location); /* Copy array element data to local buffer. */ - BLI_assert(((current_array_index + 1) * uni->length) <= MAX_UNIFORM_STACK_SIZE); - if (uni->type == DRW_UNIFORM_INT_COPY) { - memcpy(&uniform_stack.istack[current_array_index * uni->length], - uni->ivalue, - sizeof(int) * uni->length); - } - else { - memcpy(&uniform_stack.fstack[current_array_index * uni->length], - uni->fvalue, - sizeof(float) * uni->length); - } - current_array_index++; - BLI_assert(current_array_index <= uni->arraysize); - + array_index -= uni->length; + memcpy(&mat4_stack[array_index], uni->fvalue, sizeof(float) * uni->length); /* Flush array data to shader. */ - if (current_array_index == uni->arraysize) { - if (uni->type == DRW_UNIFORM_INT_COPY) { - GPU_shader_uniform_vector_int( - shgroup->shader, uni->location, uni->length, uni->arraysize, uniform_stack.istack); - } - else { - GPU_shader_uniform_vector( - shgroup->shader, uni->location, uni->length, uni->arraysize, uniform_stack.fstack); - } - current_array_index = 0; - current_uniform_array_loc = -1; + if (array_index <= 0) { + GPU_shader_uniform_vector( + shgroup->shader, uni->location, uni->length, uni->arraysize, mat4_stack); + array_uniform_loc = -1; } continue; } @@ -738,9 +714,9 @@ static void draw_update_uniforms(DRWShadingGroup *shgroup, } } /* Ensure uniform arrays copied. */ - BLI_assert(current_array_index == 0); - BLI_assert(current_uniform_array_loc == -1); - UNUSED_VARS_NDEBUG(current_uniform_array_loc); + BLI_assert(array_index == 0); + BLI_assert(array_uniform_loc == -1); + UNUSED_VARS_NDEBUG(array_uniform_loc); } BLI_INLINE void draw_select_buffer(DRWShadingGroup *shgroup, -- cgit v1.2.3