From 4f3233dd536e8839c304dd34cd8aec89bfd8ca34 Mon Sep 17 00:00:00 2001 From: Pablo Dobarro Date: Fri, 17 Jul 2020 18:07:47 +0200 Subject: Fix T78242: Crash when using a Sculpt color tools that needs connectivity for the first time When there is no color layer available, BKE_sculpt_update_object_for_edit creates a new one and tags the mesh with ID_RECLAC_GEOMETRY, so this layer is inmediatly available when the tool starts. This also deletes the PBVH and when it is created again in BKE_sculpt_update_object_after_eval, the pmap is not initialized, making the tool crash. This moves the color layer creation to a separate function outside BKE_sculpt_update_object_for_edit, which now runs after the color layer is available, so it won't need to update again and the pmap will still be available when the tool is used. Reviewed By: sergey Maniphest Tasks: T78242 Differential Revision: https://developer.blender.org/D8135 --- source/blender/blenkernel/BKE_paint.h | 4 +++ source/blender/blenkernel/intern/paint.c | 30 +++++++++++++--------- source/blender/editors/sculpt_paint/sculpt.c | 9 ++++++- .../editors/sculpt_paint/sculpt_filter_color.c | 6 ++++- 4 files changed, 35 insertions(+), 14 deletions(-) diff --git a/source/blender/blenkernel/BKE_paint.h b/source/blender/blenkernel/BKE_paint.h index e3a6fb4ba2e..1c02aece06f 100644 --- a/source/blender/blenkernel/BKE_paint.h +++ b/source/blender/blenkernel/BKE_paint.h @@ -451,6 +451,10 @@ void BKE_sculptsession_free_vwpaint_data(struct SculptSession *ss); void BKE_sculptsession_bm_to_me(struct Object *ob, bool reorder); void BKE_sculptsession_bm_to_me_for_render(struct Object *object); +/* Create new color layer on object if it doesn't have one and if experimental feature set has + * sculpt vertex color enabled. Returns truth if new layer has been added, false otherwise. */ +void BKE_sculpt_color_layer_create_if_needed(struct Object *object); + void BKE_sculpt_update_object_for_edit(struct Depsgraph *depsgraph, struct Object *ob_orig, bool need_pmap, diff --git a/source/blender/blenkernel/intern/paint.c b/source/blender/blenkernel/intern/paint.c index 50feb8e99c8..5feaacee254 100644 --- a/source/blender/blenkernel/intern/paint.c +++ b/source/blender/blenkernel/intern/paint.c @@ -1484,7 +1484,7 @@ static void sculpt_update_object(Depsgraph *depsgraph, Mesh *me_eval, bool need_pmap, bool need_mask, - bool need_colors) + bool UNUSED(need_colors)) { Scene *scene = DEG_get_input_scene(depsgraph); Sculpt *sd = scene->toolsettings->sculpt; @@ -1514,16 +1514,6 @@ static void sculpt_update_object(Depsgraph *depsgraph, } } - /* Add a color layer if a color tool is used. */ - Mesh *orig_me = BKE_object_get_original_mesh(ob); - if (need_colors && U.experimental.use_sculpt_vertex_colors) { - if (!CustomData_has_layer(&orig_me->vdata, CD_PROP_COLOR)) { - CustomData_add_layer(&orig_me->vdata, CD_PROP_COLOR, CD_DEFAULT, NULL, orig_me->totvert); - BKE_mesh_update_customdata_pointers(orig_me, true); - DEG_id_tag_update(&orig_me->id, ID_RECALC_GEOMETRY); - } - } - /* tessfaces aren't used and will become invalid */ BKE_mesh_tessface_clear(me); @@ -1684,10 +1674,26 @@ void BKE_sculpt_update_object_after_eval(Depsgraph *depsgraph, Object *ob_eval) Mesh *me_eval = BKE_object_get_evaluated_mesh(ob_eval); BLI_assert(me_eval != NULL); - sculpt_update_object(depsgraph, ob_orig, me_eval, false, false, false); } +void BKE_sculpt_color_layer_create_if_needed(struct Object *object) +{ + Mesh *orig_me = BKE_object_get_original_mesh(object); + if (!U.experimental.use_sculpt_vertex_colors) { + return; + } + + if (CustomData_has_layer(&orig_me->vdata, CD_PROP_COLOR)) { + return; + } + + CustomData_add_layer(&orig_me->vdata, CD_PROP_COLOR, CD_DEFAULT, NULL, orig_me->totvert); + BKE_mesh_update_customdata_pointers(orig_me, true); + DEG_id_tag_update(&orig_me->id, ID_RECALC_GEOMETRY); + return; +} + void BKE_sculpt_update_object_for_edit( Depsgraph *depsgraph, Object *ob_orig, bool need_pmap, bool need_mask, bool need_colors) { diff --git a/source/blender/editors/sculpt_paint/sculpt.c b/source/blender/editors/sculpt_paint/sculpt.c index da5d6588dc8..aa8161836f9 100644 --- a/source/blender/editors/sculpt_paint/sculpt.c +++ b/source/blender/editors/sculpt_paint/sculpt.c @@ -7140,7 +7140,6 @@ static void sculpt_brush_init_tex(const Scene *scene, Sculpt *sd, SculptSession static void sculpt_brush_stroke_init(bContext *C, wmOperator *op) { - Depsgraph *depsgraph = CTX_data_ensure_evaluated_depsgraph(C); Scene *scene = CTX_data_scene(C); Object *ob = CTX_data_active_object(C); Sculpt *sd = CTX_data_tool_settings(C)->sculpt; @@ -7163,6 +7162,14 @@ static void sculpt_brush_stroke_init(bContext *C, wmOperator *op) is_smooth = sculpt_needs_connectivity_info(sd, brush, ss, mode); needs_colors = ELEM(brush->sculpt_tool, SCULPT_TOOL_PAINT, SCULPT_TOOL_SMEAR); + + if (needs_colors) { + BKE_sculpt_color_layer_create_if_needed(ob); + } + + /* CTX_data_ensure_evaluated_depsgraph should be used at the end to include the updates of + * earlier steps modifying the data. */ + Depsgraph *depsgraph = CTX_data_ensure_evaluated_depsgraph(C); BKE_sculpt_update_object_for_edit(depsgraph, ob, is_smooth, need_mask, needs_colors); } diff --git a/source/blender/editors/sculpt_paint/sculpt_filter_color.c b/source/blender/editors/sculpt_paint/sculpt_filter_color.c index 556b73b0ea5..59d82825740 100644 --- a/source/blender/editors/sculpt_paint/sculpt_filter_color.c +++ b/source/blender/editors/sculpt_paint/sculpt_filter_color.c @@ -265,7 +265,6 @@ static int sculpt_color_filter_modal(bContext *C, wmOperator *op, const wmEvent static int sculpt_color_filter_invoke(bContext *C, wmOperator *op, const wmEvent *UNUSED(event)) { Object *ob = CTX_data_active_object(C); - Depsgraph *depsgraph = CTX_data_depsgraph_pointer(C); Sculpt *sd = CTX_data_tool_settings(C)->sculpt; SculptSession *ss = ob->sculpt; int mode = RNA_enum_get(op->ptr, "type"); @@ -285,6 +284,11 @@ static int sculpt_color_filter_invoke(bContext *C, wmOperator *op, const wmEvent SCULPT_undo_push_begin("color filter"); + BKE_sculpt_color_layer_create_if_needed(ob); + + /* CTX_data_ensure_evaluated_depsgraph should be used at the end to include the updates of + * earlier steps modifying the data. */ + Depsgraph *depsgraph = CTX_data_ensure_evaluated_depsgraph(C); bool needs_pmap = mode == COLOR_FILTER_SMOOTH; BKE_sculpt_update_object_for_edit(depsgraph, ob, needs_pmap, false, true); -- cgit v1.2.3 From 7eebebebc7bb08621d97a8318a37366fbd7b02e8 Mon Sep 17 00:00:00 2001 From: Pablo Dobarro Date: Wed, 22 Jul 2020 16:44:47 +0200 Subject: Fix T79164: Sculpting with smooth shading doesn't update normals Just a missing update flag Reviewed By: sergey Maniphest Tasks: T79164 Differential Revision: https://developer.blender.org/D8364 --- source/blender/editors/sculpt_paint/sculpt_smooth.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source/blender/editors/sculpt_paint/sculpt_smooth.c b/source/blender/editors/sculpt_paint/sculpt_smooth.c index 1a699c91e9b..7fbbcd1c896 100644 --- a/source/blender/editors/sculpt_paint/sculpt_smooth.c +++ b/source/blender/editors/sculpt_paint/sculpt_smooth.c @@ -242,6 +242,9 @@ static void do_smooth_brush_task_cb_ex(void *__restrict userdata, madd_v3_v3v3fl(val, vd.co, val, fade); SCULPT_clip(sd, ss, vd.co, val); } + if (vd.mvert) { + vd.mvert->flag |= ME_VERT_PBVH_UPDATE; + } } } BKE_pbvh_vertex_iter_end; -- cgit v1.2.3 From 00065269521186ba90d088ee2443d89fbcebfce0 Mon Sep 17 00:00:00 2001 From: Pablo Dobarro Date: Tue, 21 Jul 2020 03:25:03 +0200 Subject: Fix T79074: Mesh Topology info not being updated after changes All these data arrays are created for a specific topology, so they should be freed and updated when the PBVH rebuilds. Previously, this was only happening when freeing the SculptSession, but it also needs to happen in BKE_sculpt_update_object_before_eval to avoid reusing out of date data. Reviewed By: sergey Maniphest Tasks: T79074 Differential Revision: https://developer.blender.org/D8357 --- source/blender/blenkernel/intern/paint.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/source/blender/blenkernel/intern/paint.c b/source/blender/blenkernel/intern/paint.c index 5feaacee254..d6fbba74a89 100644 --- a/source/blender/blenkernel/intern/paint.c +++ b/source/blender/blenkernel/intern/paint.c @@ -1314,6 +1314,13 @@ static void sculptsession_free_pbvh(Object *object) MEM_SAFE_FREE(ss->preview_vert_index_list); ss->preview_vert_index_count = 0; + + MEM_SAFE_FREE(ss->preview_vert_index_list); + + MEM_SAFE_FREE(ss->vertex_info.connected_component); + MEM_SAFE_FREE(ss->vertex_info.boundary); + + MEM_SAFE_FREE(ss->fake_neighbors.fake_neighbor_index); } void BKE_sculptsession_bm_to_me_for_render(Object *object) @@ -1366,13 +1373,6 @@ void BKE_sculptsession_free(Object *ob) MEM_SAFE_FREE(ss->deform_cos); MEM_SAFE_FREE(ss->deform_imats); - MEM_SAFE_FREE(ss->preview_vert_index_list); - - MEM_SAFE_FREE(ss->vertex_info.connected_component); - MEM_SAFE_FREE(ss->vertex_info.boundary); - - MEM_SAFE_FREE(ss->fake_neighbors.fake_neighbor_index); - if (ss->pose_ik_chain_preview) { for (int i = 0; i < ss->pose_ik_chain_preview->tot_segments; i++) { MEM_SAFE_FREE(ss->pose_ik_chain_preview->segments[i].weights); -- cgit v1.2.3 From 221604cdd663e88c0d2c2400144329f0a70f3d87 Mon Sep 17 00:00:00 2001 From: Pablo Dobarro Date: Sun, 19 Jul 2020 23:42:44 +0200 Subject: Fix Sculpt Relax operation when deforming mesh boundaries Previously, mesh boundaries were relaxed as any other vertex, which was causing artifacts and unwanted deformation. In order to prevent this, the mesh filter was using the automasking system to lock the boundary vertices, which was hacked into the tool. For the brush, the only solution was to enable boundary automasking to lock those vertices in plance. Now the relax vertex function slides the boundary vertices along the mesh boundary edges, relaxing all the topology correctly while preserving the shape of the mesh. The automasking hack in the relax mesh filter was also removed as now vertices slide correctly along the boundary. Reviewed By: sergey Differential Revision: https://developer.blender.org/D8350 --- source/blender/editors/sculpt_paint/sculpt.c | 46 ++++++++++++++++++---- .../blender/editors/sculpt_paint/sculpt_face_set.c | 1 + .../editors/sculpt_paint/sculpt_filter_mesh.c | 17 +++----- 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/source/blender/editors/sculpt_paint/sculpt.c b/source/blender/editors/sculpt_paint/sculpt.c index aa8161836f9..590d04bed3d 100644 --- a/source/blender/editors/sculpt_paint/sculpt.c +++ b/source/blender/editors/sculpt_paint/sculpt.c @@ -3147,21 +3147,49 @@ void SCULPT_relax_vertex(SculptSession *ss, { float smooth_pos[3]; float final_disp[3]; - int count = 0; + float boundary_normal[3]; + int avg_count = 0; + int neighbor_count = 0; zero_v3(smooth_pos); + zero_v3(boundary_normal); + const bool is_boundary = SCULPT_vertex_is_boundary(ss, vd->index); SculptVertexNeighborIter ni; SCULPT_VERTEX_NEIGHBORS_ITER_BEGIN (ss, vd->index, ni) { + neighbor_count++; if (!filter_boundary_face_sets || (filter_boundary_face_sets && !SCULPT_vertex_has_unique_face_set(ss, ni.index))) { - add_v3_v3(smooth_pos, SCULPT_vertex_co_get(ss, ni.index)); - count++; + + /* When the vertex to relax is boundary, use only connected boundary vertices for the average + * position. */ + if (is_boundary) { + if (SCULPT_vertex_is_boundary(ss, ni.index)) { + add_v3_v3(smooth_pos, SCULPT_vertex_co_get(ss, ni.index)); + avg_count++; + + /* Calculate a normal for the constraint plane using the edges of the boundary. */ + float to_neighbor[3]; + sub_v3_v3v3(to_neighbor, SCULPT_vertex_co_get(ss, ni.index), vd->co); + normalize_v3(to_neighbor); + add_v3_v3(boundary_normal, to_neighbor); + } + } + else { + add_v3_v3(smooth_pos, SCULPT_vertex_co_get(ss, ni.index)); + avg_count++; + } } } SCULPT_VERTEX_NEIGHBORS_ITER_END(ni); - if (count > 0) { - mul_v3_fl(smooth_pos, 1.0f / (float)count); + /* Don't modify corner vertices. */ + if (neighbor_count <= 2) { + copy_v3_v3(r_final_pos, vd->co); + return; + } + + if (avg_count > 0) { + mul_v3_fl(smooth_pos, 1.0f / (float)avg_count); } else { copy_v3_v3(r_final_pos, vd->co); @@ -3171,11 +3199,12 @@ void SCULPT_relax_vertex(SculptSession *ss, float plane[4]; float smooth_closest_plane[3]; float vno[3]; - if (vd->no) { - normal_short_to_float_v3(vno, vd->no); + + if (is_boundary && avg_count == 2) { + normalize_v3_v3(vno, boundary_normal); } else { - copy_v3_v3(vno, vd->fno); + SCULPT_vertex_normal_get(ss, vd->index, vno); } if (is_zero_v3(vno)) { @@ -3256,6 +3285,7 @@ static void do_slide_relax_brush(Sculpt *sd, Object *ob, PBVHNode **nodes, int t TaskParallelSettings settings; BKE_pbvh_parallel_range_settings(&settings, true, totnode); if (ss->cache->alt_smooth) { + SCULPT_boundary_info_ensure(ob); for (int i = 0; i < 4; i++) { BLI_task_parallel_range(0, totnode, &data, do_topology_relax_task_cb_ex, &settings); } diff --git a/source/blender/editors/sculpt_paint/sculpt_face_set.c b/source/blender/editors/sculpt_paint/sculpt_face_set.c index 031b4f8731d..1940b007cb0 100644 --- a/source/blender/editors/sculpt_paint/sculpt_face_set.c +++ b/source/blender/editors/sculpt_paint/sculpt_face_set.c @@ -205,6 +205,7 @@ void SCULPT_do_draw_face_sets_brush(Sculpt *sd, Object *ob, PBVHNode **nodes, in TaskParallelSettings settings; BKE_pbvh_parallel_range_settings(&settings, true, totnode); if (ss->cache->alt_smooth) { + SCULPT_boundary_info_ensure(ob); for (int i = 0; i < 4; i++) { BLI_task_parallel_range(0, totnode, &data, do_relax_face_sets_brush_task_cb_ex, &settings); } diff --git a/source/blender/editors/sculpt_paint/sculpt_filter_mesh.c b/source/blender/editors/sculpt_paint/sculpt_filter_mesh.c index 9c9726ff3db..e9a98a17f8a 100644 --- a/source/blender/editors/sculpt_paint/sculpt_filter_mesh.c +++ b/source/blender/editors/sculpt_paint/sculpt_filter_mesh.c @@ -310,8 +310,7 @@ static void mesh_filter_task_cb(void *__restrict userdata, break; } case MESH_FILTER_RELAX: { - SCULPT_relax_vertex( - ss, &vd, clamp_f(fade * ss->filter_cache->automask[vd.index], 0.0f, 1.0f), false, val); + SCULPT_relax_vertex(ss, &vd, clamp_f(fade, 0.0f, 1.0f), false, val); sub_v3_v3v3(disp, val, vd.co); break; } @@ -543,6 +542,10 @@ static int sculpt_mesh_filter_invoke(bContext *C, wmOperator *op, const wmEvent SCULPT_undo_push_begin("Mesh filter"); + if (ELEM(filter_type, MESH_FILTER_RELAX, MESH_FILTER_RELAX_FACE_SETS)) { + SCULPT_boundary_info_ensure(ob); + } + SCULPT_filter_cache_init(ob, sd, SCULPT_UNDO_COORDS); if (use_face_sets) { @@ -572,16 +575,6 @@ static int sculpt_mesh_filter_invoke(bContext *C, wmOperator *op, const wmEvent ss->filter_cache->enabled_axis[1] = deform_axis & MESH_FILTER_DEFORM_Y; ss->filter_cache->enabled_axis[2] = deform_axis & MESH_FILTER_DEFORM_Z; - if (RNA_enum_get(op->ptr, "type") == MESH_FILTER_RELAX) { - ss->filter_cache->automask = MEM_mallocN(totvert * sizeof(float), - "Relax filter edge automask"); - for (int i = 0; i < totvert; i++) { - ss->filter_cache->automask[i] = 1.0f; - } - SCULPT_boundary_automasking_init( - ob, AUTOMASK_INIT_BOUNDARY_EDGES, 1, ss->filter_cache->automask); - } - WM_event_add_modal_handler(C, op); return OPERATOR_RUNNING_MODAL; } -- cgit v1.2.3