diff --git a/source/blender/editors/sculpt_paint/brushes/draw.cc b/source/blender/editors/sculpt_paint/brushes/draw.cc index 916f06f0a6d..3b3bd21a4f0 100644 --- a/source/blender/editors/sculpt_paint/brushes/draw.cc +++ b/source/blender/editors/sculpt_paint/brushes/draw.cc @@ -43,8 +43,7 @@ static void calc_faces(const Sculpt &sd, const PBVHNode &node, Object &object, LocalData &tls, - const MutableSpan positions_sculpt, - const MutableSpan positions_mesh) + const MutableSpan positions_orig) { SculptSession &ss = *object.sculpt; StrokeCache &cache = *ss.cache; @@ -84,8 +83,9 @@ static void calc_faces(const Sculpt &sd, apply_crazyspace_to_translations(ss.deform_imats, verts, translations); } - apply_translations(translations, verts, positions_sculpt); - flush_positions_to_shape_keys(object, verts, positions_sculpt, positions_mesh); + apply_translations(translations, verts, positions_orig); + apply_translations_to_shape_keys(object, verts, translations, positions_orig); + apply_translations_to_pbvh(*ss.pbvh, verts, translations); } static void calc_grids(Object &object, const Brush &brush, const float3 &offset, PBVHNode &node) @@ -201,8 +201,7 @@ void do_draw_brush(const Sculpt &sd, Object &object, Span nodes) const PBVH &pbvh = *ss.pbvh; const Span positions_eval = BKE_pbvh_get_vert_positions(pbvh); const Span vert_normals = BKE_pbvh_get_vert_normals(pbvh); - MutableSpan positions_sculpt = mesh_brush_positions_for_write(*object.sculpt, mesh); - MutableSpan positions_mesh = mesh.vert_positions_for_write(); + MutableSpan positions_orig = mesh.vert_positions_for_write(); threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { LocalData &tls = all_tls.local(); for (const int i : range) { @@ -214,8 +213,7 @@ void do_draw_brush(const Sculpt &sd, Object &object, Span nodes) *nodes[i], object, tls, - positions_sculpt, - positions_mesh); + positions_orig); BKE_pbvh_node_mark_positions_update(nodes[i]); } }); diff --git a/source/blender/editors/sculpt_paint/brushes/draw_vector_displacement.cc b/source/blender/editors/sculpt_paint/brushes/draw_vector_displacement.cc index a3ed0b122a5..a7ecc997549 100644 --- a/source/blender/editors/sculpt_paint/brushes/draw_vector_displacement.cc +++ b/source/blender/editors/sculpt_paint/brushes/draw_vector_displacement.cc @@ -64,8 +64,7 @@ static void calc_faces(const Sculpt &sd, const PBVHNode &node, Object &object, LocalData &tls, - MutableSpan positions_orig, - MutableSpan mesh_positions) + MutableSpan positions_orig) { SculptSession &ss = *object.sculpt; StrokeCache &cache = *ss.cache; @@ -108,7 +107,8 @@ static void calc_faces(const Sculpt &sd, } apply_translations(translations, verts, positions_orig); - flush_positions_to_shape_keys(object, verts, positions_orig, mesh_positions); + apply_translations_to_shape_keys(object, verts, translations, positions_orig); + apply_translations_to_pbvh(*ss.pbvh, verts, translations); } static void calc_grids(Object &object, const Brush &brush, PBVHNode &node) @@ -224,20 +224,12 @@ void do_draw_vector_displacement_brush(const Sculpt &sd, Object &object, Span positions_eval = BKE_pbvh_get_vert_positions(pbvh); const Span vert_normals = BKE_pbvh_get_vert_normals(pbvh); - MutableSpan positions_orig = mesh_brush_positions_for_write(*object.sculpt, mesh); - MutableSpan mesh_positions = mesh.vert_positions_for_write(); + MutableSpan positions_orig = mesh.vert_positions_for_write(); threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { LocalData &tls = all_tls.local(); for (const int i : range) { - calc_faces(sd, - brush, - positions_eval, - vert_normals, - *nodes[i], - object, - tls, - positions_orig, - mesh_positions); + calc_faces( + sd, brush, positions_eval, vert_normals, *nodes[i], object, tls, positions_orig); BKE_pbvh_node_mark_positions_update(nodes[i]); } }); diff --git a/source/blender/editors/sculpt_paint/mesh_brush_common.hh b/source/blender/editors/sculpt_paint/mesh_brush_common.hh index a5048f356b1..0e3d144c729 100644 --- a/source/blender/editors/sculpt_paint/mesh_brush_common.hh +++ b/source/blender/editors/sculpt_paint/mesh_brush_common.hh @@ -40,21 +40,10 @@ struct Cache; /** * Note on the various positions arrays: - * - positions_sculpt: The positions affected by brush strokes (maybe indirectly). Owned by the - * PBVH or mesh. - * - positions_mesh: Positions owned by the original mesh. Not the same as `positions_sculpt` if + * - positions_orig: Positions owned by the original mesh. Not the same as `positions_eval` if * there are deform modifiers. * - positions_eval: Positions after procedural deformation, used to build the PBVH. Translations - * are built for these values, then applied to `positions_sculpt`. - * - * Only two of these arrays are actually necessary. The third comes from the fact that the PBVH - * currently stores its own copy of positions when there are deformations. If that was removed, the - * situation would be clearer. - * - * \todo Get rid of one of the arrays mentioned above to avoid the situation with evaluated - * positions, original positions, and then a third copy that's just there because of historical - * reasons. This would involve removing access to positions and normals from the PBVH structure, - * which should only be concerned with splitting geometry into spatially contiguous chunks. + * are built for these values, then applied to `positions_orig`. */ /** @@ -147,21 +136,21 @@ void clip_and_lock_translations(const Sculpt &sd, Span verts, MutableSpan translations); -/** - * Retrieve the final mutable positions array to be modified. - * - * \note See the comment at the top of this file for context. - */ -MutableSpan mesh_brush_positions_for_write(SculptSession &ss, Mesh &mesh); - /** * Applying final positions to shape keys is non-trivial because the mesh positions and the active * shape key positions must be kept in sync, and shape keys dependent on the active key must also * be modified. */ -void flush_positions_to_shape_keys(Object &object, - Span verts, - Span positions, - MutableSpan positions_mesh); +void apply_translations_to_shape_keys(Object &object, + Span verts, + Span translations, + MutableSpan positions_mesh); + +/** + * Currently the PBVH owns its own copy of deformed positions that needs to be updated to stay in + * sync with brush deformations. + * \todo This should be removed one the PBVH no longer stores this copy of deformed positions. + */ +void apply_translations_to_pbvh(PBVH &pbvh, Span verts, Span positions_orig); } // namespace blender::ed::sculpt_paint diff --git a/source/blender/editors/sculpt_paint/sculpt.cc b/source/blender/editors/sculpt_paint/sculpt.cc index 6a0fe56dfbd..63297ef6a8f 100644 --- a/source/blender/editors/sculpt_paint/sculpt.cc +++ b/source/blender/editors/sculpt_paint/sculpt.cc @@ -6686,58 +6686,49 @@ void clip_and_lock_translations(const Sculpt &sd, } } -MutableSpan mesh_brush_positions_for_write(SculptSession &ss, Mesh & /*mesh*/) -{ - /* TODO: Eventually this should retrieve mutable positions directly from the mesh or the active - * shape key, instead of keeping a mutable reference to an array stored in the PBVH. That will - * help avoid copies when user edits don't affect positions, will help to make code safer because - * there will be less potentially-stale state, and will make it less confusing by avoiding - * redundant data storage. */ - return BKE_pbvh_get_vert_positions(*ss.pbvh); -} - -void flush_positions_to_shape_keys(Object &object, - const Span verts, - const Span positions, - const MutableSpan positions_mesh) +void apply_translations_to_shape_keys(Object &object, + const Span verts, + const Span translations, + const MutableSpan positions_orig) { Mesh &mesh = *static_cast(object.data); KeyBlock *active_key = BKE_keyblock_from_object(&object); if (!active_key) { return; } + MutableSpan active_key_data(static_cast(active_key->data), active_key->totelem); + if (active_key == mesh.key->refkey) { + for (const int vert : verts) { + active_key_data[vert] = positions_orig[vert]; + } + } + else { + apply_translations(translations, verts, active_key_data); + } /* For relative keys editing of base should update other keys. */ if (bool *dependent = BKE_keyblock_get_dependent_keys(mesh.key, object.shapenr - 1)) { - /* TODO: Avoid allocation by using translations already calculated by brush. */ - Array offsets(verts.size()); - for (const int i : verts.index_range()) { - offsets[i] = positions[verts[i]] - active_key_data[verts[i]]; - } - int i; LISTBASE_FOREACH_INDEX (KeyBlock *, other_key, &mesh.key->block, i) { if ((other_key != active_key) && dependent[i]) { MutableSpan data(static_cast(other_key->data), other_key->totelem); - apply_translations(offsets, verts, data); + apply_translations(translations, verts, data); } } - MEM_freeN(dependent); } +} - /* Modifying of basis key should update mesh. */ - if (active_key == mesh.key->refkey) { - /* TODO: There are too many positions arrays getting passed around. We should have a better - * naming system or not have to constantly update both the base shape key and original - * positions. OTOH, maybe it's just a consequence of the bad design of shape keys. */ - apply_translations(positions, verts, positions_mesh); +void apply_translations_to_pbvh(PBVH &pbvh, Span verts, const Span translations) +{ + if (!BKE_pbvh_is_deformed(pbvh)) { + return; } - - /* Apply new positions to active shape key. */ - for (const int vert : verts) { - active_key_data[vert] = positions[vert]; + MutableSpan pbvh_positions = BKE_pbvh_get_vert_positions(pbvh); + for (const int i : verts.index_range()) { + const int vert = verts[i]; + pbvh_positions[vert] += translations[i]; } }