[Bf-blender-cvs] [7540842ca76] master: Fix T99592: Exact Boolean: Skip empty materials, add index-based option
Hans Goudey
noreply at git.blender.org
Mon Nov 28 21:06:57 CET 2022
Commit: 7540842ca7632dad4c2806989027a3a6b2a15668
Author: Hans Goudey
Date: Mon Nov 28 12:42:08 2022 -0600
Branches: master
https://developer.blender.org/rB7540842ca7632dad4c2806989027a3a6b2a15668
Fix T99592: Exact Boolean: Skip empty materials, add index-based option
**Empty Slot Fix**
Currently the boolean modifier transfers the default material from
meshes with no materials and empty material slots to the faces on the
base mesh. I added this in a2d59b2dac9e for the sake of consistency,
but the behavior is actually not useful at all. The default empty
material isn't chosen by users, it just signifies "nothing," so when
it replaces a material chosen by users, it feels like a bug.
This commit corrects that behavior by only transferring materials from
non-empty material slots. The implementation is now consistent between
exact mode of the boolean modifier and the geometry node.
**Index-Based Option**
"Index-based" is the new default material method for the boolean
modifier, to access the old behavior from before the breaking commit.
a2d59b2dac9e actually broke some Boolean workflows fundamentally, since
it was important to set up matching slot indices on each operand. That
isn't the cleanest workflow, and it breaks when materials change
procedurally, but historically that hasn't been a problem. The
"transfer" behavior transfers all materials except for empty slots,
but the fundamental problem is that there isn't a good way to specify
the result materials besides using the slot indices.
Even then, the transfer option is a bit more intuitive and useful for
some simpler situations, and it allows accessing the behavior that has
been in 3.2 and 3.3 for a long time, so it's also left in as an option.
The geometry node doesn't get this new option, in the hope that we'll
find a better solution in the future.
Differential Revision: https://developer.blender.org/D16187
===================================================================
M source/blender/blenkernel/BKE_mesh_boolean_convert.hh
M source/blender/blenkernel/intern/mesh_boolean_convert.cc
M source/blender/makesdna/DNA_modifier_types.h
M source/blender/makesrna/intern/rna_modifier.c
M source/blender/modifiers/intern/MOD_boolean.cc
M source/blender/nodes/geometry/nodes/node_geo_boolean.cc
===================================================================
diff --git a/source/blender/blenkernel/BKE_mesh_boolean_convert.hh b/source/blender/blenkernel/BKE_mesh_boolean_convert.hh
index 441783d46a1..77c90716d42 100644
--- a/source/blender/blenkernel/BKE_mesh_boolean_convert.hh
+++ b/source/blender/blenkernel/BKE_mesh_boolean_convert.hh
@@ -22,7 +22,7 @@ namespace blender::meshintersect {
* It is allowed for the pointers to be null, meaning the transformation is the identity.
* \param material_remaps: An array of maps from material slot numbers in the corresponding mesh
* to the material slot in the first mesh. It is OK for material_remaps or any of its constituent
- * arrays to be empty.
+ * arrays to be empty. A -1 value means that the original index should be used with no mapping.
* \param r_intersecting_edges: Array to store indices of edges on the resulting mesh in. These
* 'new' edges are the result of the intersections.
*/
diff --git a/source/blender/blenkernel/intern/mesh_boolean_convert.cc b/source/blender/blenkernel/intern/mesh_boolean_convert.cc
index 21d9baf7f7e..1252e90e11c 100644
--- a/source/blender/blenkernel/intern/mesh_boolean_convert.cc
+++ b/source/blender/blenkernel/intern/mesh_boolean_convert.cc
@@ -430,12 +430,14 @@ static void copy_poly_attributes(Mesh *dest_mesh,
const VArray<int> src_material_indices = orig_me->attributes().lookup_or_default<int>(
"material_index", ATTR_DOMAIN_FACE, 0);
const int src_index = src_material_indices[index_in_orig_me];
- if (material_remap.size() > 0 && material_remap.index_range().contains(src_index)) {
- dst_material_indices[mp_index] = material_remap[src_index];
+ if (material_remap.index_range().contains(src_index)) {
+ const int remapped_index = material_remap[src_index];
+ dst_material_indices[mp_index] = remapped_index >= 0 ? remapped_index : src_index;
}
else {
dst_material_indices[mp_index] = src_index;
}
+ BLI_assert(dst_material_indices[mp_index] >= 0);
}
/* Similar to copy_vert_attributes but for edge attributes. */
diff --git a/source/blender/makesdna/DNA_modifier_types.h b/source/blender/makesdna/DNA_modifier_types.h
index c4180071352..97e42efd5ac 100644
--- a/source/blender/makesdna/DNA_modifier_types.h
+++ b/source/blender/makesdna/DNA_modifier_types.h
@@ -899,10 +899,18 @@ typedef struct BooleanModifierData {
float double_threshold;
char operation;
char solver;
+ /** #BooleanModifierMaterialMode. */
+ char material_mode;
char flag;
char bm_flag;
+ char _pad[7];
} BooleanModifierData;
+typedef enum BooleanModifierMaterialMode {
+ eBooleanModifierMaterialMode_Index = 0,
+ eBooleanModifierMaterialMode_Transfer = 1,
+} BooleanModifierMaterialMode;
+
/** #BooleanModifierData.operation */
typedef enum {
eBooleanModifierOp_Intersect = 0,
diff --git a/source/blender/makesrna/intern/rna_modifier.c b/source/blender/makesrna/intern/rna_modifier.c
index c5da15003e1..edcf07b5336 100644
--- a/source/blender/makesrna/intern/rna_modifier.c
+++ b/source/blender/makesrna/intern/rna_modifier.c
@@ -2760,6 +2760,24 @@ static void rna_def_modifier_boolean(BlenderRNA *brna)
{0, NULL, 0, NULL, NULL},
};
+ static const EnumPropertyItem material_mode_items[] = {
+ {eBooleanModifierMaterialMode_Index,
+ "INDEX",
+ 0,
+ "Index Based",
+ "Set the material on new faces based on the order of the material slot lists. If a "
+ "material doesn't exist on the modifier object, the face will use the same material slot "
+ "or the first if the object doesn't have enough slots"},
+ {eBooleanModifierMaterialMode_Transfer,
+ "TRANSFER",
+ 0,
+ "Transfer",
+ "Transfer materials from non-empty slots to the result mesh, adding new materials as "
+ "necessary. For empty slots, fall back to using the same material index as the operand "
+ "mesh"},
+ {0, NULL, 0, NULL, NULL},
+ };
+
srna = RNA_def_struct(brna, "BooleanModifier", "Modifier");
RNA_def_struct_ui_text(srna, "Boolean Modifier", "Boolean operations modifier");
RNA_def_struct_sdna(srna, "BooleanModifierData");
@@ -2819,6 +2837,12 @@ static void rna_def_modifier_boolean(BlenderRNA *brna)
RNA_def_property_ui_text(prop, "Hole Tolerant", "Better results when there are holes (slower)");
RNA_def_property_update(prop, 0, "rna_Modifier_update");
+ prop = RNA_def_property(srna, "material_mode", PROP_ENUM, PROP_NONE);
+ RNA_def_property_enum_items(prop, material_mode_items);
+ RNA_def_property_enum_default(prop, eBooleanModifierMaterialMode_Index);
+ RNA_def_property_ui_text(prop, "Material Mode", "Method for setting materials on the new faces");
+ RNA_def_property_update(prop, 0, "rna_Modifier_update");
+
/* BMesh debugging options, only used when G_DEBUG is set */
/* BMesh intersection options */
diff --git a/source/blender/modifiers/intern/MOD_boolean.cc b/source/blender/modifiers/intern/MOD_boolean.cc
index 21f05158e8b..178be2d8962 100644
--- a/source/blender/modifiers/intern/MOD_boolean.cc
+++ b/source/blender/modifiers/intern/MOD_boolean.cc
@@ -377,21 +377,31 @@ static void BMD_mesh_intersection(BMesh *bm,
#ifdef WITH_GMP
+/* Get a mapping from material slot numbers in the src_ob to slot numbers in the dst_ob.
+ * If a material doesn't exist in the dst_ob, the mapping just goes to the same slot
+ * or to zero if there aren't enough slots in the destination. */
+static Array<short> get_material_remap_index_based(Object *dest_ob, Object *src_ob)
+{
+ int n = src_ob->totcol;
+ if (n <= 0) {
+ n = 1;
+ }
+ Array<short> remap(n);
+ BKE_object_material_remap_calc(dest_ob, src_ob, remap.data());
+ return remap;
+}
+
/* Get a mapping from material slot numbers in the source geometry to slot numbers in the result
* geometry. The material is added to the result geometry if it doesn't already use it. */
-static Array<short> get_material_remap(Object &object,
- const Mesh &mesh,
- VectorSet<Material *> &materials)
+static Array<short> get_material_remap_transfer(Object &object,
+ const Mesh &mesh,
+ VectorSet<Material *> &materials)
{
const int material_num = mesh.totcol;
- if (material_num == 0) {
- /* Necessary for faces using the default material when there are no material slots. */
- return Array<short>({materials.index_of_or_add(nullptr)});
- }
Array<short> map(material_num);
for (const int i : IndexRange(material_num)) {
Material *material = BKE_object_material_get_eval(&object, i + 1);
- map[i] = materials.index_of_or_add(material);
+ map[i] = material ? materials.index_of_or_add(material) : -1;
}
return map;
}
@@ -403,7 +413,6 @@ static Mesh *exact_boolean_mesh(BooleanModifierData *bmd,
Vector<const Mesh *> meshes;
Vector<float4x4 *> obmats;
- VectorSet<Material *> materials;
Vector<Array<short>> material_remaps;
# ifdef DEBUG_TIME
@@ -417,12 +426,18 @@ static Mesh *exact_boolean_mesh(BooleanModifierData *bmd,
meshes.append(mesh);
obmats.append((float4x4 *)&ctx->object->object_to_world);
material_remaps.append({});
- if (mesh->totcol == 0) {
- /* Necessary for faces using the default material when there are no material slots. */
- materials.add(nullptr);
- }
- else {
- materials.add_multiple({mesh->mat, mesh->totcol});
+
+ const BooleanModifierMaterialMode material_mode = BooleanModifierMaterialMode(
+ bmd->material_mode);
+ VectorSet<Material *> materials;
+ if (material_mode == eBooleanModifierMaterialMode_Transfer) {
+ if (mesh->totcol == 0) {
+ /* Necessary for faces using the default material when there are no material slots. */
+ materials.add(nullptr);
+ }
+ else {
+ materials.add_multiple({mesh->mat, mesh->totcol});
+ }
}
if (bmd->flag & eBooleanModifierFlag_Object) {
@@ -433,7 +448,12 @@ static Mesh *exact_boolean_mesh(BooleanModifierData *bmd,
BKE_mesh_wrapper_ensure_mdata(mesh_operand);
meshes.append(mesh_operand);
obmats.append((float4x4 *)&bmd->object->object_to_world);
- material_remaps.append(get_material_remap(*bmd->object, *mesh_operand, materials));
+ if (material_mode == eBooleanModifierMaterialMode_Index) {
+ material_remaps.append(get_material_remap_index_based(ctx->object, bmd->object));
+ }
+ else {
+ material_remaps.append(get_material_remap_transfer(*bmd->object, *mesh_operand, materials));
+ }
}
else if (bmd->flag & eBooleanModifierFlag_Collection) {
Collection *collection = bmd->collection;
@@ -448,7 +468,12 @@ static Mesh *exact_boolean_mesh(BooleanModifierData *bmd,
BKE_mesh_wrapper_ensure_mdata(collection_mesh);
meshes.append(collection_mesh);
obmats.append((float4x4 *)&ob->object_to_world);
- material_remaps.append(get_material_remap(*ob, *collection_mesh, materials));
+ if (material_mode == eBooleanModifierMaterialMode_Index) {
+ material_remaps.append(get_material_remap_index_based(ctx->object, ob));
+ }
+ else {
+ material_remaps.append(get_material_remap_transfer(*ob, *collection_mesh, materials));
+ }
}
}
FOREACH_COLLECTION_OBJECT_RECURSIVE_END;
@@ -466,10 +491,14 @@ static Mesh *exact_boolean_mesh(BooleanModifierData *bmd,
hole_tolerant,
bmd->operation,
nullptr);
- MEM_SAFE_FREE(result->mat);
- result->mat = (Material **)MEM_malloc_arrayN(materials.size(), sizeof(Material *), __func__);
- result->totcol = materials.size();
- MutableSpan(result->mat, result->totcol).copy_from(materials);
+
+ if (material_mode == eBooleanModifierMaterialMode_Transfer) {
+ MEM_SAFE_FREE(result->mat);
+ result->mat = (Material **)MEM_malloc_arrayN(materials.size(), sizeof(Material *), __func__);
+ result->totcol = materials.size();
+ MutableSpan(result->mat, result->totcol).copy_from(materials);
+ }
+
return result;
}
#endif
@@ -610,6 +639,7 @@ static void solver_options_panel_draw(const bContext * /*C*/, Panel *panel)
uiLayout *col = uiLayoutColumn(layout, true);
if (use_exact) {
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list