Fix #133128: Multires cavity automasking creates artifacts

Prior to the brush refactor in 4.3, the cavity automasking feature
had an implicit dependency on stable position values. In many cases,
such as during brush evaulation, this was upheld by the usage of the
proxy system, which meant that even if a vertex's neighbor was changed
during a stroke in paralell, these changes would not be accessed until
after a stroke finished.

With the new method of brush evaulation removing the proxy system, this
introduced subtle artifacts on BVH node boundaries, as vertices were
guaranteed to change and cause differences as there was no temporary
storage to use the "old" position data. This effect was most noticable
in 4.3 and beyond in multires sculpting, though similar artifacts can be
seen when using the mesh filter in all prior versions.

To fix these issues, instead of calculating the internal cavity factor
during a stroke or during deformations, we calculate it ahead of time
based on the affected nodes of a stroke. This has the benefit of now
behaving consistently for a given mesh and given brush or filter
application.

From a performance perspective, this change should have no noticeable
impact, as each BVH node, and by extension each vertex, only has its
corresponding cavity factor calculated once, when the stroke would
affect the node.

Related: #102745

Pull Request: https://projects.blender.org/blender/blender/pulls/133224

source/blender/editors/sculpt_paint/sculpt.cc

@@ -3151,6 +3151,12 @@ static void do_brush_action(const Depsgraph &depsgraph,
     return;
   }
 
+  if (auto_mask::is_enabled(sd, ob, &brush) && ss.cache->automasking &&
+      ss.cache->automasking->settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL)
+  {
+    ss.cache->automasking->calc_cavity_factor(depsgraph, ob, node_mask);
+  }
+
   if (!use_pixels) {
     push_undo_nodes(depsgraph, ob, brush, node_mask);
   }

source/blender/editors/sculpt_paint/sculpt_automask.hh

