From 97f2b01ea93b599a537d634e222aa6c7c7d84aaa Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Tue, 26 Sep 2023 14:21:07 +0200 Subject: [PATCH] Fix #112351: Sculpt implicit sharing thread safety crash Currently the iteration over a PBVH node's vertices retrieves mutable access to the mask custom data layer. This isn't threadsafe, but it is done in the multithreaded loops over all nodes. In general, we need to be more careful and conservative about storage of non-const pointers to mesh data. Ideally we would only have one mutable reference to a resource at a time. And we should avoid doing work like looking up custom data layers more than we need to. To that end, make the pointer to the custom data layer used everywhere const, and retrieve mutable access before parallel node iteration with a specific function, and write to the mask data with that in mind. This pushes us in the direction of sharing less code per PBVH type. In my opinion that's a good thing, because we can actually optimize for each type. For example, `write_mask_data` gives a picture of how each of these hot loops could become much simpler. Pull Request: https://projects.blender.org/blender/blender/pulls/112690 --- source/blender/blenkernel/BKE_paint.hh | 2 +- source/blender/blenkernel/BKE_pbvh_api.hh | 6 +- source/blender/blenkernel/intern/pbvh.cc | 8 +- .../editors/sculpt_paint/paint_mask.cc | 34 ++++---- source/blender/editors/sculpt_paint/sculpt.cc | 67 +++++++++++++--- .../sculpt_paint/sculpt_brush_types.cc | 18 +++-- .../editors/sculpt_paint/sculpt_expand.cc | 78 +++++++++++++------ .../sculpt_paint/sculpt_filter_mask.cc | 41 ++++++---- .../editors/sculpt_paint/sculpt_intern.hh | 24 ++++++ .../editors/sculpt_paint/sculpt_mask_init.cc | 19 +++-- .../editors/sculpt_paint/sculpt_ops.cc | 32 +++++--- .../editors/sculpt_paint/sculpt_smooth.cc | 14 +++- .../editors/sculpt_paint/sculpt_undo.cc | 6 +- 13 files changed, 254 insertions(+), 95 deletions(-) diff --git a/source/blender/blenkernel/BKE_paint.hh b/source/blender/blenkernel/BKE_paint.hh index 228165afee2..4790e3e2d07 100644 --- a/source/blender/blenkernel/BKE_paint.hh +++ b/source/blender/blenkernel/BKE_paint.hh @@ -583,7 +583,7 @@ struct SculptSession { eAttrDomain vcol_domain; eCustomDataType vcol_type; - float *vmask; + const float *vmask; /* Mesh connectivity maps. */ /* Vertices to adjacent polys. */ diff --git a/source/blender/blenkernel/BKE_pbvh_api.hh b/source/blender/blenkernel/BKE_pbvh_api.hh index fd463bffd01..318f7eda225 100644 --- a/source/blender/blenkernel/BKE_pbvh_api.hh +++ b/source/blender/blenkernel/BKE_pbvh_api.hh @@ -105,6 +105,8 @@ BLI_INLINE BMesh *BKE_pbvh_get_bmesh(PBVH *pbvh) return ((struct PBVHPublic *)pbvh)->bm; } +Mesh *BKE_pbvh_get_mesh(PBVH *pbvh); + BLI_INLINE PBVHVertRef BKE_pbvh_make_vref(intptr_t i) { PBVHVertRef ret = {i}; @@ -501,7 +503,7 @@ struct PBVHVertexIter { const bool *hide_vert; int totvert; const int *vert_indices; - float *vmask; + const float *vmask; bool is_mesh; /* bmesh */ @@ -516,7 +518,7 @@ struct PBVHVertexIter { float *co; const float *no; const float *fno; - float *mask; + const float *mask; bool visible; }; diff --git a/source/blender/blenkernel/intern/pbvh.cc b/source/blender/blenkernel/intern/pbvh.cc index 6dfcb72563f..a9d89d8be94 100644 --- a/source/blender/blenkernel/intern/pbvh.cc +++ b/source/blender/blenkernel/intern/pbvh.cc @@ -3118,8 +3118,7 @@ void pbvh_vertex_iter_init(PBVH *pbvh, PBVHNode *node, PBVHVertexIter *vi, int m vi->vert_normals = pbvh->vert_normals; vi->hide_vert = pbvh->hide_vert; - vi->vmask = static_cast( - CustomData_get_layer_for_write(pbvh->vert_data, CD_PAINT_MASK, pbvh->mesh->totvert)); + vi->vmask = static_cast(CustomData_get_layer(pbvh->vert_data, CD_PAINT_MASK)); } } @@ -3176,6 +3175,11 @@ void BKE_pbvh_parallel_range_settings(TaskParallelSettings *settings, settings->use_threading = use_threading && totnode > 1; } +Mesh *BKE_pbvh_get_mesh(PBVH *pbvh) +{ + return pbvh->mesh; +} + float (*BKE_pbvh_get_vert_positions(const PBVH *pbvh))[3] { BLI_assert(pbvh->header.type == PBVH_FACES); diff --git a/source/blender/editors/sculpt_paint/paint_mask.cc b/source/blender/editors/sculpt_paint/paint_mask.cc index f3200ce9c28..8b5107a778e 100644 --- a/source/blender/editors/sculpt_paint/paint_mask.cc +++ b/source/blender/editors/sculpt_paint/paint_mask.cc @@ -73,19 +73,20 @@ static const EnumPropertyItem mode_items[] = { {PAINT_MASK_INVERT, "INVERT", 0, "Invert", "Invert the mask"}, {0}}; -static void mask_flood_fill_set_elem(float *elem, PaintMaskFloodMode mode, float value) +static float mask_flood_fill_get_new_value_for_elem(const float elem, + PaintMaskFloodMode mode, + float value) { switch (mode) { case PAINT_MASK_FLOOD_VALUE: - (*elem) = value; - break; + return value; case PAINT_MASK_FLOOD_VALUE_INVERSE: - (*elem) = 1.0f - value; - break; + return 1.0f - value; case PAINT_MASK_INVERT: - (*elem) = 1.0f - (*elem); - break; + return 1.0f - elem; } + BLI_assert_unreachable(); + return 0.0f; } static int mask_flood_fill_exec(bContext *C, wmOperator *op) @@ -106,6 +107,7 @@ static int mask_flood_fill_exec(bContext *C, wmOperator *op) const bool multires = (BKE_pbvh_type(pbvh) == PBVH_GRIDS); Vector nodes = blender::bke::pbvh::search_gather(pbvh, {}); + const SculptMaskWriteInfo mask_write = SCULPT_mask_get_for_write(ob->sculpt); SCULPT_undo_push_begin(ob, op); @@ -118,8 +120,9 @@ static int mask_flood_fill_exec(bContext *C, wmOperator *op) PBVHVertexIter vi; BKE_pbvh_vertex_iter_begin (pbvh, node, vi, PBVH_ITER_UNIQUE) { float prevmask = *vi.mask; - mask_flood_fill_set_elem(vi.mask, mode, value); - if (prevmask != *vi.mask) { + const float new_mask = mask_flood_fill_get_new_value_for_elem(prevmask, mode, value); + if (prevmask != new_mask) { + SCULPT_mask_vert_set(BKE_pbvh_type(ob->sculpt->pbvh), mask_write, new_mask, vi); redraw = true; } } @@ -776,7 +779,9 @@ static void sculpt_gesture_mask_begin(bContext *C, SculptGestureContext *sgconte BKE_sculpt_update_object_for_edit(depsgraph, sgcontext->vc.obact, false, true, false); } -static void mask_gesture_apply_task(SculptGestureContext *sgcontext, PBVHNode *node) +static void mask_gesture_apply_task(SculptGestureContext *sgcontext, + const SculptMaskWriteInfo mask_write, + PBVHNode *node) { SculptGestureMaskOperation *mask_operation = (SculptGestureMaskOperation *)sgcontext->operation; Object *ob = sgcontext->vc.obact; @@ -799,8 +804,10 @@ static void mask_gesture_apply_task(SculptGestureContext *sgcontext, PBVHNode *n BKE_pbvh_node_mark_normals_update(node); } } - mask_flood_fill_set_elem(vd.mask, mask_operation->mode, mask_operation->value); - if (prevmask != *vd.mask) { + const float new_mask = mask_flood_fill_get_new_value_for_elem( + prevmask, mask_operation->mode, mask_operation->value); + if (prevmask != new_mask) { + SCULPT_mask_vert_set(BKE_pbvh_type(ob->sculpt->pbvh), mask_write, new_mask, vd); redraw = true; } } @@ -816,9 +823,10 @@ static void sculpt_gesture_mask_apply_for_symmetry_pass(bContext * /*C*/, SculptGestureContext *sgcontext) { using namespace blender; + const SculptMaskWriteInfo mask_write = SCULPT_mask_get_for_write(sgcontext->ss); threading::parallel_for(sgcontext->nodes.index_range(), 1, [&](const IndexRange range) { for (const int i : range) { - mask_gesture_apply_task(sgcontext, sgcontext->nodes[i]); + mask_gesture_apply_task(sgcontext, mask_write, sgcontext->nodes[i]); } }); } diff --git a/source/blender/editors/sculpt_paint/sculpt.cc b/source/blender/editors/sculpt_paint/sculpt.cc index e9801af977a..09d21dac15e 100644 --- a/source/blender/editors/sculpt_paint/sculpt.cc +++ b/source/blender/editors/sculpt_paint/sculpt.cc @@ -111,6 +111,25 @@ static float sculpt_calc_radius(ViewContext *vc, * different index for each grid. * \{ */ +SculptMaskWriteInfo SCULPT_mask_get_for_write(SculptSession *ss) +{ + SculptMaskWriteInfo info; + switch (BKE_pbvh_type(ss->pbvh)) { + case PBVH_FACES: { + Mesh *mesh = BKE_pbvh_get_mesh(ss->pbvh); + info.layer = static_cast( + CustomData_get_layer_for_write(&mesh->vert_data, CD_PAINT_MASK, mesh->totvert)); + break; + } + case PBVH_BMESH: + info.bm_offset = CustomData_get_offset(&BKE_pbvh_get_bmesh(ss->pbvh)->vdata, CD_PAINT_MASK); + break; + case PBVH_GRIDS: + break; + } + return info; +} + void SCULPT_vertex_random_access_ensure(SculptSession *ss) { if (BKE_pbvh_type(ss->pbvh) == PBVH_BMESH) { @@ -1463,7 +1482,10 @@ bool SCULPT_stroke_is_dynamic_topology(const SculptSession *ss, const Brush *bru /** \name Sculpt Paint Mesh * \{ */ -static void paint_mesh_restore_node(Object *ob, const SculptUndoType type, PBVHNode *node) +static void paint_mesh_restore_node(Object *ob, + const SculptUndoType type, + const SculptMaskWriteInfo mask_write, + PBVHNode *node) { SculptSession *ss = ob->sculpt; @@ -1481,14 +1503,34 @@ static void paint_mesh_restore_node(Object *ob, const SculptUndoType type, PBVHN switch (type) { case SCULPT_UNDO_MASK: { - SculptOrigVertData orig_vert_data; - SCULPT_orig_vert_data_unode_init(&orig_vert_data, ob, unode); - PBVHVertexIter vd; - BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { - SCULPT_orig_vert_data_update(&orig_vert_data, &vd); - *vd.mask = orig_vert_data.mask; + switch (BKE_pbvh_type(ss->pbvh)) { + case PBVH_FACES: { + PBVHVertexIter vd; + BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { + mask_write.layer[vd.index] = unode->mask[vd.i]; + } + BKE_pbvh_vertex_iter_end; + break; + } + case PBVH_BMESH: { + PBVHVertexIter vd; + BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { + const float orig_mask = BM_log_original_mask(ss->bm_log, vd.bm_vert); + BM_ELEM_CD_SET_FLOAT(vd.bm_vert, mask_write.bm_offset, orig_mask); + } + BKE_pbvh_vertex_iter_end; + break; + } + case PBVH_GRIDS: { + PBVHVertexIter vd; + BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { + *CCG_elem_mask(&vd.key, vd.grid) = unode->mask[vd.i]; + break; + } + BKE_pbvh_vertex_iter_end; + break; + } } - BKE_pbvh_vertex_iter_end; BKE_pbvh_node_mark_update_mask(node); break; } @@ -1563,18 +1605,23 @@ static void paint_mesh_restore_co(Sculpt *sd, Object *ob) break; } + SculptMaskWriteInfo mask_write; + if (type == SCULPT_UNDO_MASK) { + mask_write = SCULPT_mask_get_for_write(ss); + } + if (ss->bm) { /* Disable multi-threading when dynamic-topology is enabled. Otherwise, * new entries might be inserted by #SCULPT_undo_push_node() into the #GHash * used internally by #BM_log_original_vert_co() by a different thread. See #33787. */ for (const int i : nodes.index_range()) { - paint_mesh_restore_node(ob, type, nodes[i]); + paint_mesh_restore_node(ob, type, mask_write, nodes[i]); } } else { threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { for (const int i : range) { - paint_mesh_restore_node(ob, type, nodes[i]); + paint_mesh_restore_node(ob, type, mask_write, nodes[i]); } }); } diff --git a/source/blender/editors/sculpt_paint/sculpt_brush_types.cc b/source/blender/editors/sculpt_paint/sculpt_brush_types.cc index 0034d822739..53b4478917a 100644 --- a/source/blender/editors/sculpt_paint/sculpt_brush_types.cc +++ b/source/blender/editors/sculpt_paint/sculpt_brush_types.cc @@ -21,7 +21,7 @@ #include "DNA_brush_types.h" #include "DNA_customdata_types.h" -#include "DNA_meshdata_types.h" +#include "DNA_mesh_types.h" #include "DNA_object_types.h" #include "DNA_scene_types.h" @@ -2685,7 +2685,10 @@ void SCULPT_bmesh_topology_rake(Sculpt *sd, Object *ob, Span nodes, /** \name Sculpt Mask Brush * \{ */ -static void do_mask_brush_draw_task(Object *ob, const Brush *brush, PBVHNode *node) +static void do_mask_brush_draw_task(Object *ob, + const Brush *brush, + const SculptMaskWriteInfo mask_write, + PBVHNode *node) { SculptSession *ss = ob->sculpt; const float bstrength = ss->cache->bstrength; @@ -2717,13 +2720,15 @@ static void do_mask_brush_draw_task(Object *ob, const Brush *brush, PBVHNode *no thread_id, &automask_data); + float mask = *vd.mask; if (bstrength > 0.0f) { - (*vd.mask) += fade * bstrength * (1.0f - *vd.mask); + mask += fade * bstrength * (1.0f - *vd.mask); } else { - (*vd.mask) += fade * bstrength * (*vd.mask); + mask += fade * bstrength * mask; } - *vd.mask = clamp_f(*vd.mask, 0.0f, 1.0f); + mask = clamp_f(mask, 0.0f, 1.0f); + SCULPT_mask_vert_set(BKE_pbvh_type(ss->pbvh), mask_write, mask, vd); } BKE_pbvh_vertex_iter_end; } @@ -2732,9 +2737,10 @@ void SCULPT_do_mask_brush_draw(Sculpt *sd, Object *ob, Span nodes) { using namespace blender; const Brush *brush = BKE_paint_brush(&sd->paint); + const SculptMaskWriteInfo mask_write = SCULPT_mask_get_for_write(ob->sculpt); threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { for (const int i : range) { - do_mask_brush_draw_task(ob, brush, nodes[i]); + do_mask_brush_draw_task(ob, brush, mask_write, nodes[i]); } }); } diff --git a/source/blender/editors/sculpt_paint/sculpt_expand.cc b/source/blender/editors/sculpt_paint/sculpt_expand.cc index 5cf7abc3130..2cbda113946 100644 --- a/source/blender/editors/sculpt_paint/sculpt_expand.cc +++ b/source/blender/editors/sculpt_paint/sculpt_expand.cc @@ -1194,16 +1194,50 @@ static void sculpt_expand_restore_color_data(SculptSession *ss, ExpandCache *exp } } -static void sculpt_expand_restore_mask_data(SculptSession *ss, ExpandCache *expand_cache) +static void write_mask_data(SculptSession *ss, const Span mask) { Vector nodes = blender::bke::pbvh::search_gather(ss->pbvh, {}); - for (PBVHNode *node : nodes) { - PBVHVertexIter vd; - BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { - *vd.mask = expand_cache->original_mask[vd.index]; + + switch (BKE_pbvh_type(ss->pbvh)) { + case PBVH_FACES: { + Mesh *mesh = BKE_pbvh_get_mesh(ss->pbvh); + float *layer = static_cast( + CustomData_get_layer_for_write(&mesh->vert_data, CD_PAINT_MASK, mesh->totvert)); + for (PBVHNode *node : nodes) { + PBVHVertexIter vd; + BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { + layer[vd.index] = mask[vd.index]; + } + BKE_pbvh_vertex_iter_end; + BKE_pbvh_node_mark_redraw(node); + } + break; + } + case PBVH_BMESH: { + const int offset = CustomData_get_offset(&BKE_pbvh_get_bmesh(ss->pbvh)->vdata, + CD_PAINT_MASK); + for (PBVHNode *node : nodes) { + PBVHVertexIter vd; + BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { + BM_ELEM_CD_SET_FLOAT(vd.bm_vert, offset, mask[vd.index]); + } + BKE_pbvh_vertex_iter_end; + BKE_pbvh_node_mark_redraw(node); + } + break; + } + case PBVH_GRIDS: { + for (PBVHNode *node : nodes) { + PBVHVertexIter vd; + BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { + *CCG_elem_mask(&vd.key, vd.grid) = mask[vd.index]; + break; + } + BKE_pbvh_vertex_iter_end; + BKE_pbvh_node_mark_redraw(node); + } + break; } - BKE_pbvh_vertex_iter_end; - BKE_pbvh_node_mark_redraw(node); } } @@ -1217,7 +1251,7 @@ static void sculpt_expand_restore_original_state(bContext *C, SculptSession *ss = ob->sculpt; switch (expand_cache->target) { case SCULPT_EXPAND_TARGET_MASK: - sculpt_expand_restore_mask_data(ss, expand_cache); + write_mask_data(ss, {expand_cache->original_mask, SCULPT_vertex_count_get(ss)}); SCULPT_flush_update_step(C, SCULPT_UPDATE_MASK); SCULPT_flush_update_done(C, ob, SCULPT_UPDATE_MASK); SCULPT_tag_update_overlays(C); @@ -1255,7 +1289,9 @@ static void sculpt_expand_cancel(bContext *C, wmOperator * /*op*/) /** * Callback to update mask data per PBVH node. */ -static void sculpt_expand_mask_update_task(SculptSession *ss, PBVHNode *node) +static void sculpt_expand_mask_update_task(SculptSession *ss, + const SculptMaskWriteInfo mask_write, + PBVHNode *node) { ExpandCache *expand_cache = ss->expand_cache; @@ -1294,7 +1330,8 @@ static void sculpt_expand_mask_update_task(SculptSession *ss, PBVHNode *node) continue; } - *vd.mask = clamp_f(new_mask, 0.0f, 1.0f); + new_mask = clamp_f(new_mask, 0.0f, 1.0f); + SCULPT_mask_vert_set(BKE_pbvh_type(ss->pbvh), mask_write, new_mask, vd); any_changed = true; } BKE_pbvh_vertex_iter_end; @@ -1485,13 +1522,15 @@ static void sculpt_expand_update_for_vertex(bContext *C, Object *ob, const PBVHV } switch (expand_cache->target) { - case SCULPT_EXPAND_TARGET_MASK: + case SCULPT_EXPAND_TARGET_MASK: { + const SculptMaskWriteInfo mask_write = SCULPT_mask_get_for_write(ss); threading::parallel_for(expand_cache->nodes.index_range(), 1, [&](const IndexRange range) { for (const int i : range) { - sculpt_expand_mask_update_task(ss, expand_cache->nodes[i]); + sculpt_expand_mask_update_task(ss, mask_write, expand_cache->nodes[i]); } }); break; + } case SCULPT_EXPAND_TARGET_FACE_SETS: sculpt_expand_face_sets_update(ss, expand_cache); break; @@ -2098,6 +2137,7 @@ static int sculpt_expand_invoke(bContext *C, wmOperator *op, const wmEvent *even Object *ob = CTX_data_active_object(C); SculptSession *ss = ob->sculpt; Sculpt *sd = CTX_data_tool_settings(C)->sculpt; + Mesh *mesh = static_cast(ob->data); SCULPT_stroke_id_next(ob); @@ -2133,18 +2173,7 @@ static int sculpt_expand_invoke(bContext *C, wmOperator *op, const wmEvent *even } if (ok) { - /* TODO: implement SCULPT_vertex_mask_set and use it here. */ - Vector nodes = blender::bke::pbvh::search_gather(ss->pbvh, {}); - for (PBVHNode *node : nodes) { - PBVHVertexIter vd; - - BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { - *vd.mask = 1.0f; - } - BKE_pbvh_vertex_iter_end; - - BKE_pbvh_node_mark_update_mask(node); - } + write_mask_data(ss, blender::Array(SCULPT_vertex_count_get(ss), 1.0f)); } } } @@ -2158,7 +2187,6 @@ static int sculpt_expand_invoke(bContext *C, wmOperator *op, const wmEvent *even return OPERATOR_CANCELLED; } - Mesh *mesh = static_cast(ob->data); if (ss->expand_cache->target == SCULPT_EXPAND_TARGET_FACE_SETS) { ss->face_sets = BKE_sculpt_face_sets_ensure(ob); } diff --git a/source/blender/editors/sculpt_paint/sculpt_filter_mask.cc b/source/blender/editors/sculpt_paint/sculpt_filter_mask.cc index 6975a2e6172..633a78b40fa 100644 --- a/source/blender/editors/sculpt_paint/sculpt_filter_mask.cc +++ b/source/blender/editors/sculpt_paint/sculpt_filter_mask.cc @@ -10,7 +10,7 @@ #include "BLI_task.h" -#include "DNA_meshdata_types.h" +#include "DNA_mesh_types.h" #include "DNA_modifier_types.h" #include "BKE_context.h" @@ -60,7 +60,11 @@ static EnumPropertyItem prop_mask_filter_types[] = { {0, nullptr, 0, nullptr, nullptr}, }; -static void mask_filter_task(SculptSession *ss, const int mode, float *prev_mask, PBVHNode *node) +static void mask_filter_task(SculptSession *ss, + const int mode, + float *prev_mask, + const SculptMaskWriteInfo mask_write, + PBVHNode *node) { bool update = false; @@ -78,26 +82,27 @@ static void mask_filter_task(SculptSession *ss, const int mode, float *prev_mask BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { float delta, gain, offset, max, min; - float prev_val = *vd.mask; + + float mask = *vd.mask; SculptVertexNeighborIter ni; switch (mode) { case MASK_FILTER_SMOOTH: case MASK_FILTER_SHARPEN: { float val = SCULPT_neighbor_mask_average(ss, vd.vertex); - val -= *vd.mask; + val -= mask; if (mode == MASK_FILTER_SMOOTH) { - *vd.mask += val; + mask += val; } else if (mode == MASK_FILTER_SHARPEN) { - if (*vd.mask > 0.5f) { - *vd.mask += 0.05f; + if (mask > 0.5f) { + mask += 0.05f; } else { - *vd.mask -= 0.05f; + mask -= 0.05f; } - *vd.mask += val / 2.0f; + mask += val / 2.0f; } break; } @@ -110,7 +115,7 @@ static void mask_filter_task(SculptSession *ss, const int mode, float *prev_mask } } SCULPT_VERTEX_NEIGHBORS_ITER_END(ni); - *vd.mask = max; + mask = max; break; case MASK_FILTER_SHRINK: min = 1.0f; @@ -121,7 +126,7 @@ static void mask_filter_task(SculptSession *ss, const int mode, float *prev_mask } } SCULPT_VERTEX_NEIGHBORS_ITER_END(ni); - *vd.mask = min; + mask = min; break; case MASK_FILTER_CONTRAST_INCREASE: case MASK_FILTER_CONTRAST_DECREASE: @@ -135,11 +140,12 @@ static void mask_filter_task(SculptSession *ss, const int mode, float *prev_mask delta *= -1.0f; offset = gain * (delta); } - *vd.mask = gain * (*vd.mask) + offset; + mask = gain * (mask) + offset; break; } - *vd.mask = clamp_f(*vd.mask, 0.0f, 1.0f); - if (*vd.mask != prev_val) { + mask = clamp_f(mask, 0.0f, 1.0f); + if (mask != *vd.mask) { + SCULPT_mask_vert_set(BKE_pbvh_type(ss->pbvh), mask_write, mask, vd); update = true; } } @@ -188,6 +194,8 @@ static int sculpt_mask_filter_exec(bContext *C, wmOperator *op) iterations = int(num_verts / 50000.0f) + 1; } + const SculptMaskWriteInfo mask_write = SCULPT_mask_get_for_write(ob->sculpt); + for (int i = 0; i < iterations; i++) { if (ELEM(filter_type, MASK_FILTER_GROW, MASK_FILTER_SHRINK)) { prev_mask = static_cast(MEM_mallocN(num_verts * sizeof(float), __func__)); @@ -199,7 +207,7 @@ static int sculpt_mask_filter_exec(bContext *C, wmOperator *op) threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { for (const int i : range) { - mask_filter_task(ss, filter_type, prev_mask, nodes[i]); + mask_filter_task(ss, filter_type, prev_mask, mask_write, nodes[i]); } }); @@ -221,10 +229,11 @@ void SCULPT_mask_filter_smooth_apply(Sculpt * /*sd*/, const int smooth_iterations) { using namespace blender; + const SculptMaskWriteInfo mask_write = SCULPT_mask_get_for_write(ob->sculpt); for (int i = 0; i < smooth_iterations; i++) { threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { for (const int i : range) { - mask_filter_task(ob->sculpt, MASK_FILTER_SMOOTH, nullptr, nodes[i]); + mask_filter_task(ob->sculpt, MASK_FILTER_SMOOTH, nullptr, mask_write, nodes[i]); } }); } diff --git a/source/blender/editors/sculpt_paint/sculpt_intern.hh b/source/blender/editors/sculpt_paint/sculpt_intern.hh index 04f99e86e3c..964f68489cc 100644 --- a/source/blender/editors/sculpt_paint/sculpt_intern.hh +++ b/source/blender/editors/sculpt_paint/sculpt_intern.hh @@ -851,6 +851,30 @@ bool SCULPT_stroke_is_first_brush_step_of_symmetry_pass(StrokeCache *cache); /** \name Sculpt mesh accessor API * \{ */ +struct SculptMaskWriteInfo { + float *layer = nullptr; + int bm_offset = -1; +}; +SculptMaskWriteInfo SCULPT_mask_get_for_write(SculptSession *ss); +inline void SCULPT_mask_vert_set(const PBVHType type, + const SculptMaskWriteInfo mask_write, + const float value, + PBVHVertexIter &vd) +{ + switch (type) { + case PBVH_FACES: + mask_write.layer[vd.index] = value; + break; + case PBVH_BMESH: + BM_ELEM_CD_SET_FLOAT(vd.bm_vert, mask_write.bm_offset, value); + break; + case PBVH_GRIDS: + *CCG_elem_mask(&vd.key, vd.grid) = value; + break; + } +} +void SCULPT_mask_write_array(SculptSession *ss, Span nodes, Span mask); + /** Ensure random access; required for PBVH_BMESH */ void SCULPT_vertex_random_access_ensure(SculptSession *ss); diff --git a/source/blender/editors/sculpt_paint/sculpt_mask_init.cc b/source/blender/editors/sculpt_paint/sculpt_mask_init.cc index 78d7b29ae2a..bbebd4eabf3 100644 --- a/source/blender/editors/sculpt_paint/sculpt_mask_init.cc +++ b/source/blender/editors/sculpt_paint/sculpt_mask_init.cc @@ -14,7 +14,7 @@ #include "PIL_time.h" #include "DNA_brush_types.h" -#include "DNA_meshdata_types.h" +#include "DNA_mesh_types.h" #include "DNA_modifier_types.h" #include "DNA_object_types.h" @@ -73,25 +73,31 @@ static EnumPropertyItem prop_sculpt_mask_init_mode_types[] = { {0, nullptr, 0, nullptr, nullptr}, }; -static void mask_init_task(Object *ob, const int mode, const int seed, PBVHNode *node) +static void mask_init_task(Object *ob, + const int mode, + const int seed, + const SculptMaskWriteInfo mask_write, + PBVHNode *node) { SculptSession *ss = ob->sculpt; PBVHVertexIter vd; SCULPT_undo_push_node(ob, node, SCULPT_UNDO_MASK); BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { + float mask; switch (mode) { case SCULPT_MASK_INIT_RANDOM_PER_VERTEX: - *vd.mask = BLI_hash_int_01(vd.index + seed); + mask = BLI_hash_int_01(vd.index + seed); break; case SCULPT_MASK_INIT_RANDOM_PER_FACE_SET: { const int face_set = SCULPT_vertex_face_set_get(ss, vd.vertex); - *vd.mask = BLI_hash_int_01(face_set + seed); + mask = BLI_hash_int_01(face_set + seed); break; } case SCULPT_MASK_INIT_RANDOM_PER_LOOSE_PART: - *vd.mask = BLI_hash_int_01(SCULPT_vertex_island_get(ss, vd.vertex) + seed); + mask = BLI_hash_int_01(SCULPT_vertex_island_get(ss, vd.vertex) + seed); break; } + SCULPT_mask_vert_set(BKE_pbvh_type(ss->pbvh), mask_write, mask, vd); } BKE_pbvh_vertex_iter_end; BKE_pbvh_node_mark_update_mask(node); @@ -126,9 +132,10 @@ static int sculpt_mask_init_exec(bContext *C, wmOperator *op) const int mask_init_seed = PIL_check_seconds_timer(); + const SculptMaskWriteInfo mask_write = SCULPT_mask_get_for_write(ss); threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { for (const int i : range) { - mask_init_task(ob, mode, mask_init_seed, nodes[i]); + mask_init_task(ob, mode, mask_init_seed, mask_write, nodes[i]); } }); diff --git a/source/blender/editors/sculpt_paint/sculpt_ops.cc b/source/blender/editors/sculpt_paint/sculpt_ops.cc index 6e1e55b18c2..7012c5c3ced 100644 --- a/source/blender/editors/sculpt_paint/sculpt_ops.cc +++ b/source/blender/editors/sculpt_paint/sculpt_ops.cc @@ -734,6 +734,7 @@ static void do_mask_by_color_contiguous_update_node(Object *ob, const float *mask_by_color_floodfill, const bool invert, const bool preserve_mask, + const SculptMaskWriteInfo mask_write, PBVHNode *node) { SculptSession *ss = ob->sculpt; @@ -745,10 +746,14 @@ static void do_mask_by_color_contiguous_update_node(Object *ob, BKE_pbvh_vertex_iter_begin (ss->pbvh, node, vd, PBVH_ITER_UNIQUE) { const float current_mask = *vd.mask; const float new_mask = mask_by_color_floodfill[vd.index]; - *vd.mask = sculpt_mask_by_color_final_mask_get(current_mask, new_mask, invert, preserve_mask); - if (current_mask == *vd.mask) { + const float mask = sculpt_mask_by_color_final_mask_get( + current_mask, new_mask, invert, preserve_mask); + if (current_mask == current_mask) { continue; } + + SCULPT_mask_vert_set(BKE_pbvh_type(ss->pbvh), mask_write, mask, vd); + update_node = true; } BKE_pbvh_vertex_iter_end; @@ -818,10 +823,12 @@ static void sculpt_mask_by_color_contiguous(Object *object, SCULPT_floodfill_free(&flood); Vector nodes = blender::bke::pbvh::search_gather(ss->pbvh, {}); + const SculptMaskWriteInfo mask_write = SCULPT_mask_get_for_write(ss); threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { for (const int i : range) { - do_mask_by_color_contiguous_update_node(object, new_mask, invert, preserve_mask, nodes[i]); + do_mask_by_color_contiguous_update_node( + object, new_mask, invert, preserve_mask, mask_write, nodes[i]); } }); @@ -833,6 +840,7 @@ static void do_mask_by_color_task(Object *ob, const bool invert, const bool preserve_mask, const PBVHVertRef mask_by_color_vertex, + const SculptMaskWriteInfo mask_write, PBVHNode *node) { SculptSession *ss = ob->sculpt; @@ -851,11 +859,14 @@ static void do_mask_by_color_task(Object *ob, const float current_mask = *vd.mask; const float new_mask = sculpt_mask_by_color_delta_get(active_color, col, threshold, invert); - *vd.mask = sculpt_mask_by_color_final_mask_get(current_mask, new_mask, invert, preserve_mask); - + const float mask = sculpt_mask_by_color_final_mask_get( + current_mask, new_mask, invert, preserve_mask); if (current_mask == *vd.mask) { continue; } + + SCULPT_mask_vert_set(BKE_pbvh_type(ss->pbvh), mask_write, mask, vd); + update_node = true; } BKE_pbvh_vertex_iter_end; @@ -874,10 +885,11 @@ static void sculpt_mask_by_color_full_mesh(Object *object, SculptSession *ss = object->sculpt; Vector nodes = blender::bke::pbvh::search_gather(ss->pbvh, {}); - + const SculptMaskWriteInfo mask_write = SCULPT_mask_get_for_write(ss); threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { for (const int i : range) { - do_mask_by_color_task(object, threshold, invert, preserve_mask, vertex, nodes[i]); + do_mask_by_color_task( + object, threshold, invert, preserve_mask, vertex, mask_write, nodes[i]); } }); } @@ -993,6 +1005,7 @@ static void sculpt_bake_cavity_exec_task(Object *ob, AutomaskingCache *automasking, const CavityBakeMixMode mode, const float factor, + const SculptMaskWriteInfo mask_write, PBVHNode *node) { SculptSession *ss = ob->sculpt; @@ -1032,7 +1045,7 @@ static void sculpt_bake_cavity_exec_task(Object *ob, mask = *vd.mask + (mask - *vd.mask) * factor; CLAMP(mask, 0.0f, 1.0f); - *vd.mask = mask; + SCULPT_mask_vert_set(BKE_pbvh_type(ss->pbvh), mask_write, mask, vd); } BKE_pbvh_vertex_iter_end; @@ -1122,10 +1135,11 @@ static int sculpt_bake_cavity_exec(bContext *C, wmOperator *op) SCULPT_stroke_id_next(ob); AutomaskingCache *automasking = SCULPT_automasking_cache_init(&sd2, &brush2, ob); + const SculptMaskWriteInfo mask_write = SCULPT_mask_get_for_write(ss); threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { for (const int i : range) { - sculpt_bake_cavity_exec_task(ob, automasking, mode, factor, nodes[i]); + sculpt_bake_cavity_exec_task(ob, automasking, mode, factor, mask_write, nodes[i]); } }); diff --git a/source/blender/editors/sculpt_paint/sculpt_smooth.cc b/source/blender/editors/sculpt_paint/sculpt_smooth.cc index 40074ef50a8..190327154a5 100644 --- a/source/blender/editors/sculpt_paint/sculpt_smooth.cc +++ b/source/blender/editors/sculpt_paint/sculpt_smooth.cc @@ -260,6 +260,7 @@ static void do_smooth_brush_task(Object *ob, Sculpt *sd, const Brush *brush, const bool smooth_mask, + const SculptMaskWriteInfo mask_write, float bstrength, PBVHNode *node) { @@ -298,8 +299,10 @@ static void do_smooth_brush_task(Object *ob, if (smooth_mask) { float val = SCULPT_neighbor_mask_average(ss, vd.vertex) - *vd.mask; val *= fade * bstrength; - *vd.mask += val; - CLAMP(*vd.mask, 0.0f, 1.0f); + float new_mask = *vd.mask + val; + CLAMP(new_mask, 0.0f, 1.0f); + + SCULPT_mask_vert_set(BKE_pbvh_type(ss->pbvh), mask_write, new_mask, vd); } else { float avg[3], val[3]; @@ -337,9 +340,14 @@ void SCULPT_smooth( for (iteration = 0; iteration <= count; iteration++) { const float strength = (iteration != count) ? 1.0f : last; + SculptMaskWriteInfo mask_write; + if (smooth_mask) { + mask_write = SCULPT_mask_get_for_write(ss); + } + threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { for (const int i : range) { - do_smooth_brush_task(ob, sd, brush, smooth_mask, strength, nodes[i]); + do_smooth_brush_task(ob, sd, brush, smooth_mask, mask_write, strength, nodes[i]); } }); } diff --git a/source/blender/editors/sculpt_paint/sculpt_undo.cc b/source/blender/editors/sculpt_paint/sculpt_undo.cc index d9f1a6fdf07..01d00628a02 100644 --- a/source/blender/editors/sculpt_paint/sculpt_undo.cc +++ b/source/blender/editors/sculpt_paint/sculpt_undo.cc @@ -567,16 +567,18 @@ static bool sculpt_undo_restore_mask(bContext *C, SculptUndoNode *unode, bool *m ViewLayer *view_layer = CTX_data_view_layer(C); BKE_view_layer_synced_ensure(scene, view_layer); Object *ob = BKE_view_layer_active_object_get(view_layer); + Mesh *mesh = BKE_object_get_original_mesh(ob); SculptSession *ss = ob->sculpt; SubdivCCG *subdiv_ccg = ss->subdiv_ccg; - float *vmask; int *index; if (unode->maxvert) { /* Regular mesh restore. */ + float *vmask = static_cast( + CustomData_get_layer_for_write(&mesh->vert_data, CD_PAINT_MASK, mesh->totvert)); + ss->vmask = vmask; index = unode->index; - vmask = ss->vmask; for (int i = 0; i < unode->totvert; i++) { if (vmask[index[i]] != unode->mask[i]) {