PBVH: update mesh data pointers stored in pbvh #106271

Merged
Jacques Lucke merged 11 commits from JacquesLucke/blender:pbvh-update-pointers into main 2023-04-08 13:29:49 +02:00
4 changed files with 72 additions and 74 deletions

View File

@ -283,28 +283,11 @@ typedef void (*BKE_pbvh_SearchNearestCallback)(PBVHNode *node, void *data, float
PBVH *BKE_pbvh_new(PBVHType type);
#ifdef __cplusplus
/**
* Do a full rebuild with on Mesh data structure.
*
* \note Unlike mpoly/corner_verts/verts, looptri is *totally owned* by PBVH
* (which means it may rewrite it if needed, see #BKE_pbvh_vert_coords_apply().
*/
void BKE_pbvh_build_mesh(PBVH *pbvh,
struct Mesh *mesh,
blender::OffsetIndices<int> polys,
const int *corner_verts,
float (*vert_positions)[3],
int totvert,
struct CustomData *vdata,
struct CustomData *ldata,
struct CustomData *pdata,
const struct MLoopTri *looptri,
int looptri_num);
#endif
void BKE_pbvh_build_mesh(PBVH *pbvh, struct Mesh *mesh);
void BKE_pbvh_update_mesh_pointers(PBVH *pbvh, struct Mesh *mesh);
/**
* Do a full rebuild with on Grids data structure.
*/

View File

@ -2177,30 +2177,10 @@ static PBVH *build_pbvh_for_dynamic_topology(Object *ob)
static PBVH *build_pbvh_from_regular_mesh(Object *ob, Mesh *me_eval_deform, bool respect_hide)
{
Mesh *me = BKE_object_get_original_mesh(ob);
const int looptris_num = poly_to_tri_count(me->totpoly, me->totloop);
PBVH *pbvh = BKE_pbvh_new(PBVH_FACES);
BKE_pbvh_respect_hide_set(pbvh, respect_hide);
MutableSpan<float3> positions = me->vert_positions_for_write();
const blender::OffsetIndices polys = me->polys();
const Span<int> corner_verts = me->corner_verts();
MLoopTri *looptri = static_cast<MLoopTri *>(
MEM_malloc_arrayN(looptris_num, sizeof(*looptri), __func__));
blender::bke::mesh::looptris_calc(positions, polys, corner_verts, {looptri, looptris_num});
BKE_pbvh_build_mesh(pbvh,
me,
polys,
corner_verts.data(),
reinterpret_cast<float(*)[3]>(positions.data()),
me->totvert,
&me->vdata,
&me->ldata,
&me->pdata,
looptri,
looptris_num);
BKE_pbvh_build_mesh(pbvh, me);
const bool is_deformed = check_sculpt_object_deformed(ob, true);
if (is_deformed && me_eval_deform != nullptr) {
Review

I wonder if it would make sense to check this before BKE_pbvh_build_mesh and give BKE_pbvh_build_mesh an optional argument for positions not owned by the mesh. That would go a long way to help clarify whether PBVH should reuse the arrays for the mesh's looptris and normals too, which is the biggest remaining pain point in this area IMO.

Something similar would apply to sculpt_update_object, where the chunks of code currently calling BKE_pbvh_vert_coords_apply could be moved to happen before BKE_sculpt_object_pbvh_ensure (which would also get a new non-mesh positions argument).

Not sure if you'd consider this related (feel free to disregard it). If not, I'll probably look into this in the near future.

I wonder if it would make sense to check this before `BKE_pbvh_build_mesh` and give `BKE_pbvh_build_mesh` an optional argument for positions not owned by the mesh. That would go a long way to help clarify whether `PBVH` should reuse the arrays for the mesh's looptris and normals too, which is the biggest remaining pain point in this area IMO. Something similar would apply to `sculpt_update_object`, where the chunks of code currently calling `BKE_pbvh_vert_coords_apply` could be moved to happen before `BKE_sculpt_object_pbvh_ensure` (which would also get a new non-mesh positions argument). Not sure if you'd consider this related (feel free to disregard it). If not, I'll probably look into this in the near future.
Review

Yeah that would probably improve the code even more, but won't integrate it into this patch.

Yeah that would probably improve the code even more, but won't integrate it into this patch.
@ -2243,14 +2223,25 @@ PBVH *BKE_sculpt_object_pbvh_ensure(Depsgraph *depsgraph, Object *ob)
PBVH *pbvh = ob->sculpt->pbvh;
if (pbvh != nullptr) {
/* NOTE: It is possible that grids were re-allocated due to modifier
* stack. Need to update those pointers. */
if (BKE_pbvh_type(pbvh) == PBVH_GRIDS) {
Object *object_eval = DEG_get_evaluated_object(depsgraph, ob);
Mesh *mesh_eval = static_cast<Mesh *>(object_eval->data);
SubdivCCG *subdiv_ccg = mesh_eval->runtime->subdiv_ccg;
if (subdiv_ccg != nullptr) {
BKE_sculpt_bvh_update_from_ccg(pbvh, subdiv_ccg);
/* NOTE: It is possible that pointers to grids or other geometry data changed. Need to update
* those pointers. */
const PBVHType pbvh_type = BKE_pbvh_type(pbvh);
switch (pbvh_type) {
case PBVH_FACES: {
BKE_pbvh_update_mesh_pointers(pbvh, BKE_object_get_original_mesh(ob));
break;
}
case PBVH_GRIDS: {
Object *object_eval = DEG_get_evaluated_object(depsgraph, ob);
Mesh *mesh_eval = static_cast<Mesh *>(object_eval->data);
SubdivCCG *subdiv_ccg = mesh_eval->runtime->subdiv_ccg;
if (subdiv_ccg != nullptr) {
BKE_sculpt_bvh_update_from_ccg(pbvh, subdiv_ccg);
}
break;
}
case PBVH_BMESH: {
break;
}
}

View File

@ -36,6 +36,10 @@
#include "pbvh_intern.hh"
using blender::float3;
using blender::MutableSpan;
using blender::Span;
#define LEAF_LIMIT 10000
/* Uncomment to test if triangles of the same face are
@ -823,36 +827,58 @@ static void pbvh_validate_node_prims(PBVH *pbvh)
}
#endif
void BKE_pbvh_build_mesh(PBVH *pbvh,
Mesh *mesh,
const blender::OffsetIndices<int> polys,
const int *corner_verts,
float (*vert_positions)[3],
int totvert,
CustomData *vdata,
CustomData *ldata,
CustomData *pdata,
const MLoopTri *looptri,
int looptri_num)
void BKE_pbvh_update_mesh_pointers(PBVH *pbvh, Mesh *mesh)
{
BLI_assert(pbvh->header.type == PBVH_FACES);
pbvh->polys = mesh->polys();
pbvh->corner_verts = mesh->corner_verts().data();
if (!pbvh->deformed) {
/* Deformed positions not matching the original mesh are owned directly by the PBVH, and are
JacquesLucke marked this conversation as resolved Outdated

This comment is a bit confusing. I think a more correct phrasing would be Deformed positions not matching the original mesh are owned directly by the PBVH, and are set separately by #BKE_pbvh_vert_coords_apply (though I do suggest tweaking that a bit below).

This comment is a bit confusing. I think a more correct phrasing would be `Deformed positions not matching the original mesh are owned directly by the PBVH, and are set separately by #BKE_pbvh_vert_coords_apply` (though I do suggest tweaking that a bit below).
* set separately by #BKE_pbvh_vert_coords_apply. */
pbvh->vert_positions = BKE_mesh_vert_positions_for_write(mesh);
}
pbvh->material_indices = static_cast<const int *>(
CustomData_get_layer_named(&mesh->pdata, CD_PROP_INT32, "material_index"));
pbvh->hide_poly = static_cast<bool *>(CustomData_get_layer_named_for_write(
&mesh->pdata, CD_PROP_BOOL, ".hide_poly", mesh->totpoly));
pbvh->hide_vert = static_cast<bool *>(CustomData_get_layer_named_for_write(
&mesh->vdata, CD_PROP_BOOL, ".hide_vert", mesh->totvert));
/* Make sure cached normals start out calculated. */
mesh->vert_normals();
pbvh->vert_normals = BKE_mesh_vert_normals_for_write(mesh);
pbvh->vdata = &mesh->vdata;
pbvh->ldata = &mesh->ldata;
pbvh->pdata = &mesh->pdata;
}
void BKE_pbvh_build_mesh(PBVH *pbvh, Mesh *mesh)
{
BBC *prim_bbc = nullptr;
BB cb;
const int totvert = mesh->totvert;
const int looptri_num = poly_to_tri_count(mesh->totpoly, mesh->totloop);
MutableSpan<float3> vert_positions = mesh->vert_positions_for_write();
const blender::OffsetIndices<int> polys = mesh->polys();
const Span<int> corner_verts = mesh->corner_verts();
MLoopTri *looptri = static_cast<MLoopTri *>(
MEM_malloc_arrayN(looptri_num, sizeof(*looptri), __func__));
blender::bke::mesh::looptris_calc(vert_positions, polys, corner_verts, {looptri, looptri_num});
pbvh->mesh = mesh;
pbvh->header.type = PBVH_FACES;
pbvh->polys = polys;
pbvh->hide_poly = static_cast<bool *>(CustomData_get_layer_named_for_write(
&mesh->pdata, CD_PROP_BOOL, ".hide_poly", mesh->totpoly));
pbvh->material_indices = static_cast<const int *>(
CustomData_get_layer_named(&mesh->pdata, CD_PROP_INT32, "material_index"));
pbvh->corner_verts = corner_verts;
BKE_pbvh_update_mesh_pointers(pbvh, mesh);
/* Those are not set in #BKE_pbvh_update_mesh_pointers because they are owned by the #PBVH. */
pbvh->looptri = looptri;
pbvh->vert_positions = vert_positions;
/* Make sure cached normals start out calculated. */
mesh->vert_normals();
pbvh->vert_normals = BKE_mesh_vert_normals_for_write(mesh);
pbvh->hide_vert = static_cast<bool *>(CustomData_get_layer_named_for_write(
&mesh->vdata, CD_PROP_BOOL, ".hide_vert", mesh->totvert));
pbvh->vert_bitmap = static_cast<bool *>(
MEM_calloc_arrayN(totvert, sizeof(bool), "bvh->vert_bitmap"));
pbvh->totvert = totvert;
@ -866,9 +892,6 @@ void BKE_pbvh_build_mesh(PBVH *pbvh,
pbvh->leaf_limit = LEAF_LIMIT;
#endif
pbvh->vdata = vdata;
pbvh->ldata = ldata;
pbvh->pdata = pdata;
pbvh->faces_num = mesh->totpoly;
pbvh->face_sets_color_seed = mesh->face_sets_color_seed;

View File

@ -160,6 +160,7 @@ struct PBVH {
/** Material indices. Only valid for polygon meshes. */
const int *material_indices;
const int *corner_verts;
/* Owned by the #PBVH, because after deformations they have to be recomputed. */
const MLoopTri *looptri;
CustomData *vdata;
CustomData *ldata;