@@ -76,6 +76,21 @@ struct Cache {
    * \note -1 means the vertex value still needs to be calculated.
    */
   Array<float> cavity_factor;
+
+  /**
+   * Calculates the cavity factor for a set of nodes.
+   *
+   * Has no effect on an individual node level if the factor for a vertex has already been
+   * calculated. This data is best calculated outside of any loops that affect the stroke's
+   * position, as the curvature calculation is sensitive to small changes, meaning processing
+   * inside the normal brush update step may result in odd artifacts from ordering of position
+   * updates.
+   *
+   * \note Should be called prior to any call that may use the cavity mode.
+   */
+  void calc_cavity_factor(const Depsgraph &depsgraph,
+                          const Object &object,
+                          const IndexMask &node_mask);
 };
 
 /**

source/blender/editors/sculpt_paint/sculpt_automasking.cc

@@ -590,10 +590,10 @@ static float process_cavity_factor(const Cache &automasking, float factor)
   return factor;
 }
 
-static float calc_cavity_factor_mesh(const Depsgraph &depsgraph,
+static void calc_cavity_factor_mesh(const Depsgraph &depsgraph,
-                                     const Cache &automasking,
+                                    const Cache &automasking,
-                                     const Object &object,
+                                    const Object &object,
-                                     const int vert)
+                                    const int vert)
 {
   if (automasking.cavity_factor[vert] == -1.0f) {
     calc_blurred_cavity_mesh(depsgraph,
@@ -603,13 +603,12 @@ static float calc_cavity_factor_mesh(const Depsgraph &depsgraph,
                              vert,
                              const_cast<Cache &>(automasking).cavity_factor);
   }
-  return process_cavity_factor(automasking, automasking.cavity_factor[vert]);
 }
 
-static float calc_cavity_factor_grids(const CCGKey &key,
+static void calc_cavity_factor_grids(const CCGKey &key,
-                                      const Cache &automasking,
+                                     const Cache &automasking,
-                                      const Object &object,
+                                     const Object &object,
-                                      const int vert)
+                                     const int vert)
 {
   if (automasking.cavity_factor[vert] == -1.0f) {
     calc_blurred_cavity_grids(object,
@@ -618,10 +617,9 @@ static float calc_cavity_factor_grids(const CCGKey &key,
                               SubdivCCGCoord::from_index(key, vert),
                               const_cast<Cache &>(automasking).cavity_factor);
   }
-  return process_cavity_factor(automasking, automasking.cavity_factor[vert]);
 }
 
-static float calc_cavity_factor_bmesh(const Cache &automasking, BMVert *vert, const int vert_i)
+static void calc_cavity_factor_bmesh(const Cache &automasking, BMVert *vert, const int vert_i)
 {
   if (automasking.cavity_factor[vert_i] == -1.0f) {
     calc_blurred_cavity_bmesh(automasking,
@@ -629,7 +627,6 @@ static float calc_cavity_factor_bmesh(const Cache &automasking, BMVert *vert, co
                               vert,
                               const_cast<Cache &>(automasking).cavity_factor);
   }
-  return process_cavity_factor(automasking, automasking.cavity_factor[vert_i]);
 }
 
 void calc_vert_factors(const Depsgraph &depsgraph,
@@ -676,7 +673,8 @@ void calc_vert_factors(const Depsgraph &depsgraph,
       float cached_factor = automasking.factor[vert];
 
       if (automasking.settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL) {
-        cached_factor *= calc_cavity_factor_mesh(depsgraph, automasking, object, vert);
+        BLI_assert(automasking.cavity_factor[vert] != -1.0f);
+        cached_factor *= process_cavity_factor(automasking, automasking.cavity_factor[vert]);
       }
 
       factors[i] *= cached_factor;
@@ -738,7 +736,8 @@ void calc_vert_factors(const Depsgraph &depsgraph,
     }
 
     if (automasking.settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL) {
-      factors[i] *= calc_cavity_factor_mesh(depsgraph, automasking, object, vert);
+      BLI_assert(automasking.cavity_factor[vert] != -1.0f);
+      factors[i] *= process_cavity_factor(automasking, automasking.cavity_factor[vert]);
     }
   }
 }
@@ -783,7 +782,8 @@ void calc_face_factors(const Depsgraph &depsgraph,
         float cached_factor = automasking.factor[vert];
 
         if (automasking.settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL) {
-          cached_factor *= calc_cavity_factor_mesh(depsgraph, automasking, object, vert);
+          BLI_assert(automasking.cavity_factor[vert] != -1.0f);
+          cached_factor *= process_cavity_factor(automasking, automasking.cavity_factor[vert]);
         }
 
         factor *= cached_factor;
@@ -845,7 +845,8 @@ void calc_face_factors(const Depsgraph &depsgraph,
       }
 
       if (automasking.settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL) {
-        factor *= calc_cavity_factor_mesh(depsgraph, automasking, object, vert);
+        BLI_assert(automasking.cavity_factor[vert] != -1.0f);
+        factor *= process_cavity_factor(automasking, automasking.cavity_factor[vert]);
       }
     }
     factors[i] *= sum * math::rcp(float(face_verts.size()));
@@ -907,7 +908,8 @@ void calc_grids_factors(const Depsgraph &depsgraph,
         float cached_factor = automasking.factor[vert];
 
         if (automasking.settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL) {
-          cached_factor *= calc_cavity_factor_grids(key, automasking, object, vert);
+          BLI_assert(automasking.cavity_factor[vert] != -1.0f);
+          cached_factor *= process_cavity_factor(automasking, automasking.cavity_factor[vert]);
         }
 
         factors[node_vert] *= cached_factor;
@@ -974,7 +976,8 @@ void calc_grids_factors(const Depsgraph &depsgraph,
       }
 
       if (automasking.settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL) {
-        factors[node_vert] *= calc_cavity_factor_grids(key, automasking, object, vert);
+        BLI_assert(automasking.cavity_factor[vert] != -1.0f);
+        factors[node_vert] *= process_cavity_factor(automasking, automasking.cavity_factor[vert]);
       }
     }
   }
@@ -1020,7 +1023,8 @@ void calc_vert_factors(const Depsgraph &depsgraph,
       float cached_factor = automasking.factor[vert_i];
 
       if (automasking.settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL) {
-        cached_factor *= calc_cavity_factor_bmesh(automasking, vert, vert_i);
+        BLI_assert(automasking.cavity_factor[vert_i] != -1.0f);
+        cached_factor *= process_cavity_factor(automasking, automasking.cavity_factor[vert_i]);
       }
 
       factors[i] *= cached_factor;
@@ -1082,7 +1086,8 @@ void calc_vert_factors(const Depsgraph &depsgraph,
     }
 
     if (automasking.settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL) {
-      factors[i] *= calc_cavity_factor_bmesh(automasking, vert, vert_i);
+      BLI_assert(automasking.cavity_factor[vert_i] != -1.0f);
+      factors[i] *= process_cavity_factor(automasking, automasking.cavity_factor[vert_i]);
     }
   }
 }
@@ -1723,4 +1728,53 @@ std::unique_ptr<Cache> cache_init(const Depsgraph &depsgraph,
   return automasking;
 }
 
+void Cache::calc_cavity_factor(const Depsgraph &depsgraph,
+                               const Object &object,
+                               const IndexMask &node_mask)
+{
+  if ((this->settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL) == 0) {
+    return;
+  }
+
+  BLI_assert(!this->cavity_factor.is_empty());
+
+  const SculptSession &ss = *object.sculpt;
+  const bke::pbvh::Tree &pbvh = *bke::object::pbvh_get(object);
+  switch (pbvh.type()) {
+    case bke::pbvh::Type::Mesh: {
+      const Span<bke::pbvh::MeshNode> nodes = pbvh.nodes<bke::pbvh::MeshNode>();
+      node_mask.foreach_index(GrainSize(1), [&](const int i) {
+        const Span<int> verts = nodes[i].verts();
+        for (const int vert : verts) {
+          calc_cavity_factor_mesh(depsgraph, *this, object, vert);
+        }
+      });
+      break;
+    }
+    case bke::pbvh::Type::Grids: {
+      const SubdivCCG &subdiv_ccg = *ss.subdiv_ccg;
+      const CCGKey key = BKE_subdiv_ccg_key_top_level(subdiv_ccg);
+      const Span<bke::pbvh::GridsNode> nodes = pbvh.nodes<bke::pbvh::GridsNode>();
+      node_mask.foreach_index(GrainSize(1), [&](const int i) {
+        const Span<int> grids = nodes[i].grids();
+        for (const int grid : grids) {
+          for (const int vert : bke::ccg::grid_range(subdiv_ccg.grid_area, grid)) {
+            calc_cavity_factor_grids(key, *this, object, vert);
+          }
+        }
+      });
+      break;
+    }
+    case bke::pbvh::Type::BMesh: {
+      const Span<bke::pbvh::BMeshNode> nodes = pbvh.nodes<bke::pbvh::BMeshNode>();
+      node_mask.foreach_index(GrainSize(1), [&](const int i) {
+        const Set<BMVert *, 0> verts = nodes[i].bm_unique_verts_;
+        for (BMVert *vert : verts) {
+          calc_cavity_factor_bmesh(*this, vert, BM_elem_index_get(vert));
+        }
+      });
+    }
+  }
+}
+
 }  // namespace blender::ed::sculpt_paint::auto_mask

source/blender/editors/sculpt_paint/sculpt_cloth.cc

@@ -2296,6 +2296,12 @@ static int sculpt_cloth_filter_modal(bContext *C, wmOperator *op, const wmEvent
 
   const IndexMask &node_mask = ss.filter_cache->node_mask;
 
+  if (auto_mask::is_enabled(sd, object, nullptr) && ss.filter_cache->automasking &&
+      ss.filter_cache->automasking->settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL)
+  {
+    ss.filter_cache->automasking->calc_cavity_factor(*depsgraph, object, node_mask);
+  }
+
   float3 gravity(0.0f);
   if (sd.gravity_object) {
     gravity = sd.gravity_object->object_to_world().ptr()[2];

source/blender/editors/sculpt_paint/sculpt_filter_color.cc

@@ -371,6 +371,7 @@ static void sculpt_color_presmooth_init(const Mesh &mesh, Object &object)
 static void sculpt_color_filter_apply(bContext *C, wmOperator *op, Object &ob)
 {
   const Depsgraph &depsgraph = *CTX_data_depsgraph_pointer(C);
+  const Sculpt &sd = *CTX_data_tool_settings(C)->sculpt;
   SculptSession &ss = *ob.sculpt;
   bke::pbvh::Tree &pbvh = *bke::object::pbvh_get(ob);
   MutableSpan<bke::pbvh::MeshNode> nodes = pbvh.nodes<bke::pbvh::MeshNode>();
@@ -388,6 +389,11 @@ static void sculpt_color_filter_apply(bContext *C, wmOperator *op, Object &ob)
   }
 
   const IndexMask &node_mask = ss.filter_cache->node_mask;
+  if (auto_mask::is_enabled(sd, ob, nullptr) && ss.filter_cache->automasking &&
+      ss.filter_cache->automasking->settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL)
+  {
+    ss.filter_cache->automasking->calc_cavity_factor(depsgraph, ob, node_mask);
+  }
 
   const OffsetIndices<int> faces = mesh.faces();
   const Span<int> corner_verts = mesh.corner_verts();

source/blender/editors/sculpt_paint/sculpt_filter_mesh.cc

@@ -2132,6 +2132,11 @@ static void sculpt_mesh_filter_apply(bContext *C, wmOperator *op, bool is_replay
   SCULPT_vertex_random_access_ensure(ob);
 
   const IndexMask &node_mask = ss.filter_cache->node_mask;
+  if (auto_mask::is_enabled(sd, ob, nullptr) && ss.filter_cache->automasking &&
+      ss.filter_cache->automasking->settings.flags & BRUSH_AUTOMASKING_CAVITY_ALL)
+  {
+    ss.filter_cache->automasking->calc_cavity_factor(depsgraph, ob, node_mask);
+  }
   switch (filter_type) {
     case MeshFilterType::Smooth:
       calc_smooth_filter(depsgraph,