griefith/test
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix #142633: Crash calling bmesh.ops.split from Python

Browse Source 
The bmesh.ops.split operator could include deleted edges as the keys
for `boundary_map`.

Resolve by replacing deleted edges with so the same edge is used for
the key & value, needed so the boundary_map can access the entire
boundary, even when it's source edges have been deleted.
This commit is contained in:
Campbell Barton
2025-07-23 09:30:48 +00:00
parent 2e48d33101
commit 50f60dbe2d
6 changed files with 109 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
source/blender/bmesh/intern/bmesh_delete.cc
View File

@@ -81,7 +81,10 @@ void BMO_mesh_delete_oflag_tagged(BMesh *bm, const short oflag, const char htype
}
}
void BMO_mesh_delete_oflag_context(BMesh *bm, const short oflag, const int type)
void BMO_mesh_delete_oflag_context(BMesh *bm,
const short oflag,
const int type,
blender::FunctionRef<void()> prepare_fn)
{
BMEdge *e;
@@ -90,8 +93,10 @@ void BMO_mesh_delete_oflag_context(BMesh *bm, const short oflag, const int type)
switch (type) {
case DEL_VERTS: {
if (prepare_fn) {
prepare_fn();
}
bmo_remove_tagged_verts(bm, oflag);
break;
}
case DEL_EDGES: {
@@ -102,24 +107,32 @@ void BMO_mesh_delete_oflag_context(BMesh *bm, const short oflag, const int type)
BMO_vert_flag_enable(bm, e->v2, oflag);
}
}
if (prepare_fn) {
prepare_fn();
}
bmo_remove_tagged_edges(bm, oflag);
bmo_remove_tagged_verts_loose(bm, oflag);
break;
}
case DEL_EDGESFACES: {
if (prepare_fn) {
prepare_fn();
}
bmo_remove_tagged_edges(bm, oflag);
break;
}
case DEL_ONLYFACES: {
if (prepare_fn) {
prepare_fn();
}
bmo_remove_tagged_faces(bm, oflag);
break;
}
case DEL_ONLYTAGGED: {
if (prepare_fn) {
prepare_fn();
}
BMO_mesh_delete_oflag_tagged(bm, oflag, BM_ALL_NOLOOP);
break;
}
case DEL_FACES:
@@ -150,23 +163,40 @@ void BMO_mesh_delete_oflag_context(BMesh *bm, const short oflag, const int type)
BMO_edge_flag_disable(bm, l_iter->e, oflag);
} while ((l_iter = l_iter->next) != l_first);
}
}
/* also mark all the vertices of remaining edges for keeping */
BM_ITER_MESH (e, &eiter, bm, BM_EDGES_OF_MESH) {
/* now go through and mark all remaining faces all edges for keeping */
BM_ITER_MESH (f, &fiter, bm, BM_FACES_OF_MESH) {
if (!BMO_face_flag_test(bm, f, oflag)) {
BMLoop *l_first = BM_FACE_FIRST_LOOP(f);
BMLoop *l_iter;
/* Only exception to normal 'DEL_FACES' logic. */
if (type == DEL_FACES_KEEP_BOUNDARY) {
if (BM_edge_is_boundary(e)) {
BMO_edge_flag_disable(bm, e, oflag);
l_iter = l_first;
do {
BMO_vert_flag_disable(bm, l_iter->v, oflag);
BMO_edge_flag_disable(bm, l_iter->e, oflag);
} while ((l_iter = l_iter->next) != l_first);
}
}
/* also mark all the vertices of remaining edges for keeping */
BM_ITER_MESH (e, &eiter, bm, BM_EDGES_OF_MESH) {
if (!BMO_edge_flag_test(bm, e, oflag)) {
BMO_vert_flag_disable(bm, e->v1, oflag);
BMO_vert_flag_disable(bm, e->v2, oflag);
/* Only exception to normal 'DEL_FACES' logic. */
if (type == DEL_FACES_KEEP_BOUNDARY) {
if (BM_edge_is_boundary(e)) {
BMO_edge_flag_disable(bm, e, oflag);
}
}
if (!BMO_edge_flag_test(bm, e, oflag)) {
BMO_vert_flag_disable(bm, e->v1, oflag);
BMO_vert_flag_disable(bm, e->v2, oflag);
}
}
}
if (prepare_fn) {
prepare_fn();
}
/* now delete marked face */
bmo_remove_tagged_faces(bm, oflag);
/* delete marked edge */

8
source/blender/bmesh/intern/bmesh_delete.hh
View File

@@ -10,6 +10,8 @@
#include "bmesh_class.hh"
#include "BLI_function_ref.hh"
void BMO_mesh_delete_oflag_tagged(BMesh *bm, short oflag, char htype);
void BM_mesh_delete_hflag_tagged(BMesh *bm, char hflag, char htype);
@@ -17,7 +19,11 @@ void BM_mesh_delete_hflag_tagged(BMesh *bm, char hflag, char htype);
* \warning oflag applies to different types in some contexts,
* not just the type being removed.
*/
void BMO_mesh_delete_oflag_context(BMesh *bm, short oflag, int type);
void BMO_mesh_delete_oflag_context(BMesh *bm,
short oflag,
int type,
blender::FunctionRef<void()> prepare_fn);
/**
* \warning oflag applies to different types in some contexts,
* not just the type being removed.

7
source/blender/bmesh/intern/bmesh_opdefines.cc
View File

@@ -1828,6 +1828,8 @@ static BMOpDefine bmo_duplicate_def = {
{"face_map.out",
BMO_OP_SLOT_MAPPING,
{eBMOpSlotSubType_Elem(BMO_OP_SLOT_SUBTYPE_MAP_ELEM)}},
/* Boundary edges from the split geometry that maps edges from the original geometry
* to the destination edges. */
{"boundary_map.out",
BMO_OP_SLOT_MAPPING,
{eBMOpSlotSubType_Elem(BMO_OP_SLOT_SUBTYPE_MAP_ELEM)}},
@@ -1862,6 +1864,11 @@ static BMOpDefine bmo_split_def = {
/*slot_types_out*/
{
{"geom.out", BMO_OP_SLOT_ELEMENT_BUF, {BM_VERT | BM_EDGE | BM_FACE}},
/* Boundary edges from the split geometry that maps edges from the original geometry
* to the destination edges.
*
* When the source edges have been deleted, the destination edge will be used
* for both the key and the value. */
{"boundary_map.out",
BMO_OP_SLOT_MAPPING,
{eBMOpSlotSubType_Elem(BMO_OP_SLOT_SUBTYPE_MAP_ELEM)}},

36
source/blender/bmesh/operators/bmo_dupe.cc
View File

@@ -489,14 +489,38 @@ void bmo_split_exec(BMesh *bm, BMOperator *op)
}
}
/* connect outputs of dupe to delete, excluding keep geometry */
BMO_mesh_delete_oflag_context(bm, SPLIT_INPUT, DEL_FACES);
/* now we make our outputs by copying the dupe output */
BMO_slot_copy(&dupeop, slots_out, "geom.out", splitop, slots_out, "geom.out");
BMO_slot_copy(&dupeop, slots_out, "boundary_map.out", splitop, slots_out, "boundary_map.out");
BMO_slot_copy(&dupeop, slots_out, "isovert_map.out", splitop, slots_out, "isovert_map.out");
/* connect outputs of dupe to delete, excluding keep geometry */
BMO_mesh_delete_oflag_context(
bm,
SPLIT_INPUT,
DEL_FACES,
/* Call before deletion so deleted geometry isn't copied. */
[&bm, &dupeop, &splitop]() {
/* Now we make our outputs by copying the dupe output. */
/* NOTE: `boundary_map.out` can't use #BMO_slot_copy` because some of the "source"
* geometry has been removed. In this case the (source -> destination) map doesn't work.
* In this case there is isn't an especially good option.
* The geometry needs to be included so the boundary is accessible.
* Use the "destination" as the key and the value since it avoids adding freed
* geometry into the map and can be easily detected by other operators.
* See: #142633. */
const char *slot_name_boundary_map = "boundary_map.out";
BMOpSlot *splitop_boundary_map = BMO_slot_get(splitop->slots_out, slot_name_boundary_map);
BMOIter siter;
BMElem *ele_key;
BMO_ITER (ele_key, &siter, dupeop.slots_out, slot_name_boundary_map, 0) {
BMElem *ele_val = static_cast<BMElem *>(BMO_iter_map_value_ptr(&siter));
if (BMO_elem_flag_test(bm, ele_key, SPLIT_INPUT)) {
ele_key = ele_val;
}
BMO_slot_map_elem_insert(splitop, splitop_boundary_map, ele_key, ele_val);
}
});
/* cleanup */
BMO_op_finish(bm, &dupeop);
@@ -512,7 +536,7 @@ void bmo_delete_exec(BMesh *bm, BMOperator *op)
/* Mark Buffer */
BMO_slot_buffer_flag_enable(bm, delop->slots_in, "geom", BM_ALL_NOLOOP, DEL_INPUT);
BMO_mesh_delete_oflag_context(bm, DEL_INPUT, BMO_slot_int_get(op->slots_in, "context"));
BMO_mesh_delete_oflag_context(bm, DEL_INPUT, BMO_slot_int_get(op->slots_in, "context"), nullptr);
#undef DEL_INPUT
}

2
source/blender/bmesh/operators/bmo_removedoubles.cc
View File

@@ -311,7 +311,7 @@ void bmo_weld_verts_exec(BMesh *bm, BMOperator *op)
BLI_ghash_free(targetmap_all, nullptr, nullptr);
}
BMO_mesh_delete_oflag_context(bm, ELE_DEL, DEL_ONLYTAGGED);
BMO_mesh_delete_oflag_context(bm, ELE_DEL, DEL_ONLYTAGGED, nullptr);
}
#define VERT_KEEP 8

