From 637ba52d244539902d43b3e9a0fc2d71b9a74f5d Mon Sep 17 00:00:00 2001 From: Sean Kim Date: Mon, 29 Jan 2024 14:32:08 -0800 Subject: [PATCH 1/3] Cleanup: Perform cleanup on sculpt_automasking.cc * Adds documentation / comments * Applies const where possible to cache_init function * Minor cleanup of conditionals and initialization --- .../sculpt_paint/sculpt_automasking.cc | 39 +++++++++---------- .../editors/sculpt_paint/sculpt_intern.hh | 14 ++++++- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/source/blender/editors/sculpt_paint/sculpt_automasking.cc b/source/blender/editors/sculpt_paint/sculpt_automasking.cc index 49dd1afd0c5..ca438dd3635 100644 --- a/source/blender/editors/sculpt_paint/sculpt_automasking.cc +++ b/source/blender/editors/sculpt_paint/sculpt_automasking.cc @@ -204,6 +204,9 @@ static bool needs_factors_cache(const Sculpt *sd, const Brush *brush) return true; } + /* TODO: Unsure why the BRUSH_AUTOMASKING_VIEW_NORMAL mode requires a check against the + * propagation steps variable. This can probably be limited to checking if the brush itself + * exists. */ if (automasking_flags & BRUSH_AUTOMASKING_VIEW_NORMAL) { return brush && brush->automasking_boundary_edges_propagation_steps != 1; } @@ -632,10 +635,10 @@ static bool floodfill_cb( SCULPT_vertex_co_get(ss, to_v), data->location, data->radius, data->symm)); } -static void topology_automasking_init(Sculpt *sd, Object *ob) +static void topology_automasking_init(const Sculpt *sd, Object *ob) { SculptSession *ss = ob->sculpt; - Brush *brush = BKE_paint_brush(&sd->paint); + const Brush *brush = BKE_paint_brush_for_read(&sd->paint); const int totvert = SCULPT_vertex_count_get(ss); for (int i : IndexRange(totvert)) { @@ -661,10 +664,10 @@ static void topology_automasking_init(Sculpt *sd, Object *ob) flood_fill::execute(ss, &flood, floodfill_cb, &fdata); } -static void init_face_sets_masking(Sculpt *sd, Object *ob) +static void init_face_sets_masking(const Sculpt *sd, Object *ob) { SculptSession *ss = ob->sculpt; - Brush *brush = BKE_paint_brush(&sd->paint); + const Brush *brush = BKE_paint_brush_for_read(&sd->paint); if (!is_enabled(sd, ss, brush)) { return; @@ -740,7 +743,10 @@ static void init_boundary_masking(Object *ob, eBoundaryAutomaskMode mode, int pr } /* Updates the cached values, preferring brush settings over tool-level settings. */ -static void cache_settings_update(Cache &automasking, SculptSession *ss, Sculpt *sd, Brush *brush) +static void cache_settings_update(Cache &automasking, + SculptSession *ss, + const Sculpt *sd, + const Brush *brush) { automasking.settings.flags = calc_effective_bits(sd, brush); automasking.settings.initial_face_set = face_set::active_face_set_get(ss); @@ -816,10 +822,9 @@ bool tool_can_reuse_automask(int sculpt_tool) SCULPT_TOOL_DRAW_FACE_SETS); } -std::unique_ptr cache_init(Sculpt *sd, Brush *brush, Object *ob) +std::unique_ptr cache_init(const Sculpt *sd, const Brush *brush, Object *ob) { SculptSession *ss = ob->sculpt; - const int totvert = SCULPT_vertex_count_get(ss); if (!is_enabled(sd, ss, brush)) { return nullptr; @@ -831,7 +836,6 @@ std::unique_ptr cache_init(Sculpt *sd, Brush *brush, Object *ob) automasking->current_stroke_id = ss->stroke_id; - bool use_stroke_id = false; int mode = calc_effective_bits(sd, brush); if (mode & BRUSH_AUTOMASKING_TOPOLOGY && ss->active_vertex.i != PBVH_REF_NONE) { @@ -839,6 +843,7 @@ std::unique_ptr cache_init(Sculpt *sd, Brush *brush, Object *ob) automasking->settings.initial_island_nr = SCULPT_vertex_island_get(ss, ss->active_vertex); } + bool use_stroke_id = false; if ((mode & BRUSH_AUTOMASKING_VIEW_OCCLUSION) && (mode & BRUSH_AUTOMASKING_VIEW_NORMAL)) { use_stroke_id = true; @@ -895,6 +900,8 @@ std::unique_ptr cache_init(Sculpt *sd, Brush *brush, Object *ob) } } + /* Avoid precomputing data on the vertex level if the current auto-masking modes do not require + * it to function. */ if (!needs_factors_cache(sd, brush)) { if (ss->attrs.automasking_factor) { BKE_sculpt_attribute_destroy(ob, ss->attrs.automasking_factor); @@ -912,19 +919,11 @@ std::unique_ptr cache_init(Sculpt *sd, Brush *brush, Object *ob) SCULPT_ATTRIBUTE_NAME(automasking_factor), ¶ms); - float initial_value; - - /* Topology, boundary and boundary face sets build up the mask - * from zero which other modes can subtract from. If none of them are - * enabled initialize to 1. - */ - if (!(mode & BRUSH_AUTOMASKING_TOPOLOGY)) { - initial_value = 1.0f; - } - else { - initial_value = 0.0f; - } + /* Topology builds up the mask from zero which other modes can subtract from. + * If it isn't enabled, initialize to 1. */ + float initial_value = !(mode & BRUSH_AUTOMASKING_TOPOLOGY) ? 1.0f : 0.0f; + const int totvert = SCULPT_vertex_count_get(ss); for (int i : IndexRange(totvert)) { PBVHVertRef vertex = BKE_pbvh_index_to_vertex(ss->pbvh, i); diff --git a/source/blender/editors/sculpt_paint/sculpt_intern.hh b/source/blender/editors/sculpt_paint/sculpt_intern.hh index 0c6dcede029..5e191acc6a2 100644 --- a/source/blender/editors/sculpt_paint/sculpt_intern.hh +++ b/source/blender/editors/sculpt_paint/sculpt_intern.hh @@ -1285,8 +1285,18 @@ float factor_get(Cache *automasking, * brushes and filter. */ Cache *active_cache_get(SculptSession *ss); -/* Brush can be null. */ -std::unique_ptr cache_init(Sculpt *sd, Brush *brush, Object *ob); +/** + * Creates and initializes an automasking cache. + * + * For automasking modes that cannot be calculated in real time, + * data is also stored at the vertex level prior to the stroke starting. + * + * \param sd: the sculpt tool + * \param brush: the brush being used (optional) + * \param ob: the object being operated on + * \return the automask cache + */ +std::unique_ptr cache_init(const Sculpt *sd, const Brush *brush, Object *ob); void cache_free(Cache *automasking); bool mode_enabled(const Sculpt *sd, const Brush *br, eAutomasking_flag mode); -- 2.30.2 From 41c651d87253366517d68e7b477c6544ff12a582 Mon Sep 17 00:00:00 2001 From: Sean Kim Date: Wed, 31 Jan 2024 11:13:43 -0800 Subject: [PATCH 2/3] Address PR comments --- source/blender/editors/sculpt_paint/sculpt_automasking.cc | 8 +++++--- source/blender/editors/sculpt_paint/sculpt_intern.hh | 6 +----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/source/blender/editors/sculpt_paint/sculpt_automasking.cc b/source/blender/editors/sculpt_paint/sculpt_automasking.cc index ca438dd3635..6ac743f0dbc 100644 --- a/source/blender/editors/sculpt_paint/sculpt_automasking.cc +++ b/source/blender/editors/sculpt_paint/sculpt_automasking.cc @@ -204,9 +204,6 @@ static bool needs_factors_cache(const Sculpt *sd, const Brush *brush) return true; } - /* TODO: Unsure why the BRUSH_AUTOMASKING_VIEW_NORMAL mode requires a check against the - * propagation steps variable. This can probably be limited to checking if the brush itself - * exists. */ if (automasking_flags & BRUSH_AUTOMASKING_VIEW_NORMAL) { return brush && brush->automasking_boundary_edges_propagation_steps != 1; } @@ -822,6 +819,11 @@ bool tool_can_reuse_automask(int sculpt_tool) SCULPT_TOOL_DRAW_FACE_SETS); } +std::unique_ptr cache_init(const Sculpt *sd, Object *ob) +{ + return cache_init(sd, nullptr, ob); +} + std::unique_ptr cache_init(const Sculpt *sd, const Brush *brush, Object *ob) { SculptSession *ss = ob->sculpt; diff --git a/source/blender/editors/sculpt_paint/sculpt_intern.hh b/source/blender/editors/sculpt_paint/sculpt_intern.hh index 5e191acc6a2..f4b3a5c6b23 100644 --- a/source/blender/editors/sculpt_paint/sculpt_intern.hh +++ b/source/blender/editors/sculpt_paint/sculpt_intern.hh @@ -1290,12 +1290,8 @@ Cache *active_cache_get(SculptSession *ss); * * For automasking modes that cannot be calculated in real time, * data is also stored at the vertex level prior to the stroke starting. - * - * \param sd: the sculpt tool - * \param brush: the brush being used (optional) - * \param ob: the object being operated on - * \return the automask cache */ +std::unique_ptr cache_init(const Sculpt *sd, Object *ob); std::unique_ptr cache_init(const Sculpt *sd, const Brush *brush, Object *ob); void cache_free(Cache *automasking); -- 2.30.2 From df9490c0b514ad4037e200d9aa8d5626d80167c3 Mon Sep 17 00:00:00 2001 From: Sean Kim Date: Wed, 31 Jan 2024 11:21:32 -0800 Subject: [PATCH 3/3] Update callers of automask::cache_init --- source/blender/editors/sculpt_paint/sculpt_cloth.cc | 2 +- source/blender/editors/sculpt_paint/sculpt_filter_color.cc | 2 +- source/blender/editors/sculpt_paint/sculpt_filter_mesh.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source/blender/editors/sculpt_paint/sculpt_cloth.cc b/source/blender/editors/sculpt_paint/sculpt_cloth.cc index 54ae74c7d85..c4397e63f1f 100644 --- a/source/blender/editors/sculpt_paint/sculpt_cloth.cc +++ b/source/blender/editors/sculpt_paint/sculpt_cloth.cc @@ -1556,7 +1556,7 @@ static int sculpt_cloth_filter_invoke(bContext *C, wmOperator *op, const wmEvent RNA_float_get(op->ptr, "area_normal_radius"), RNA_float_get(op->ptr, "strength")); - ss->filter_cache->automasking = auto_mask::cache_init(sd, nullptr, ob); + ss->filter_cache->automasking = auto_mask::cache_init(sd, ob); const float cloth_mass = RNA_float_get(op->ptr, "cloth_mass"); const float cloth_damping = RNA_float_get(op->ptr, "cloth_damping"); diff --git a/source/blender/editors/sculpt_paint/sculpt_filter_color.cc b/source/blender/editors/sculpt_paint/sculpt_filter_color.cc index 6f8151b2ebc..44319a12fa0 100644 --- a/source/blender/editors/sculpt_paint/sculpt_filter_color.cc +++ b/source/blender/editors/sculpt_paint/sculpt_filter_color.cc @@ -371,7 +371,7 @@ static int sculpt_color_filter_init(bContext *C, wmOperator *op) RNA_float_get(op->ptr, "strength")); filter::Cache *filter_cache = ss->filter_cache; filter_cache->active_face_set = SCULPT_FACE_SET_NONE; - filter_cache->automasking = auto_mask::cache_init(sd, nullptr, ob); + filter_cache->automasking = auto_mask::cache_init(sd, ob); return OPERATOR_PASS_THROUGH; } diff --git a/source/blender/editors/sculpt_paint/sculpt_filter_mesh.cc b/source/blender/editors/sculpt_paint/sculpt_filter_mesh.cc index c9b0a5bf738..4b6701e75d5 100644 --- a/source/blender/editors/sculpt_paint/sculpt_filter_mesh.cc +++ b/source/blender/editors/sculpt_paint/sculpt_filter_mesh.cc @@ -1010,7 +1010,7 @@ static int sculpt_mesh_filter_start(bContext *C, wmOperator *op) filter::Cache *filter_cache = ss->filter_cache; filter_cache->active_face_set = SCULPT_FACE_SET_NONE; - filter_cache->automasking = auto_mask::cache_init(sd, nullptr, ob); + filter_cache->automasking = auto_mask::cache_init(sd, ob); sculpt_filter_specific_init(filter_type, op, ss); -- 2.30.2