[Bf-blender-cvs] [b8a8e4f9b18] temp_bmesh_multires: Today I ran outside screaming and an hour later came back in and made BMLog reference counted. This fixes various undo bugs caused by dyntopo needing to execute an undo push but not being able too (e.g. during file load, and I think it also happens sometimes with global undo).
Joseph Eagar
noreply at git.blender.org
Thu May 13 01:40:23 CEST 2021
Commit: b8a8e4f9b1872fca7da4adea4de8857475296453
Author: Joseph Eagar
Date: Tue May 11 21:14:45 2021 -0700
Branches: temp_bmesh_multires
https://developer.blender.org/rBb8a8e4f9b1872fca7da4adea4de8857475296453
Today I ran outside screaming and an hour later came back in and made BMLog
reference counted. This fixes various undo bugs caused by dyntopo
needing to execute an undo push but not being able too (e.g. during
file load, and I think it also happens sometimes with global undo).
A much better way of fixing this would be to add unique IDs to mesh
verts and faces, perhaps as a customdata layer.
The root problem is that BMLog assigns unique IDs to mesh elements,
which it does via a series of GHash's. I imagine this is a fairly
serious violation of the undo system's design rules, since it means
BMLogs are tied to specific instances of the bmesh in SculptSession->bm.
There were some hacks to try and get around this, but they were buggy
and needed to push the unstack stack to work, which wasn't possible in
all cases (e.g. if G_MAIN->wm.first->undo_stack is NULL, as it is during
file loading and apparently global undo).
Anyway, long story short each chain of SculptUndoNodes needs some way
to reconstruct their ID GHash's when exiting/entering the chain. The
only way I could think to do this was to make BMLog reference counted,
with BMLogEntry the users.
Like I said, having a proper system to assign unique IDs to mesh
elements would be *much* better.
===================================================================
M source/blender/blenkernel/BKE_pbvh.h
M source/blender/blenkernel/intern/paint.c
M source/blender/blenkernel/intern/pbvh.c
M source/blender/bmesh/intern/bmesh_log.c
M source/blender/bmesh/intern/bmesh_log.h
M source/blender/editors/sculpt_paint/sculpt.c
M source/blender/editors/sculpt_paint/sculpt_dyntopo.c
M source/blender/editors/sculpt_paint/sculpt_expand.c
M source/blender/editors/sculpt_paint/sculpt_face_set.c
M source/blender/editors/sculpt_paint/sculpt_intern.h
M source/blender/editors/sculpt_paint/sculpt_undo.c
===================================================================
diff --git a/source/blender/blenkernel/BKE_pbvh.h b/source/blender/blenkernel/BKE_pbvh.h
index 59818dbfb3a..5912e52b079 100644
--- a/source/blender/blenkernel/BKE_pbvh.h
+++ b/source/blender/blenkernel/BKE_pbvh.h
@@ -652,6 +652,10 @@ void pbvh_vertex_iter_init(PBVH *pbvh, PBVHNode *node, PBVHVertexIter *vi, int m
(BKE_pbvh_type(pbvh) == PBVH_BMESH && v.i != -1 ? BM_elem_index_get((BMVert *)(v.i)) : (v.i))
SculptVertRef BKE_pbvh_table_index_to_vertex(PBVH *pbvh, int idx);
+#define BKE_pbvh_face_index_to_table(pbvh, v) \
+ (BKE_pbvh_type(pbvh) == PBVH_BMESH && v.i != -1 ? BM_elem_index_get((BMFace *)(v.i)) : (v.i))
+SculptFaceRef BKE_pbvh_table_index_to_face(PBVH *pbvh, int idx);
+
void BKE_pbvh_node_get_proxies(PBVHNode *node, PBVHProxyNode **proxies, int *proxy_count);
void BKE_pbvh_node_free_proxies(PBVHNode *node);
PBVHProxyNode *BKE_pbvh_node_add_proxy(PBVH *pbvh, PBVHNode *node);
diff --git a/source/blender/blenkernel/intern/paint.c b/source/blender/blenkernel/intern/paint.c
index 9c316483c53..49fc04f7588 100644
--- a/source/blender/blenkernel/intern/paint.c
+++ b/source/blender/blenkernel/intern/paint.c
@@ -51,6 +51,7 @@
#include "BKE_context.h"
#include "BKE_crazyspace.h"
#include "BKE_deform.h"
+#include "BKE_global.h"
#include "BKE_gpencil.h"
#include "BKE_idtype.h"
#include "BKE_image.h"
@@ -67,6 +68,7 @@
#include "BKE_pbvh.h"
#include "BKE_subdiv_ccg.h"
#include "BKE_subsurf.h"
+#include "BKE_undo_system.h"
#include "DEG_depsgraph.h"
#include "DEG_depsgraph_query.h"
@@ -79,6 +81,7 @@
// XXX todo: work our bad module cross ref
void SCULPT_dynamic_topology_sync_layers(Object *ob, Mesh *me);
+void SCULPT_on_sculptsession_bmesh_free(SculptSession *ss);
static void palette_init_data(ID *id)
{
@@ -1380,12 +1383,6 @@ static void sculptsession_bm_to_me_update_data_only(Object *ob, bool reorder)
if (ss->bm) {
if (ob->data) {
- BMIter iter;
- BMFace *efa;
- BM_ITER_MESH (efa, &iter, ss->bm, BM_FACES_OF_MESH) {
- BM_elem_flag_set(efa, BM_ELEM_SMOOTH, ss->bm_smooth_shading);
- }
-
if (reorder) {
BM_log_mesh_elems_reorder(ss->bm, ss->bm_log);
}
@@ -1473,7 +1470,14 @@ void BKE_sculptsession_free(Object *ob)
if (ob && ob->sculpt) {
SculptSession *ss = ob->sculpt;
+ if (ss->bm_log) {
+ BM_log_free(ss->bm_log, true);
+ }
+
+ /*try to save current mesh*/
if (ss->bm) {
+ SCULPT_on_sculptsession_bmesh_free(ss);
+
BKE_sculptsession_bm_to_me(ob, true);
BM_mesh_free(ss->bm);
}
@@ -1489,10 +1493,6 @@ void BKE_sculptsession_free(Object *ob)
MEM_SAFE_FREE(ss->vemap);
MEM_SAFE_FREE(ss->vemap_mem);
- if (ss->bm_log) {
- BM_log_free(ss->bm_log);
- }
-
MEM_SAFE_FREE(ss->texcache);
if (ss->tex_pool) {
@@ -1911,7 +1911,7 @@ void BKE_sculpt_toolsettings_data_ensure(struct Scene *scene)
if (!sd->detail_range) {
sd->detail_range = 0.4f;
- sd->flags |= SCULPT_DYNTOPO_CLEANUP; //should really do this in do_versions_290.c
+ sd->flags |= SCULPT_DYNTOPO_CLEANUP; // should really do this in do_versions_290.c
}
if (!sd->detail_percent) {
diff --git a/source/blender/blenkernel/intern/pbvh.c b/source/blender/blenkernel/intern/pbvh.c
index 36a236b07c6..33c0894f1d3 100644
--- a/source/blender/blenkernel/intern/pbvh.c
+++ b/source/blender/blenkernel/intern/pbvh.c
@@ -3174,6 +3174,17 @@ SculptVertRef BKE_pbvh_table_index_to_vertex(PBVH *pbvh, int idx)
return BKE_pbvh_make_vref(idx);
}
+
+SculptFaceRef BKE_pbvh_table_index_to_face(PBVH *pbvh, int idx)
+{
+ if (pbvh->type == PBVH_BMESH) {
+ SculptFaceRef ref = {(intptr_t)pbvh->bm->ftable[idx]};
+ return ref;
+ }
+
+ return BKE_pbvh_make_fref(idx);
+}
+
bool pbvh_has_face_sets(PBVH *pbvh)
{
switch (pbvh->type) {
diff --git a/source/blender/bmesh/intern/bmesh_log.c b/source/blender/bmesh/intern/bmesh_log.c
index 6ce62008d3a..8a37212f9d6 100644
--- a/source/blender/bmesh/intern/bmesh_log.c
+++ b/source/blender/bmesh/intern/bmesh_log.c
@@ -108,6 +108,10 @@ struct BMLog {
/* Tree of free IDs */
struct RangeTreeUInt *unused_ids;
+ BMLogEntry *frozen_full_mesh;
+
+ int refcount;
+
/* Mapping from unique IDs to vertices and faces
*
* Each vertex and face in the log gets a unique uinteger
@@ -161,6 +165,9 @@ typedef struct {
#define logkey_hash BLI_ghashutil_inthash_p_simple
#define logkey_cmp BLI_ghashutil_intcmp
+static void full_copy_swap(BMesh *bm, BMLog *log, BMLogEntry *entry);
+static void bm_log_entry_free(BMLogEntry *entry);
+
static void *log_ghash_lookup(BMLog *log, GHash *gh, const void *key)
{
BLI_rw_mutex_lock(&log->lock, THREAD_LOCK_READ);
@@ -665,6 +672,15 @@ static BMLogEntry *bm_log_entry_create(void)
* Note: does not free the log entry itself */
static void bm_log_entry_free(BMLogEntry *entry)
{
+ if (entry->log) {
+ entry->log->refcount--;
+
+ if (entry->log->refcount < 0) {
+ fprintf(stderr, "BMLog refcount error\n");
+ entry->log->refcount = 0;
+ }
+ }
+
if (entry->full_copy_mesh) {
BKE_mesh_free(entry->full_copy_mesh);
@@ -855,11 +871,41 @@ BMLog *BM_log_from_existing_entries_create(BMesh *bm, BMLogEntry *entry)
return log;
}
-/* Free all the data in a BMLog including the log itself */
-void BM_log_free(BMLog *log)
+ATTR_NO_OPT BMLog *BM_log_unfreeze(BMesh *bm, BMLogEntry *entry)
+{
+ if (!entry || !entry->log) {
+ return NULL;
+ }
+
+ if (!entry->log->frozen_full_mesh) {
+ return entry->log;
+ }
+
+ full_copy_swap(bm, entry->log, entry->log->frozen_full_mesh);
+
+ entry->log->frozen_full_mesh->log = NULL;
+ bm_log_entry_free(entry->log->frozen_full_mesh);
+ entry->log->frozen_full_mesh = NULL;
+
+ return entry->log;
+}
+
+/* Free all the data in a BMLog including the log itself
+ * safe_mode means log->refcount will be checked, and if nonzero log will not be freed
+ */
+void BM_log_free(BMLog *log, bool safe_mode)
{
BMLogEntry *entry;
+ if (safe_mode && log->refcount) {
+ if (!log->frozen_full_mesh) {
+ log->frozen_full_mesh = bm_log_entry_create();
+ bm_log_full_mesh_intern(log->bm, log, log->frozen_full_mesh);
+ }
+
+ return;
+ }
+
BLI_rw_mutex_end(&log->lock);
if (log->unused_ids) {
@@ -1010,6 +1056,8 @@ BMLogEntry *BM_log_entry_add_ex(BMesh *bm, BMLog *log, bool combine_with_last)
BLI_addtail(&log->entries, entry);
entry->log = log;
+ log->refcount++;
+
#ifdef CUSTOMDATA
if (combine_with_last) {
if (log->current_entry) {
@@ -1162,7 +1210,7 @@ static void full_copy_swap(BMesh *bm, BMLog *log, BMLogEntry *entry)
else {
// eek, error!
printf("bmlog error!\n");
- log_ghash_remove(log, log->id_to_elem, (void*)id, NULL, NULL);
+ log_ghash_remove(log, log->id_to_elem, (void *)id, NULL, NULL);
}
}
@@ -1474,6 +1522,10 @@ void BM_log_face_removed(BMLog *log, BMFace *f)
/* Log all vertices/faces in the BMesh as added */
void BM_log_all_added(BMesh *bm, BMLog *log)
{
+ if (!log->current_entry) {
+ BM_log_entry_add_ex(bm, log, false);
+ }
+
const int cd_vert_mask_offset = CustomData_get_offset(&bm->vdata, CD_PAINT_MASK);
BMIter bm_iter;
BMVert *v;
@@ -1524,6 +1576,10 @@ void BM_log_full_mesh(BMesh *bm, BMLog *log)
/* Log all vertices/faces in the BMesh as removed */
void BM_log_before_all_removed(BMesh *bm, BMLog *log)
{
+ if (!log->current_entry) {
+ BM_log_entry_add_ex(bm, log, false);
+ }
+
const int cd_vert_mask_offset = CustomData_get_offset(&bm->vdata, CD_PAINT_MASK);
BMIter bm_iter;
BMVert *v;
diff --git a/source/blender/bmesh/intern/bmesh_log.h b/source/blender/bmesh/intern/bmesh_log.h
index 2b8b4fa6675..199b6a21ff4 100644
--- a/source/blender/bmesh/intern/bmesh_log.h
+++ b/source/blender/bmesh/intern/bmesh_log.h
@@ -36,9 +36,11 @@ void BM_log_set_cd_offsets(BMLog *log, int cd_dyn_vert);
BMLog *BM_log_from_existing_entries_create(BMesh *bm, BMLogEntry *entry);
/* Free all the data in a BMLog including the log itself */
-void BM_log_free(BMLog *log);
+void BM_log_free(BMLog *log, bool safe_mode);
-/* Get the number of log entries */
+BMLog *BM_log_unfreeze(BMesh *bm, BMLogEntry *entry);
+
+ /* Get the number of log entries */
int BM_log_length(const BMLog *log);
/* Apply a consistent ordering to BMesh vertices and faces */
diff --git a/source/blender/editors/sculpt_paint/sculpt.c b/source/blender/editors/sculpt_paint/sculpt.c
index 36f09b35499..d0abd706bc2 100644
--- a/source/blender/editors/sculpt_paint/sculpt.c
+++ b/source/blender/editors/sculpt_paint/sculpt.c
@@ -117,6 +117,22 @@ void SCULPT_vertex_random_access_ensure(SculptSession *ss)
}
}
+
+/* Sculpt PBVH abstraction API
+ *
+ * This is read-only, for writing use PBVH vertex iterators. There vd.index matches
+ * the indices used here.
+ *
+ * For multi-resolution, the same vertex in multiple grids is counted multiple times, with
+ * different index for each grid. */
+
+void SCULPT_face_random_access_ensure(SculptSession *ss)
+{
+ if (ss->bm) {
+ BM_mesh_elem_index_ensure(ss->bm, BM_FACE);
+ BM_mesh_elem_table_ensure(ss->bm, BM_FACE);
+ }
+}
int SCULPT_vertex_count_get(SculptSession *ss)
{
switch (BKE_pbvh_type(ss->pbvh)) {
@@ -9176,7 +9192,7 @@ static void sculpt_init_session(Main *bmain, Depsgraph *depsgraph, Scene *scene,
}
}
-void ED_object_sculptmode_enter_ex(Main *bmain,
+ATTR_NO_OPT void ED_object_sculptmode_enter_ex(Main *bmain,
Depsgraph *depsgraph,
Scene *scene,
Object *ob,
diff --git a/source/blender/editors/sculpt_paint/sculpt_dyntopo.c b/source/blender/editors/sculpt_paint/sculpt_dyntopo.c
index d046bf9b7f3..7c783f73ce2 100644
--- a/source/blender/editors/sculpt_paint/sculpt_dyntopo.c
+++ b/source/blender/editors/sculpt_paint/sculpt_dyntopo.c
@@ -23,10 +23,15 @@
#include "MEM_guardedalloc.h"
+#include "BLI_alloca.h"
#include "BLI_array.h"
#include "BLI_blenlib.h"
#include "BLI_hash.h"
+#include "BLI_linklist.h"
#include "BLI_math.h"
+#include "BLI_memarena.h"
+#include "BLI_compiler_attrs.h"
+#include "BLI_polyfill_2d.h"
#include "BLI_task.h"
#include "BLT_translation
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list