31
source/blender/editors/mesh/editmesh_tools.cc
View File

@@ -4813,21 +4813,26 @@ static FillGridSplitJoin *edbm_fill_grid_split_join_init(BMEditMesh *em)
BLI_assert(e_dst);
/* For edges, flip the selection from the edge of the hole to the edge of the island. */
BM_elem_flag_disable(e, BM_ELEM_SELECT);
BM_elem_flag_enable(e_dst, BM_ELEM_SELECT);
/* For verts, flip the selection from the edge of the hole to the edge of the island.
* Also add it to the weld map. But check selection first. Don't try to add the same vert to
* the map more than once. If the selection was changed false, it's already been processed. */
if (BM_elem_flag_test(e->v1, BM_ELEM_SELECT)) {
BM_elem_flag_disable(e->v1, BM_ELEM_SELECT);
BM_elem_flag_enable(e_dst->v1, BM_ELEM_SELECT);
BMO_slot_map_elem_insert(&split_join->weld_op, weld_target_map, e->v1, e_dst->v1);
}
if (BM_elem_flag_test(e->v2, BM_ELEM_SELECT)) {
BM_elem_flag_disable(e->v2, BM_ELEM_SELECT);
BM_elem_flag_enable(e_dst->v2, BM_ELEM_SELECT);
BMO_slot_map_elem_insert(&split_join->weld_op, weld_target_map, e->v2, e_dst->v2);
/* When these match, the source edge has been deleted. */
if (e != e_dst) {
BM_elem_flag_disable(e, BM_ELEM_SELECT);
/* For verts, flip the selection from the edge of the hole to the edge of the island.
* Also add it to the weld map. But check selection first. Don't try to add the same vert to
* the map more than once. If the selection was changed false, it's already been processed.
*/
if (BM_elem_flag_test(e->v1, BM_ELEM_SELECT)) {
BM_elem_flag_disable(e->v1, BM_ELEM_SELECT);
BM_elem_flag_enable(e_dst->v1, BM_ELEM_SELECT);
BMO_slot_map_elem_insert(&split_join->weld_op, weld_target_map, e->v1, e_dst->v1);
}
if (BM_elem_flag_test(e->v2, BM_ELEM_SELECT)) {
BM_elem_flag_disable(e->v2, BM_ELEM_SELECT);
BM_elem_flag_enable(e_dst->v2, BM_ELEM_SELECT);
BMO_slot_map_elem_insert(&split_join->weld_op, weld_target_map, e->v2, e_dst->v2);
}
}
}