Sculpt: Add global automasking propagation steps #117316

Merged
Hans Goudey merged 8 commits from Sean-Kim/blender:102377-auto-masking into main 2024-01-29 15:39:50 +01:00
11 changed files with 87 additions and 31 deletions
Showing only changes of commit 619e6d440a - Show all commits

View File

@ -8527,7 +8527,7 @@ class VIEW3D_PT_sculpt_automasking(Panel):
col.prop(sculpt, "use_automasking_boundary_face_sets", text="Face Sets Boundary")
if sculpt.use_automasking_boundary_edges or sculpt.use_automasking_boundary_face_sets:
col.prop(sculpt.brush, "automasking_boundary_edges_propagation_steps")
col.prop(sculpt, "automasking_boundary_edges_propagation_steps")
col.separator()

View File

@ -29,7 +29,7 @@ extern "C" {
/* Blender file format version. */
#define BLENDER_FILE_VERSION BLENDER_VERSION
#define BLENDER_FILE_SUBVERSION 12
#define BLENDER_FILE_SUBVERSION 13
/* Minimum Blender version that supports reading file written with the current
* version. Older Blender versions will test this and cancel loading the file, showing a warning to

View File

@ -2653,6 +2653,21 @@ void blo_do_versions_400(FileData *fd, Library * /*lib*/, Main *bmain)
FOREACH_NODETREE_END;
}
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 401, 13)) {
if (!DNA_struct_member_exists(
Sean-Kim marked this conversation as resolved Outdated

The DNA_struct_member_exists check should be unnecessary given the file subversion bump.

The `DNA_struct_member_exists` check should be unnecessary given the file subversion bump.

Oh, makes sense - I got a bit confused with seeing some of the previous changes in this file that do both. I assumed that it was a prerequisite to have both layers of checks.

Oh, makes sense - I got a bit confused with seeing some of the previous changes in this file that do both. I assumed that it was a prerequisite to have both layers of checks.
fd->filesdna, "Sculpt", "int", "automasking_boundary_edges_propagation_steps"))
{
LISTBASE_FOREACH (Scene *, scene, &bmain->scenes) {
Sculpt *sculpt = scene->toolsettings->sculpt;
if (sculpt != nullptr) {
Sculpt default_sculpt = *(DNA_struct_default_get(Sculpt));
sculpt->automasking_boundary_edges_propagation_steps =
default_sculpt.automasking_boundary_edges_propagation_steps;
}
}
}
}
Sean-Kim marked this conversation as resolved Outdated

The DNA_struct_member_exists check can be removed here, it should be redundant with the file subversion bump

The `DNA_struct_member_exists` check can be removed here, it should be redundant with the file subversion bump

Whoops, thought I removed this in the previous commit

Whoops, thought I removed this in the previous commit
/**
* Always bump subversion in BKE_blender_version.h when adding versioning
* code here, and wrap it inside a MAIN_VERSION_FILE_ATLEAST check.

View File

@ -383,6 +383,12 @@ static void blo_update_defaults_scene(Main *bmain, Scene *scene)
if (idprop) {
IDP_ClearProperty(idprop);
}
/* Ensure sculpt-mode global automasking_boundary_edges_propagation_steps is defaulted
Sean-Kim marked this conversation as resolved
Review

This is basically the point of this function, so this comment isn't adding much. I'd suggest removing it.

This is basically the point of this function, so this comment isn't adding much. I'd suggest removing it.
* correctly.*/
if (ts->sculpt) {
ts->sculpt->automasking_boundary_edges_propagation_steps = 1;
}
}
void BLO_update_defaults_startup_blend(Main *bmain, const char *app_template)

View File

@ -185,9 +185,19 @@ static bool sculpt_automasking_is_constrained_by_radius(const Brush *br)
return false;
}
/* Fetch the propogation_steps value, preferring the brush level value over the global sculpt tool
* * value. */
Sean-Kim marked this conversation as resolved Outdated

* * value -> * value

