From 55e3930b253e4c6687ff94b474cc9973e0c00520 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dietrich?= Date: Thu, 26 May 2022 14:51:44 +0200 Subject: Fix T98392: GPU subdivision crash with knife tools The face dots normals may not be always requested, thus leading to a crash by null pointer dereference. --- .../draw/intern/draw_cache_impl_subdivision.cc | 24 +++++++++++++++++++--- .../mesh_extractors/extract_mesh_vbo_fdots_pos.cc | 7 +++++-- .../common_subdiv_patch_evaluation_comp.glsl | 8 ++++++-- 3 files changed, 32 insertions(+), 7 deletions(-) (limited to 'source') diff --git a/source/blender/draw/intern/draw_cache_impl_subdivision.cc b/source/blender/draw/intern/draw_cache_impl_subdivision.cc index e3842ec2231..3a5593fa2a5 100644 --- a/source/blender/draw/intern/draw_cache_impl_subdivision.cc +++ b/source/blender/draw/intern/draw_cache_impl_subdivision.cc @@ -69,6 +69,7 @@ enum { SHADER_PATCH_EVALUATION, SHADER_PATCH_EVALUATION_FVAR, SHADER_PATCH_EVALUATION_FACE_DOTS, + SHADER_PATCH_EVALUATION_FACE_DOTS_WITH_NORMALS, SHADER_COMP_CUSTOM_DATA_INTERP_1D, SHADER_COMP_CUSTOM_DATA_INTERP_2D, SHADER_COMP_CUSTOM_DATA_INTERP_3D, @@ -107,7 +108,8 @@ static const char *get_shader_code(int shader_type) } case SHADER_PATCH_EVALUATION: case SHADER_PATCH_EVALUATION_FVAR: - case SHADER_PATCH_EVALUATION_FACE_DOTS: { + case SHADER_PATCH_EVALUATION_FACE_DOTS: + case SHADER_PATCH_EVALUATION_FACE_DOTS_WITH_NORMALS: { return datatoc_common_subdiv_patch_evaluation_comp_glsl; } case SHADER_COMP_CUSTOM_DATA_INTERP_1D: @@ -163,6 +165,9 @@ static const char *get_shader_name(int shader_type) case SHADER_PATCH_EVALUATION_FACE_DOTS: { return "subdiv patch evaluation face dots"; } + case SHADER_PATCH_EVALUATION_FACE_DOTS_WITH_NORMALS: { + return "subdiv patch evaluation face dots with normals"; + } case SHADER_COMP_CUSTOM_DATA_INTERP_1D: { return "subdiv custom data interp 1D"; } @@ -206,6 +211,13 @@ static GPUShader *get_patch_evaluation_shader(int shader_type) "#define OPENSUBDIV_GLSL_COMPUTE_USE_1ST_DERIVATIVES\n" "#define FDOTS_EVALUATION\n"; } + else if (shader_type == SHADER_PATCH_EVALUATION_FACE_DOTS_WITH_NORMALS) { + defines = + "#define OSD_PATCH_BASIS_GLSL\n" + "#define OPENSUBDIV_GLSL_COMPUTE_USE_1ST_DERIVATIVES\n" + "#define FDOTS_EVALUATION\n" + "#define FOTS_NORMALS\n"; + } else { defines = "#define OSD_PATCH_BASIS_GLSL\n" @@ -1625,7 +1637,9 @@ void draw_subdiv_build_fdots_buffers(const DRWSubdivCache *cache, get_patch_param_format()); evaluator->wrapPatchParamBuffer(evaluator, &patch_param_buffer_interface); - GPUShader *shader = get_patch_evaluation_shader(SHADER_PATCH_EVALUATION_FACE_DOTS); + GPUShader *shader = get_patch_evaluation_shader( + fdots_nor ? SHADER_PATCH_EVALUATION_FACE_DOTS_WITH_NORMALS : + SHADER_PATCH_EVALUATION_FACE_DOTS); GPU_shader_bind(shader); int binding_point = 0; @@ -1638,7 +1652,11 @@ void draw_subdiv_build_fdots_buffers(const DRWSubdivCache *cache, GPU_vertbuf_bind_as_ssbo(patch_index_buffer, binding_point++); GPU_vertbuf_bind_as_ssbo(patch_param_buffer, binding_point++); GPU_vertbuf_bind_as_ssbo(fdots_pos, binding_point++); - GPU_vertbuf_bind_as_ssbo(fdots_nor, binding_point++); + /* F-dots normals may not be requested, still reserve the binding point. */ + if (fdots_nor) { + GPU_vertbuf_bind_as_ssbo(fdots_nor, binding_point); + } + binding_point++; GPU_indexbuf_bind_as_ssbo(fdots_indices, binding_point++); GPU_vertbuf_bind_as_ssbo(cache->extra_coarse_face_data, binding_point++); BLI_assert(binding_point <= MAX_GPU_SUBDIV_SSBOS); diff --git a/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_fdots_pos.cc b/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_fdots_pos.cc index c2b4d389b7c..1671a1cd1e7 100644 --- a/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_fdots_pos.cc +++ b/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_fdots_pos.cc @@ -110,8 +110,11 @@ static void extract_fdots_init_subdiv(const DRWSubdivCache *subdiv_cache, GPUVertBuf *fdots_nor_vbo = cache->final.buff.vbo.fdots_nor; GPUIndexBuf *fdots_pos_ibo = cache->final.buff.ibo.fdots; - GPU_vertbuf_init_build_on_device( - fdots_nor_vbo, get_fdots_nor_format_subdiv(), subdiv_cache->num_coarse_poly); + /* The normals may not be requested. */ + if (fdots_nor_vbo) { + GPU_vertbuf_init_build_on_device( + fdots_nor_vbo, get_fdots_nor_format_subdiv(), subdiv_cache->num_coarse_poly); + } GPU_vertbuf_init_build_on_device( fdots_pos_vbo, get_fdots_pos_format(), subdiv_cache->num_coarse_poly); GPU_indexbuf_init_build_on_device(fdots_pos_ibo, subdiv_cache->num_coarse_poly); diff --git a/source/blender/draw/intern/shaders/common_subdiv_patch_evaluation_comp.glsl b/source/blender/draw/intern/shaders/common_subdiv_patch_evaluation_comp.glsl index e8d98428a8d..e842a73b8b3 100644 --- a/source/blender/draw/intern/shaders/common_subdiv_patch_evaluation_comp.glsl +++ b/source/blender/draw/intern/shaders/common_subdiv_patch_evaluation_comp.glsl @@ -70,10 +70,12 @@ layout(std430, binding = 8) writeonly buffer outputVertices FDotVert output_verts[]; }; +# ifdef FDOTS_NORMALS layout(std430, binding = 9) writeonly buffer outputNormals { FDotNor output_nors[]; }; +# endif layout(std430, binding = 10) writeonly buffer outputFdotsIndices { @@ -375,13 +377,15 @@ void main() fnor.flag = get_face_flag(coarse_quad_index); output_verts[coarse_quad_index] = vert; +# ifdef FDOTS_NORMALS output_nors[coarse_quad_index] = fnor; +# endif if (is_face_hidden(coarse_quad_index)) { - output_indices[coarse_quad_index] = 0xffffffff; + output_indices[coarse_quad_index] = 0xffffffff; } else { - output_indices[coarse_quad_index] = coarse_quad_index; + output_indices[coarse_quad_index] = coarse_quad_index; } } #else -- cgit v1.2.3 From bf53956914732bfd3e25806867ebe63081f7486d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dietrich?= Date: Thu, 26 May 2022 15:12:30 +0200 Subject: Fix T98385: Color attributes not working with GPU subdivision Contrary to coarse extraction, GPU extraction uses the same buffer for the coarse data, only the final GPU buffer needs to be offset. --- source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_vcol.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'source') diff --git a/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_vcol.cc b/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_vcol.cc index e5dd025787d..fa5bf35198b 100644 --- a/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_vcol.cc +++ b/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_vcol.cc @@ -271,8 +271,6 @@ static void extract_vcol_init_subdiv(const DRWSubdivCache *subdiv_cache, blender::Vector refs = get_vcol_refs(cd_vdata, cd_ldata, vcol_layers); - gpuMeshVcol *vcol = mesh_vcol; - /* Index of the vertex color layer in the compact buffer. Used vertex color layers are stored in * a single buffer. */ int pack_layer_index = 0; @@ -287,10 +285,10 @@ static void extract_vcol_init_subdiv(const DRWSubdivCache *subdiv_cache, if (layer_i == -1) { printf("%s: missing color layer %s\n", __func__, ref.layer->name); - vcol += coarse_mesh->totloop; continue; } + gpuMeshVcol *vcol = mesh_vcol; MLoopCol *mcol = nullptr; MPropCol *pcol = nullptr; -- cgit v1.2.3 From 5625a21fc7cf3738278f02038cb6d8a3c2344584 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Fri, 27 May 2022 10:45:31 +0200 Subject: Fix fuzzy ID deletion user count check. `BKE_id_delete` should only check for consistency of user count with regards to the tags and flags of the ID, not 'protect' nor even warn in case a 'fake user' ID is deleted (such higher-level checks are to be handled by higher-level code). Also replace the assert + debug print by a CLOG error, this avoids 'assert crash' while still failing tests, and always producing a useful message. Fixes T98374 and T98260. --- source/blender/blenkernel/intern/lib_id_delete.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/intern/lib_id_delete.c b/source/blender/blenkernel/intern/lib_id_delete.c index 904039d56c8..f14c11a949e 100644 --- a/source/blender/blenkernel/intern/lib_id_delete.c +++ b/source/blender/blenkernel/intern/lib_id_delete.c @@ -6,6 +6,8 @@ * Contains management of ID's for freeing & deletion. */ +#include "CLG_log.h" + #include "MEM_guardedalloc.h" /* all types are needed here, in order to do memory operations */ @@ -35,8 +37,7 @@ # include "BPY_extern.h" #endif -/* Not used currently. */ -// static CLG_LogRef LOG = {.identifier = "bke.lib_id_delete"}; +static CLG_LogRef LOG = {.identifier = "bke.lib_id_delete"}; void BKE_libblock_free_data(ID *id, const bool do_id_user) { @@ -334,11 +335,13 @@ static size_t id_delete(Main *bmain, const bool do_tagged_deletion) for (id = do_tagged_deletion ? tagged_deleted_ids.first : lb->first; id; id = id_next) { id_next = id->next; if (id->tag & tag) { - if (id->us != 0) { -#ifdef DEBUG_PRINT - printf("%s: deleting %s (%d)\n", __func__, id->name, id->us); -#endif - BLI_assert(id->us == 0); + if (((id->tag & LIB_TAG_EXTRAUSER_SET) == 0 && ID_REAL_USERS(id) != 0) || + ((id->tag & LIB_TAG_EXTRAUSER_SET) != 0 && ID_REAL_USERS(id) != 1)) { + CLOG_ERROR(&LOG, + "Deleting %s which still has %d users (including %d 'extra' shallow users)\n", + id->name, + ID_REAL_USERS(id), + (id->tag & LIB_TAG_EXTRAUSER_SET) != 0 ? 1 : 0); } BKE_id_free_ex(bmain, id, free_flag, !do_tagged_deletion); ++num_datablocks_deleted; -- cgit v1.2.3