Fix: Crash in sculpt mode with shared normals caches #4

Closed
Hans Goudey wants to merge 3 commits from fix-pbvh-normals-crash into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
4 changed files with 29 additions and 5 deletions
Showing only changes of commit e690282e65 - Show all commits

View File

@ -1492,7 +1492,7 @@ std::optional<blender::Bounds<blender::float3>> Mesh::bounds_min_max() const
void Mesh::bounds_set_eager(const blender::Bounds<float3> &bounds) void Mesh::bounds_set_eager(const blender::Bounds<float3> &bounds)
{ {
this->runtime->bounds_cache.ensure([&](blender::Bounds<float3> &r_data) { r_data = bounds; }); this->runtime->bounds_cache.update([&](blender::Bounds<float3> &r_data) { r_data = bounds; });
} }
void BKE_mesh_transform(Mesh *me, const float mat[4][4], bool do_keys) void BKE_mesh_transform(Mesh *me, const float mat[4][4], bool do_keys)

View File

@ -30,6 +30,7 @@ template<typename T> class SharedCache {
struct CacheData { struct CacheData {
CacheMutex mutex; CacheMutex mutex;
T data; T data;
CacheData(T data) : data(std::move(data)) {}
}; };
std::shared_ptr<CacheData> cache_; std::shared_ptr<CacheData> cache_;
@ -60,6 +61,23 @@ template<typename T> class SharedCache {
cache_->mutex.ensure([&]() { compute_cache(this->cache_->data); }); cache_->mutex.ensure([&]() { compute_cache(this->cache_->data); });
} }
/**
* Represents a combination of "tag dirty" and "update cache for new data." Existing cached
* values are kept available (copied from shared data if necessary). This can be helpful when
* the recalculation is only expected to make a small change to the cached data, since using
* #tag_dirty() and #ensure() separately may require rebuilding the cache from scratch.
*/
void update(FunctionRef<void(T &data)> compute_cache)
{
if (cache_.unique()) {
cache_->mutex.tag_dirty();
}
else {
cache_ = std::make_shared<CacheData>(cache_->data);
}
cache_->mutex.ensure([&]() { compute_cache(this->cache_->data); });
}
/** Retrieve the cached data. */ /** Retrieve the cached data. */
const T &data() const const T &data() const
{ {

View File

@ -5421,9 +5421,12 @@ void SCULPT_flush_update_step(bContext *C, SculptUpdateType update_flags)
if (update_flags & SCULPT_UPDATE_COORDS && !ss->shapekey_active) { if (update_flags & SCULPT_UPDATE_COORDS && !ss->shapekey_active) {
if (BKE_pbvh_type(ss->pbvh) == PBVH_FACES) { if (BKE_pbvh_type(ss->pbvh) == PBVH_FACES) {
/* When sculpting and changing the positions of a mesh, tag them as changed and update. */ /* Updating mesh positions without marking caches dirty is generally not good, but since
BKE_mesh_tag_positions_changed(mesh); * sculpt mode has special requirements and is expected to have sole ownership of the mesh it
/* Update the mesh's bounds eagerly since the PBVH already has that information. */ * modifies, it's generally okay.
*
* Vertex and face normals are updated later in #BKE_pbvh_update_normals. However, we update
* the mesh's bounds eagerly here since they are trivial to access from the PBVH. */
Bounds<float3> bounds; Bounds<float3> bounds;
BKE_pbvh_bounding_box(ob->sculpt->pbvh, bounds.min, bounds.max); BKE_pbvh_bounding_box(ob->sculpt->pbvh, bounds.min, bounds.max);
mesh->bounds_set_eager(bounds); mesh->bounds_set_eager(bounds);

View File

@ -313,7 +313,10 @@ typedef struct Mesh {
*/ */
std::optional<blender::Bounds<blender::float3>> bounds_min_max() const; std::optional<blender::Bounds<blender::float3>> bounds_min_max() const;
/** Set cached mesh bounds to a known-correct value to avoid their lazy calculation later on. */ /**
* After positions are changed, set cached mesh bounds to a known-correct value to avoid their
* lazy calculation later on.
*/
void bounds_set_eager(const blender::Bounds<blender::float3> &bounds); void bounds_set_eager(const blender::Bounds<blender::float3> &bounds);
/** /**