` * * value` -> ` * value`
static int SCULPT_automasking_boundary_propagation_steps(const Sculpt *sd, const Brush *brush)
{
return brush && brush->automasking_flags &
(BRUSH_AUTOMASKING_BOUNDARY_EDGES | BRUSH_AUTOMASKING_BOUNDARY_FACE_SETS) ?
brush->automasking_boundary_edges_propagation_steps :
sd->automasking_boundary_edges_propagation_steps;
}
/* Determine if the given automasking settings require values to be precomputed and cached. */
static bool SCULPT_automasking_needs_factors_cache(const Sculpt *sd, const Brush *brush)
{
const int automasking_flags = sculpt_automasking_mode_effective_bits(sd, brush);
if (automasking_flags & BRUSH_AUTOMASKING_TOPOLOGY && brush &&
@ -196,11 +206,17 @@ static bool SCULPT_automasking_needs_factors_cache(const Sculpt *sd, const Brush
return true;
}
if (automasking_flags & (BRUSH_AUTOMASKING_BOUNDARY_EDGES |
BRUSH_AUTOMASKING_BOUNDARY_FACE_SETS | BRUSH_AUTOMASKING_VIEW_NORMAL))
{
/* TODO: I'm unsure why the BRUSH_AUTOMASKING_VIEW_NORMAL cares at all about the propagation
Sean-Kim marked this conversation as resolved Outdated

Agreed, but how about handling that in a separate PR?

Agreed, but how about handling that in a separate PR?
* steps being non 1, should this only check for brush being non-nullptr? */
if (automasking_flags & BRUSH_AUTOMASKING_VIEW_NORMAL) {
return brush && brush->automasking_boundary_edges_propagation_steps != 1;
}
if (automasking_flags &
(BRUSH_AUTOMASKING_BOUNDARY_EDGES | BRUSH_AUTOMASKING_BOUNDARY_FACE_SETS))
{
return SCULPT_automasking_boundary_propagation_steps(sd, brush) != 1;
}
return false;
}
@ -802,10 +818,10 @@ bool tool_can_reuse_automask(int sculpt_tool)
SCULPT_TOOL_DRAW_FACE_SETS);
}
/* TODO: Update Sculpt * and Brush * to be const */
Sean-Kim marked this conversation as resolved Outdated

Agreed, but let's not add these todo comments in this PR

