Sculpt dyntopo: another attempt to fix undo memory corruption bug

This commit is contained in:
Joseph Eagar 2021-07-11 13:14:24 -04:00
parent be9017c349
commit 4674795591
7 changed files with 206 additions and 16 deletions

View File

@ -17,7 +17,7 @@
# ##### END GPL LICENSE BLOCK #####
# <pep8 compliant>
from bpy.types import Menu, Panel, UIList
from bpy.types import Menu, Panel, UIList, WindowManager
from bl_ui.properties_grease_pencil_common import (
GreasePencilSculptOptionsPanel,
GreasePencilDisplayPanel,
@ -1015,7 +1015,7 @@ class VIEW3D_PT_sculpt_symmetry(Panel, View3DPaintPanel):
layout.prop(sculpt, "symmetrize_direction")
layout.operator("sculpt.symmetrize")
layout.prop(bpy.types.WindowManager.operator_properties_last("sculpt.symmetrize"), "merge_tolerance")
layout.prop(WindowManager.operator_properties_last("sculpt.symmetrize"), "merge_tolerance")
class VIEW3D_PT_sculpt_symmetry_for_topbar(Panel):

View File

@ -199,7 +199,8 @@ typedef enum {
PBVH_UpdateColor = 1 << 14,
PBVH_Delete = 1 << 15,
PBVH_UpdateCurvatureDir = 1 << 16,
PBVH_UpdateTris = 1 << 17
PBVH_UpdateTris = 1 << 17,
PBVH_RebuildNodeVerts = 1 << 18
} PBVHNodeFlags;
typedef struct PBVHFrustumPlanes {
@ -221,6 +222,7 @@ typedef void (*BKE_pbvh_HitOccludedCallback)(PBVHNode *node, void *data, float *
typedef void (*BKE_pbvh_SearchNearestCallback)(PBVHNode *node, void *data, float *tmin);
void BKE_pbvh_get_nodes(PBVH *pbvh, int flag, PBVHNode ***r_array, int *r_totnode);
PBVHNode *BKE_pbvh_get_node(PBVH *pbvh, int node);
/* Building */
@ -465,6 +467,9 @@ struct TableGSet *BKE_pbvh_bmesh_node_unique_verts(PBVHNode *node);
struct TableGSet *BKE_pbvh_bmesh_node_other_verts(PBVHNode *node);
struct TableGSet *BKE_pbvh_bmesh_node_faces(PBVHNode *node);
void BKE_pbvh_bmesh_regen_node_verts(PBVH *pbvh);
void BKE_pbvh_bmesh_mark_node_regen(PBVH *pbvh, PBVHNode *node);
// now generated PBVHTris
void BKE_pbvh_bmesh_after_stroke(PBVH *pbvh);

View File

@ -3826,3 +3826,8 @@ void SCULPT_update_flat_vcol_shading(Object *ob, Scene *scene)
BKE_pbvh_set_flat_vcol_shading(ob->sculpt->pbvh, flat_vcol_shading);
}
}
PBVHNode *BKE_pbvh_get_node(PBVH *pbvh, int node)
{
return pbvh->nodes + node;
}

View File

@ -122,6 +122,46 @@ static void check_heap()
/* Support for only operating on front-faces */
#define USE_EDGEQUEUE_FRONTFACE
static void check_nodes(PBVH *pbvh)
{
#if 0
for (int i = 0; i < pbvh->totnode; i++) {
PBVHNode *node = pbvh->nodes + i;
BMVert *v;
BMFace *f;
if (!(node->flag & PBVH_Leaf)) {
continue;
}
TGSET_ITER (v, node->bm_unique_verts) {
if (v->head.htype > 8) {
printf("corruption in pbvh! bm_unique_verts\n");
}
}
TGSET_ITER_END;
TGSET_ITER (v, node->bm_other_verts) {
if (v->head.htype > 8) {
printf("corruption in pbvh! bm_other_verts\n");
}
}
TGSET_ITER_END;
TGSET_ITER (f, node->bm_faces) {
if (f->head.htype > 8) {
printf("corruption in pbvh! bm_faces\n");
}
}
TGSET_ITER_END;
}
#endif
}
void pbvh_bmesh_check_nodes(PBVH *pbvh)
{
check_nodes(pbvh);
}
/**
* Ensure we don't have dirty tags for the edge queue, and that they are left cleared.
* (slow, even for debug mode, so leave disabled for now).
@ -538,6 +578,8 @@ static bool pbvh_bmesh_node_limit_ensure(PBVH *pbvh, int node_index)
TableGSet *bm_faces = pbvh->nodes[node_index].bm_faces;
const int bm_faces_size = BLI_table_gset_len(bm_faces);
check_nodes(pbvh);
if (bm_faces_size <= pbvh->leaf_limit) {
/* Node limit not exceeded */
return false;
@ -582,6 +624,8 @@ static bool pbvh_bmesh_node_limit_ensure(PBVH *pbvh, int node_index)
MEM_freeN(bbc_array);
check_nodes(pbvh);
return true;
}
@ -2790,6 +2834,66 @@ static void pbvh_bmesh_collapse_edge(PBVH *pbvh,
BLI_array_free(ls);
}
static void pbvh_bmesh_regen_node_verts(PBVH *pbvh, PBVHNode *node)
{
int usize = BLI_table_gset_len(node->bm_other_verts);
int osize = BLI_table_gset_len(node->bm_other_verts);
BLI_table_gset_free(node->bm_other_verts, NULL);
BLI_table_gset_free(node->bm_unique_verts, NULL);
node->bm_unique_verts = BLI_table_gset_new("bm_unique_verts");
node->bm_other_verts = BLI_table_gset_new("bm_other_verts");
const int cd_vert_node = pbvh->cd_vert_node_offset;
const int ni = (int)(node - pbvh->nodes);
BMFace *f;
TGSET_ITER (f, node->bm_faces) {
BMLoop *l = f->l_first;
do {
int ni2 = BM_ELEM_CD_GET_INT(l->v, cd_vert_node);
if (ni2 == ni) {
BLI_table_gset_add(node->bm_unique_verts, l->v);
}
else {
BLI_table_gset_add(node->bm_other_verts, l->v);
}
} while ((l = l->next) != f->l_first);
}
TGSET_ITER_END;
if (usize != BLI_table_gset_len(node->bm_unique_verts)) {
printf("possible pbvh error: bm_unique_verts might have had bad data. old: %d, new: %d\n",
usize,
BLI_table_gset_len(node->bm_unique_verts));
}
if (osize != BLI_table_gset_len(node->bm_other_verts)) {
printf("possible pbvh error: bm_other_verts might have had bad data. old: %d, new: %d\n",
osize,
BLI_table_gset_len(node->bm_other_verts));
}
}
void BKE_pbvh_bmesh_mark_node_regen(PBVH *pbvh, PBVHNode *node)
{
node->flag |= PBVH_RebuildNodeVerts;
}
void BKE_pbvh_bmesh_regen_node_verts(PBVH *pbvh)
{
for (int i = 0; i < pbvh->totnode; i++) {
PBVHNode *node = pbvh->nodes + i;
if (!(node->flag & PBVH_Leaf) || !(node->flag & PBVH_RebuildNodeVerts)) {
continue;
}
pbvh_bmesh_regen_node_verts(pbvh, node);
}
}
void BKE_pbvh_bmesh_update_origvert(
PBVH *pbvh, BMVert *v, float **r_co, float **r_no, float **r_color, bool log_undo)
{
@ -3723,6 +3827,13 @@ CLANG_OPT_BUG static bool cleanup_valence_3_4(PBVH *pbvh,
continue;
}
int ni = BM_ELEM_CD_GET_INT(v, pbvh->cd_vert_node_offset);
if (ni >= 0 && BLI_table_gset_haskey(pbvh->nodes[ni].bm_other_verts, v)) {
printf("error!\n");
BLI_table_gset_remove(pbvh->nodes[ni].bm_other_verts, v, NULL);
}
BM_log_vert_removed(pbvh->bm_log, v, pbvh->cd_vert_mask_offset);
pbvh_bmesh_vert_remove(pbvh, v);
@ -3891,7 +4002,9 @@ bool BKE_pbvh_bmesh_update_topology(PBVH *pbvh,
int max_steps = (int)((float)DYNTOPO_MAX_ITER * ratio);
check_nodes(pbvh);
modified |= pbvh_bmesh_collapse_short_edges(&eq_ctx, pbvh, &deleted_faces, max_steps);
check_nodes(pbvh);
BLI_heapsimple_free(q.heap, NULL);
if (q.elems) {
@ -3931,7 +4044,9 @@ bool BKE_pbvh_bmesh_update_topology(PBVH *pbvh,
int max_steps = (int)((float)DYNTOPO_MAX_ITER * ratio);
check_nodes(pbvh);
modified |= pbvh_bmesh_subdivide_long_edges(&eq_ctx, pbvh, &edge_loops, max_steps);
check_nodes(pbvh);
if (q.elems) {
MEM_freeN(q.elems);
@ -3942,8 +4057,10 @@ bool BKE_pbvh_bmesh_update_topology(PBVH *pbvh,
#endif
if (mode & PBVH_Cleanup) {
check_nodes(pbvh);
modified |= cleanup_valence_3_4(
pbvh, center, view_normal, radius, use_frontface, use_projected);
check_nodes(pbvh);
}
if (modified) {
@ -4686,7 +4803,9 @@ void BKE_pbvh_bmesh_after_stroke(PBVH *pbvh)
check_heap();
int totnode = pbvh->totnode;
check_nodes(pbvh);
pbvh_bmesh_join_nodes(pbvh);
check_nodes(pbvh);
check_heap();

View File

@ -827,6 +827,8 @@ void *mempool_iter_threadsafe_step(BLI_mempool_threadsafe_iter *ts_iter)
do {
ret = curnode;
BLI_asan_unpoison(ret, esize);
if (++iter->curindex != iter->pool->pchunk) {
curnode = POINTER_OFFSET(curnode, esize);
}
@ -842,29 +844,37 @@ void *mempool_iter_threadsafe_step(BLI_mempool_threadsafe_iter *ts_iter)
/* pass. */
}
if (UNLIKELY(iter->curchunk == NULL)) {
return (ret->freeword == FREEWORD) ? NULL : ret;
if (ret->freeword == FREEWORD) {
BLI_asan_poison(ret, esize);
return NULL;
}
else {
return ret;
}
}
/* End `threadsafe` exception. */
iter->curchunk = iter->curchunk->next;
if (UNLIKELY(iter->curchunk == NULL)) {
BLI_asan_unpoison(ret, iter->pool->esize);
void *ret2 = (ret->freeword == FREEWORD) ? NULL : ret;
if (ret->freeword == FREEWORD) {
BLI_asan_poison(ret, iter->pool->esize);
return NULL;
}
else {
return ret;
}
return ret2;
}
curnode = CHUNK_DATA(iter->curchunk);
}
if (ret->freeword == FREEWORD) {
BLI_asan_poison(ret, iter->pool->esize);
}
} while (ret->freeword == FREEWORD);
else {
break;
}
} while (true);
return ret;
}

View File

@ -409,6 +409,22 @@ static BMLogFace *bm_log_face_alloc(BMLog *log, BMFace *f)
/************************ Helpers for undo/redo ***********************/
// exec vert kill callbacks before killing faces
static void bm_log_verts_unmake_pre(
BMesh *bm, BMLog *log, GHash *verts, BMLogEntry *entry, BMLogCallbacks *callbacks)
{
GHashIterator gh_iter;
GHASH_ITER (gh_iter, verts) {
void *key = BLI_ghashIterator_getKey(&gh_iter);
uint id = POINTER_AS_UINT(key);
BMVert *v = bm_log_vert_from_id(log, id);
if (callbacks) {
callbacks->on_vert_kill(v, callbacks->userdata);
}
}
}
static void bm_log_verts_unmake(
BMesh *bm, BMLog *log, GHash *verts, BMLogEntry *entry, BMLogCallbacks *callbacks)
{
@ -425,10 +441,6 @@ static void bm_log_verts_unmake(
* deleting it */
bm_log_vert_bmvert_copy(log, entry, lv, v, cd_vert_mask_offset, true);
if (callbacks) {
callbacks->on_vert_kill(v, callbacks->userdata);
}
BM_vert_kill(bm, v);
}
}
@ -1258,6 +1270,7 @@ static void bm_log_undo_intern(
}
/* Delete added faces and verts */
bm_log_verts_unmake_pre(bm, log, entry->added_verts, entry, callbacks);
bm_log_faces_unmake(bm, log, entry->added_faces, entry, callbacks);
bm_log_verts_unmake(bm, log, entry->added_verts, entry, callbacks);
@ -1309,6 +1322,7 @@ static void bm_log_redo_intern(
bm->elem_table_dirty |= BM_VERT | BM_EDGE | BM_FACE;
/* Re-delete previously deleted faces and verts */
bm_log_verts_unmake_pre(bm, log, entry->deleted_verts, entry, callbacks);
bm_log_faces_unmake(bm, log, entry->deleted_faces, entry, callbacks);
bm_log_verts_unmake(bm, log, entry->deleted_verts, entry, callbacks);

View File

@ -174,6 +174,8 @@ static bool test_swap_v3_v3(float a[3], float b[3])
return false;
}
void pbvh_bmesh_check_nodes(PBVH *pbvh);
static bool sculpt_undo_restore_deformed(
const SculptSession *ss, SculptUndoNode *unode, int uindex, int oindex, float coord[3])
{
@ -429,8 +431,28 @@ static void bmesh_undo_on_vert_kill(BMVert *v, void *userdata)
BmeshUndoData *data = (BmeshUndoData *)userdata;
// data->do_full_recalc = true;
BKE_pbvh_bmesh_remove_vertex(data->pbvh, v, false);
if (BM_ELEM_CD_GET_INT(v, data->cd_vert_node_offset) < 0) {
// something went wrong
printf("pbvh bmesh undo error\n");
data->do_full_recalc = true;
return;
}
else {
int ni = BM_ELEM_CD_GET_INT(v, data->cd_vert_node_offset);
/*
regenerate bm_unique_verts, which can end up with
freed verts for some reason. I've run this through
ASAN and fixed one likely cause, but it still happens.
- joeedh
*/
if (ni >= 0) {
PBVHNode *node = BKE_pbvh_get_node(data->pbvh, ni);
BKE_pbvh_bmesh_mark_node_regen(data->pbvh, node);
}
}
BKE_pbvh_bmesh_remove_vertex(data->pbvh, v, false);
data->balance_pbvh = true;
}
static void bmesh_undo_on_vert_add(BMVert *v, void *userdata)
@ -444,6 +466,18 @@ static void bmesh_undo_on_vert_add(BMVert *v, void *userdata)
static void bmesh_undo_on_face_kill(BMFace *f, void *userdata)
{
BmeshUndoData *data = (BmeshUndoData *)userdata;
int ni = BM_ELEM_CD_GET_INT(f, data->cd_face_node_offset);
/*
regenerate bm_unique_verts, which can end up with
freed verts for some reason. I've run this through
ASAN and fixed one likely cause, but it still happens.
- joeedh
*/
if (ni >= 0) {
PBVHNode *node = BKE_pbvh_get_node(data->pbvh, ni);
BKE_pbvh_bmesh_mark_node_regen(data->pbvh, node);
}
BKE_pbvh_bmesh_remove_face(data->pbvh, f, false);
@ -546,6 +580,9 @@ static void sculpt_undo_bmesh_restore_generic(SculptUndoNode *unode, Object *ob,
unode->applied = true;
}
BKE_pbvh_bmesh_regen_node_verts(ss->pbvh);
pbvh_bmesh_check_nodes(ss->pbvh);
if (!data.do_full_recalc || unode->type == SCULPT_UNDO_MASK ||
unode->type == SCULPT_UNDO_COLOR) {
int totnode;