Differential D6927
Transform Snap: Clear 'SnapObjectData' after changes in the geometry Authored by Germano Cavalcante (mano-wii) on Feb 24 2020, 11:25 PM.
Details With tasks like T73993 and D6678, there is a need to use the snap system in Gizmos. The solution is to use the em->mesh_eval_... bvh_cache and clear the SnapObjectData cache when the object mode changes.
Diff Detail
Event TimelineComment Actions It does sounds like a hack. Instances don't have DrawData for examples. How does this patch is expected to work in this case? Comment Actions Yes, the original code always takes the original object instead of the instances. It only takes the reference of their matrices. Comment Actions This sounds like something to put in Object_Runtime, and free in BKE_object_free_derived_caches. Such data will only be freed on ID_RECALC_GEOMETRY, see ObjectRuntimeBackup::restore_to_object. Comment Actions There are bvhcache_has_tree checks, why not clear the BVH tree when the mesh is tagged for updating? Blender 2.7x did this in BKE_object_free_derived_caches via DM_release. We could add this back e.g: diff --git a/source/blender/blenkernel/intern/object.c b/source/blender/blenkernel/intern/object.c index 51d397a44bc..a63e24453ad 100644 --- a/source/blender/blenkernel/intern/object.c +++ b/source/blender/blenkernel/intern/object.c @@ -452,6 +452,11 @@ void BKE_object_free_derived_caches(Object *ob) BKE_object_to_mesh_clear(ob); BKE_object_free_curve_cache(ob); + if (ob->type == OB_MESH) { + /* Used for edit-mode BVH tree, ensure we don't reuse it. */ + Mesh *me = ob->data; + bvhcache_free(&me->runtime.bvh_cache); + } /* clear grease pencil data */ DRW_gpencil_freecache(ob); } If this isn't an option for some reason, I'd still rather not use the method proposed in this patch. It moves away from having the snap context being a view on objects in the scene, to being part of the objects. This complicates having different kinds of snapping data, for e.g. one snap context may want to have edit-mesh cage data, another context may want to perform operations on the final meshes. Having g_snap looks like it would complicate having multiple snap contexts at once too, although I didn't look into this in detail. If all we need to do is know if the dependency graph tagged the object to be updated, we could have a generic way to handle that. e.g: P1275 (untested) (we could make this more generic - reuse depsgraph so users can check for other kinds of updates). Comment Actions
bvhcache_free is already called in BKE_mesh_runtime_clear_geometry. And bvhcache_has_tree is still called in the snap code (although with this patch it would no longer be necessary). In the bvh_cache code, each type of tree has an identifier (BVHTREE_FROM_VERTS, BVHTREE_FROM_EDGES ...), I cannot use one of these identifiers for these custom trees. In the future we could also cache bvhtrees for other types of objects (like Surface), It would be necessary to think of a solution similar to this patch.
For final_meshes not really a problem, each mesh has its own bvh_cache.
g_snap is just a way to clear all snap_caches when they are no longer used. Releases bvhtrees not referenced in bvh_cache.
It seems to be a good solution (it would be somewhat more generic), although it would not solve the problem of identical custom bvhtree that was mentioned. Comment Actions
Okay, I found a much simpler solution. Comment Actions This seems reasonable to me, but would rather @Campbell Barton (campbellbarton) does a final check since I think he understands this better. Comment Actions
The previous solution didn't work and we need to recalculate sod->min and sod->max. |