Agreed, but let's not add these todo comments in this PR
Cache *cache_init(Sculpt *sd, Brush *brush, Object *ob)
{
SculptSession *ss = ob->sculpt;
const int totvert = SCULPT_vertex_count_get(ss);
if (!is_enabled(sd, ss, brush)) {
return nullptr;
@ -817,14 +833,14 @@ Cache *cache_init(Sculpt *sd, Brush *brush, Object *ob)
automasking->current_stroke_id = ss->stroke_id;
bool use_stroke_id = false;
/* Initialize and ensure variables on both SculptSession and the Cache. */
Sean-Kim marked this conversation as resolved Outdated

These changes are unrelated to the goal of the PR. I generally agree, but better to propose them separately, like above.

These changes are unrelated to the goal of the PR. I generally agree, but better to propose them separately, like above.

Just to be clear - do you mean the general restructuring that happened in the cache_init function, or just the added comments?

Just to be clear - do you mean the general restructuring that happened in the `cache_init` function, or just the added comments?

The general restructuring-- the changes should basically just be related to the functionality change in this PR

The general restructuring-- the changes should basically just be related to the functionality change in this PR
int mode = sculpt_automasking_mode_effective_bits(sd, brush);
if (mode & BRUSH_AUTOMASKING_TOPOLOGY && ss->active_vertex.i != PBVH_REF_NONE) {
SCULPT_topology_islands_ensure(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;
@ -881,6 +897,8 @@ Cache *cache_init(Sculpt *sd, Brush *brush, Object *ob)
}
}
/* If the current automasking modes do not require the cache to be precompued,
* avoid populating data on the vertex level. */
if (!SCULPT_automasking_needs_factors_cache(sd, brush)) {
if (ss->attrs.automasking_factor) {
BKE_sculpt_attribute_destroy(ob, ss->attrs.automasking_factor);
@ -898,29 +916,16 @@ Cache *cache_init(Sculpt *sd, Brush *brush, Object *ob)
SCULPT_ATTRIBUTE_NAME(automasking_factor),
&params);
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);
(*(float *)SCULPT_vertex_attr_get(vertex, ss->attrs.automasking_factor)) = initial_value;
}
const int boundary_propagation_steps = brush ?
brush->automasking_boundary_edges_propagation_steps :
1;
/* Additive modes. */
if (mode_enabled(sd, brush, BRUSH_AUTOMASKING_TOPOLOGY)) {
SCULPT_vertex_random_access_ensure(ss);
@ -935,6 +940,7 @@ Cache *cache_init(Sculpt *sd, Brush *brush, Object *ob)
sculpt_face_sets_automasking_init(sd, ob);
}
const int boundary_propagation_steps = SCULPT_automasking_boundary_propagation_steps(sd, brush);
if (mode_enabled(sd, brush, BRUSH_AUTOMASKING_BOUNDARY_EDGES)) {
SCULPT_vertex_random_access_ensure(ss);
boundary_automasking_init(ob, AUTOMASK_INIT_BOUNDARY_EDGES, boundary_propagation_steps);

View File

@ -1286,7 +1286,17 @@ float factor_get(Cache *automasking,
* brushes and filter. */
Cache *active_cache_get(SculptSession *ss);
Sean-Kim marked this conversation as resolved Outdated

This comment change is still unrelated to the goal of the path, it should be remove from this PR

This comment change is still unrelated to the goal of the path, it should be remove from this PR
/* Brush can be null. */
/**
* 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
*/
Cache *cache_init(Sculpt *sd, Brush *brush, Object *ob);
void cache_free(Cache *automasking);

View File

@ -163,6 +163,8 @@ typedef struct BrushCurvesSculptSettings {
struct CurveMapping *curve_parameter_falloff;
} BrushCurvesSculptSettings;
/** Max number of propagation steps for automasking settings.*/
#define AUTOMASKING_BOUNDARY_EDGES_MAX_PROPAGATION_STEPS 20
typedef struct Brush {
DNA_DEFINE_CXX_METHODS(Brush)
@ -313,7 +315,9 @@ typedef struct Brush {
int deform_target;
/* automasking */
/* Automasking mode flags
* See rna_brush.cc#rna_enum_brush_automasking_flag_items
Sean-Kim marked this conversation as resolved Outdated

Unrelated change

Unrelated change
*/
int automasking_flags;
int automasking_boundary_edges_propagation_steps;

View File

@ -418,6 +418,7 @@
.automasking_start_normal_falloff = 0.25f, \
.automasking_view_normal_limit = 1.570796, /* 0.5 * pi. */ \
.automasking_view_normal_falloff = 0.25f, \
.automasking_boundary_edges_propagation_steps = 1, \
.flags = SCULPT_DYNTOPO_SUBDIVIDE | SCULPT_DYNTOPO_COLLAPSE,\
.paint = {\
.symmetry_flags = PAINT_SYMMETRY_FEATHER,\

View File

@ -1087,6 +1087,9 @@ typedef struct Sculpt {
/** Transform tool. */
int transform_mode;
/* Automasking mode flags
* See rna_brush.cc#rna_enum_brush_automasking_flag_items
Sean-Kim marked this conversation as resolved Outdated

Unrelated change

Unrelated change
*/
int automasking_flags;
// /* Control tablet input. */
@ -1107,9 +1110,9 @@ typedef struct Sculpt {
float constant_detail;
float detail_percent;
int automasking_boundary_edges_propagation_steps;
int automasking_cavity_blur_steps;
float automasking_cavity_factor;
char _pad[4];
float automasking_start_normal_limit, automasking_start_normal_falloff;
float automasking_view_normal_limit, automasking_view_normal_falloff;

View File

@ -3183,8 +3183,8 @@ static void rna_def_brush(BlenderRNA *brna)
prop = RNA_def_property(
srna, "automasking_boundary_edges_propagation_steps", PROP_INT, PROP_UNSIGNED);
RNA_def_property_int_sdna(prop, nullptr, "automasking_boundary_edges_propagation_steps");
RNA_def_property_range(prop, 1, 20);
RNA_def_property_ui_range(prop, 1, 20, 1, 3);
RNA_def_property_range(prop, 1, AUTOMASKING_BOUNDARY_EDGES_MAX_PROPAGATION_STEPS);
RNA_def_property_ui_range(prop, 1, AUTOMASKING_BOUNDARY_EDGES_MAX_PROPAGATION_STEPS, 1, -1);
RNA_def_property_ui_text(prop,
"Propagation Steps",
"Distance where boundary edge automasking is going to protect vertices "

View File

@ -876,6 +876,17 @@ static void rna_def_sculpt(BlenderRNA *brna)
RNA_def_property_update(prop, NC_SCENE | ND_TOOLSETTINGS, nullptr);
} while ((++entry)->identifier);
prop = RNA_def_property(
srna, "automasking_boundary_edges_propagation_steps", PROP_INT, PROP_UNSIGNED);
RNA_def_property_int_sdna(prop, nullptr, "automasking_boundary_edges_propagation_steps");
RNA_def_property_range(prop, 1, AUTOMASKING_BOUNDARY_EDGES_MAX_PROPAGATION_STEPS);
RNA_def_property_ui_range(prop, 1, AUTOMASKING_BOUNDARY_EDGES_MAX_PROPAGATION_STEPS, 1, -1);
RNA_def_property_ui_text(prop,
"Propagation Steps",
"Distance where boundary edge automasking is going to protect vertices "
"from the fully masked edge");
RNA_def_property_update(prop, NC_SCENE | ND_TOOLSETTINGS, nullptr);
prop = RNA_def_property(srna, "automasking_cavity_factor", PROP_FLOAT, PROP_FACTOR);
RNA_def_property_float_sdna(prop, nullptr, "automasking_cavity_factor");
RNA_def_property_ui_text(prop, "Cavity Factor", "The contrast of the cavity mask");