bartvdbraak/blender
1
0
Fork 4
Code Issues 4 Pull Requests 1 Actions 1 Packages Projects Releases Wiki Activity

Fix #113988: Deformed PBVH modifies original normals

Browse Source 
The PBVH is given the evaluated positions from the end of the modifier
stack, but then updates normals from the original mesh based on those
positions. This causes a feedback loop, even when in vertex paint mode.

As mentioned in a comment, calculating these normals and duplicating
the arrays is quite wasteful. But it's necessary since the PBVH always
assumes we are interested in modifying the positions, and also always
retrieves the normals from the original mesh too. Those things would
be good to untangle at some point too, but for now this change is small
and helps to clarify the situation as well.

Caused by: b339e3937d8dc580b655efe599c3b0f97f412fb1

Pull Request: https://projects.blender.org/blender/blender/pulls/114118
This commit is contained in:
Hans Goudey 2023-10-24 18:56:15 +02:00 committed by Hans Goudey
parent 74af0528f4
commit 5af8b839cf
2 changed files with 77 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

99
source/blender/blenkernel/intern/pbvh.cc
View File

@ -809,17 +809,15 @@ void BKE_pbvh_update_mesh_pointers(PBVH *pbvh, Mesh *mesh)
pbvh->looptri_faces = mesh->looptri_faces(); pbvh->looptri_faces = mesh->looptri_faces();
if (!pbvh->deformed) { if (!pbvh->deformed) {
/* Deformed positions not matching the original mesh are owned directly by the PBVH, and are /* Deformed data not matching the original mesh are owned directly by the PBVH, and are
* set separately by #BKE_pbvh_vert_coords_apply. */ * set separately by #BKE_pbvh_vert_coords_apply. */
pbvh->vert_positions = mesh->vert_positions_for_write(); pbvh->vert_positions = mesh->vert_positions_for_write();
pbvh->vert_normals = mesh->vert_normals();
pbvh->face_normals = mesh->face_normals();
} }
BKE_pbvh_update_hide_attributes_from_mesh(pbvh); BKE_pbvh_update_hide_attributes_from_mesh(pbvh);
/* Make sure cached normals start out calculated. */
pbvh->vert_normals = mesh->vert_normals();
pbvh->face_normals = mesh->face_normals();
pbvh->vert_data = &mesh->vert_data; pbvh->vert_data = &mesh->vert_data;
pbvh->loop_data = &mesh->loop_data; pbvh->loop_data = &mesh->loop_data;
pbvh->face_data = &mesh->face_data; pbvh->face_data = &mesh->face_data;
@ -1290,6 +1288,38 @@ static bool update_search(PBVHNode *node, const int flag)
return true; return true;
} }
static void normals_calc_faces(const Span<float3> positions,
const blender::OffsetIndices<int> faces,
const Span<int> corner_verts,
const Span<int> mask,
MutableSpan<float3> face_normals)
{
using namespace blender;
using namespace blender::bke;
threading::parallel_for(mask.index_range(), 512, [&](const IndexRange range) {
for (const int i : mask.slice(range)) {
face_normals[i] = mesh::face_normal_calc(positions, corner_verts.slice(faces[i]));
}
});
}
static void normals_calc_verts_simple(const blender::GroupedSpan<int> vert_to_face_map,
const Span<float3> face_normals,
const Span<int> mask,
MutableSpan<float3> vert_normals)
{
using namespace blender;
threading::parallel_for(mask.index_range(), 1024, [&](const IndexRange range) {
for (const int vert : mask.slice(range)) {
float3 normal(0.0f);
for (const int face : vert_to_face_map[vert]) {
normal += face_normals[face];
}
vert_normals[vert] = math::normalize(normal);
}
});
}
static void pbvh_faces_update_normals(PBVH *pbvh, Span<PBVHNode *> nodes, Mesh &mesh) static void pbvh_faces_update_normals(PBVH *pbvh, Span<PBVHNode *> nodes, Mesh &mesh)
{ {
using namespace blender; using namespace blender;
@ -1316,13 +1346,17 @@ static void pbvh_faces_update_normals(PBVH *pbvh, Span<PBVHNode *> nodes, Mesh &
VectorSet<int> verts_to_update; VectorSet<int> verts_to_update;
threading::parallel_invoke( threading::parallel_invoke(
[&]() { [&]() {
mesh.runtime->face_normals_cache.update([&](Vector<float3> &r_data) { if (pbvh->deformed) {
threading::parallel_for(faces_to_update.index_range(), 512, [&](const IndexRange range) { normals_calc_faces(
for (const int i : faces_to_update.as_span().slice(range)) { positions, faces, corner_verts, faces_to_update, pbvh->face_normals_deformed);
r_data[i] = mesh::face_normal_calc(positions, corner_verts.slice(faces[i])); }
} else {
mesh.runtime->face_normals_cache.update([&](Vector<float3> &r_data) {
normals_calc_faces(positions, faces, corner_verts, faces_to_update, r_data);
}); });
}); /* #SharedCache::update() reallocates cached vectors if they were shared initially. */
pbvh->face_normals = mesh.runtime->face_normals_cache.data();
}
}, },
[&]() { [&]() {
/* Update all normals connected to affected faces, even if not explicitly tagged. */ /* Update all normals connected to affected faces, even if not explicitly tagged. */
@ -1339,22 +1373,16 @@ static void pbvh_faces_update_normals(PBVH *pbvh, Span<PBVHNode *> nodes, Mesh &
} }
}); });
const Span<float3> face_normals = mesh.face_normals(); if (pbvh->deformed) {
mesh.runtime->vert_normals_cache.update([&](Vector<float3> &r_data) { normals_calc_verts_simple(
threading::parallel_for(verts_to_update.index_range(), 1024, [&](const IndexRange range) { pbvh->pmap, pbvh->face_normals, verts_to_update, pbvh->vert_normals_deformed);
for (const int vert : verts_to_update.as_span().slice(range)) { }
float3 normal(0.0f); else {
for (const int face : pbvh->pmap[vert]) { mesh.runtime->vert_normals_cache.update([&](Vector<float3> &r_data) {
normal += face_normals[face]; normals_calc_verts_simple(pbvh->pmap, pbvh->face_normals, verts_to_update, r_data);
}
r_data[vert] = math::normalize(normal);
}
}); });
}); pbvh->vert_normals = mesh.runtime->vert_normals_cache.data();
}
/* #SharedCache::update() reallocates the cached vectors if they were shared initially. */
pbvh->face_normals = mesh.runtime->face_normals_cache.data();
pbvh->vert_normals = mesh.runtime->vert_normals_cache.data();
} }
static void node_update_mask_redraw(PBVH &pbvh, PBVHNode &node) static void node_update_mask_redraw(PBVH &pbvh, PBVHNode &node)
@ -2978,14 +3006,21 @@ void BKE_pbvh_vert_coords_apply(PBVH *pbvh, const float (*vertCos)[3], const int
if (!pbvh->deformed) { if (!pbvh->deformed) {
if (!pbvh->vert_positions.is_empty()) { if (!pbvh->vert_positions.is_empty()) {
/* if pbvh is not already deformed, verts/faces points to the */ /* When the PBVH is deformed, it creates a separate vertex position array that it owns
/* original data and applying new coords to this arrays would lead to */ * directly. Conceptually these copies often aren't and often adds extra indirection, but:
/* unneeded deformation -- duplicate verts/faces to avoid this */ * - Sculpting shape keys, the deformations are flushed back to the keys as a separate step.
pbvh->vert_positions_deformed = blender::Array<float3>(pbvh->vert_positions.as_span()); * - Sculpting on a deformed mesh, deformations are also flushed to original positions
* separately.
* - The PBVH currently always assumes we want to change positions, and has no way to avoid
* calculating normals if it's only used for painting, for example. */
pbvh->vert_positions_deformed = pbvh->vert_positions.as_span();
pbvh->vert_positions = pbvh->vert_positions_deformed; pbvh->vert_positions = pbvh->vert_positions_deformed;
/* No need to dupalloc pbvh->looptri, this one is 'totally owned' by pbvh, pbvh->vert_normals_deformed = pbvh->vert_normals;
* it's never some mesh data. */ pbvh->vert_normals = pbvh->vert_normals_deformed;
pbvh->face_normals_deformed = pbvh->face_normals;
pbvh->face_normals = pbvh->face_normals_deformed;
pbvh->deformed = true; pbvh->deformed = true;
} }

14
source/blender/blenkernel/intern/pbvh_intern.hh
View File

@ -151,13 +151,19 @@ struct PBVH {
/* Mesh data */ /* Mesh data */
Mesh *mesh; Mesh *mesh;
/** Local array used when not sculpting base mesh positions directly. */
blender::Array<blender::float3> vert_positions_deformed;
/** Local array used when not sculpting base mesh positions directly. */
blender::Array<blender::float3> vert_normals_deformed;
/** Local array used when not sculpting base mesh positions directly. */
blender::Array<blender::float3> face_normals_deformed;
blender::MutableSpan<blender::float3> vert_positions;
blender::Span<blender::float3> vert_normals; blender::Span<blender::float3> vert_normals;
blender::Span<blender::float3> face_normals; blender::Span<blender::float3> face_normals;
bool *hide_vert;
blender::MutableSpan<blender::float3> vert_positions;
/** Local vertex positions owned by the PVBH when not sculpting base mesh positions directly. */
blender::Array<blender::float3> vert_positions_deformed;
blender::OffsetIndices<int> faces; blender::OffsetIndices<int> faces;
bool *hide_vert;
bool *hide_poly; bool *hide_poly;
/** Only valid for polygon meshes. */ /** Only valid for polygon meshes. */
blender::Span<int> corner_verts; blender::Span<int> corner_verts;