blender
/
blender
136
406
Fork 589
Code Issues 6.3k Pull Requests 766 Projects 19 Wiki Activity

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
Conversation 1 Commits 11 Files Changed 4 +25 -41
3 changed files with 25 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 71e1654fb9 - Show all commits

9
source/blender/blenkernel/BKE_pbvh.h
View File

@ -286,14 +286,7 @@ PBVH *BKE_pbvh_new(PBVHType type);
* \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,
const struct MPoly *polys,
const int *corner_verts,
float (*vert_positions)[3],
int totvert,
const struct MLoopTri *looptri,
int looptri_num);
void BKE_pbvh_build_mesh(PBVH *pbvh, struct Mesh *mesh);
/**
* Do a full rebuild with on Grids data structure.
*/

20
source/blender/blenkernel/intern/paint.cc
View File

@ -2182,27 +2182,11 @@ 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);
Hans Goudey commented 2023-04-07 22:55:02 +02:00
Review
Copy Link

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.
Jacques Lucke commented 2023-04-08 12:47:32 +02:00
Review
Copy Link

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.
BKE_pbvh_respect_hide_set(pbvh, respect_hide);
MutableSpan<float3> positions = me->vert_positions_for_write();
const Span<MPoly> 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.data(),
corner_verts.data(),
reinterpret_cast<float(*)[3]>(positions.data()),
me->totvert,
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) {

37
source/blender/blenkernel/intern/pbvh.cc
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,39 @@ static void pbvh_validate_node_prims(PBVH *pbvh)
}
#endif
void BKE_pbvh_build_mesh(PBVH *pbvh,
Mesh *mesh,
const MPoly *polys,
const int *corner_verts,
float (*vert_positions)[3],
int totvert,
const MLoopTri *looptri,
int looptri_num)
void BKE_pbvh_build_mesh(PBVH *pbvh, Mesh *mesh)
{
BBC *prim_bbc = nullptr;
BB cb;
const int looptri_num = poly_to_tri_count(mesh->totpoly, mesh->totloop);
MutableSpan<float3> positions = mesh->vert_positions_for_write();
const Span<MPoly> polys = mesh->polys();
JacquesLucke marked this conversation as resolved Outdated
Hans Goudey commented 2023-04-07 22:58:13 +02:00
Outdated
Review
Copy Link

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).
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(positions, polys, corner_verts, {looptri, looptri_num});
pbvh->mesh = mesh;
pbvh->header.type = PBVH_FACES;
pbvh->polys = polys;
pbvh->polys = polys.data();
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;
pbvh->corner_verts = corner_verts.data();
pbvh->looptri = looptri;
pbvh->vert_positions = vert_positions;
pbvh->vert_positions = reinterpret_cast<float(*)[3]>(positions.data());
/* 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;
MEM_calloc_arrayN(mesh->totvert, sizeof(bool), "bvh->vert_bitmap"));
pbvh->totvert = mesh->totvert;
#ifdef TEST_PBVH_FACE_SPLIT
/* Use lower limit to increase probability of
@ -884,7 +891,7 @@ void BKE_pbvh_build_mesh(PBVH *pbvh,
BB_reset((BB *)bbc);
for (int j = 0; j < sides; j++) {
BB_expand((BB *)bbc, vert_positions[pbvh->corner_verts[lt->tri[j]]]);
BB_expand((BB *)bbc, positions[pbvh->corner_verts[lt->tri[j]]]);
}
BBC_update_centroid(bbc);
@ -905,7 +912,7 @@ void BKE_pbvh_build_mesh(PBVH *pbvh,
MEM_freeN(prim_bbc);
/* Clear the bitmap so it can be used as an update tag later on. */
memset(pbvh->vert_bitmap, 0, sizeof(bool) * totvert);
memset(pbvh->vert_bitmap, 0, sizeof(bool) * mesh->totvert);
BKE_pbvh_update_active_vcol(pbvh, mesh);