Use object edit cage to cursor snap.
AbandonedPublic
Actions

Authored by Germano Cavalcante (mano-wii) on Jun 26 2018, 2:51 AM.

Details

Reviewers

Campbell Barton (campbellbarton)
Adam Latchem (adamlatchem)
Inês Almeida (brita_)

Summary

This is the proposed fix for the T55202.
The main changes are:

Use the edit cage of all edited meshes to snap/project the cursor.
Stores the bvhtree of the edited mesh (without masking) in the Mesh bvh_cache;

More technical changes:

The key used in Ghash object_map is now the Mesh or BMEditMesh pointer instead of the Object pointer;
A temporary bound box is created for all edited meshes in order to avoid having to create a bvhtree for all of them;

Just to note, this changes the original behavior of the cursor projection in pre 2.80 versions that was made on the object with modifiers (even in edit mode).

Diff Detail

Repository

rB Blender

Branch

blender2.8_use_edit_cage_to_snap_cursor (branched from blender2.8)

Build Status

Buildable 1780
Build 1780: arc lint + arc unit

Event Timeline

Germano Cavalcante (mano-wii) created this revision.Jun 26 2018, 2:51 AM

@Germano Cavalcante (mano-wii) looks good - I see th emove to the edit cage in there. There was obviously a wider change to move the bvhtree cache to the mesh object.

I can't find the blender2.8_use_edit_cage_to_snap_cursor branch or the commit SHA so I must have a different repo cloned locally sand can't try running the above for you; looking at the diff at transform_snap_object.c:2028 you declare a local Mesh* me. There is also a local em variable which is unfortunate as the two are easily typo'd.

I am guessing the use of em not me at line 2033 is correct so perhaps move the declaration of me lower so its clear you don't want to use it until then? Otherwise I can't usefully comment :)

For click projecting I think this should not use the cage (project to surface), for vertex/edge/face snapping this makes sense.

Would rather not have multiple kinds of keys in GHash *object_map; - seems a bit error prone.

Can't we use only objects and use SnapObjectData.type to check which kind of data needs to be used?
Since the edit-mesh can be accessed from the object.

This revision is now accepted and ready to land.Jun 27 2018, 10:51 PM

Campbell Barton (campbellbarton) requested changes to this revision.Jun 28 2018, 9:57 AM

This revision now requires changes to proceed.Jun 28 2018, 9:57 AM

Campbell Barton (campbellbarton) added inline comments.Jun 28 2018, 10:22 AM

source/blender/editors/transform/transform_snap_object.c
113	It should be noted here why the min/max is needed (not just in patch description).

In D3504#80950, @Campbell Barton (campbellbarton) wrote:

(...)
Would rather not have multiple kinds of keys in GHash *object_map; - seems a bit error prone.

Can't we use only objects and use SnapObjectData.type to check which kind of data needs to be used?
Since the edit-mesh can be accessed from the object.

Now that multiple objects can be edited at the same time, objects with different data can be linked to some of the edited objects. In these cases different objects may have the same EditMesh, but the bvhtree has to be created by EditMesh and not by object, (thus avoiding unnecessary duplications).
Without the patch, the active object is used in place of each object with the same data (linked). This is not consistent with the editing of multiple objects.

Move the local Mesh declaration to a line closer to its use (facilitating readability).
Add description for the new min and max members.
Use the object as key in Ghash.

Germano Cavalcante (mano-wii) marked an inline comment as done.Jul 4 2018, 5:47 AM

LGTM, however on testing there is a problem where the BVH isn't getting invalidated.

Default file
Enter EditMode
Snap to vertex
Drag Cursor, Hold Ctrl (it snaps)
Select Cube and move it a bit.
Drag Cursor, Hold Ctrl (it snaps to old BVH or not at all)

Check uses of BKE_editmesh_free_derivedmesh, the places that call this could also free the bvhCache, especially EDBM_update_generic which will cover nearly all cases.

Germano Cavalcante (mano-wii) mentioned this in rB59286ddcf80c: transform_snap_object: Better bvhtree creation management for editing multiple….Jul 23 2018, 4:14 PM

Inês Almeida (brita_) accepted this revision.Jul 27 2018, 11:08 PM

Inês Almeida (brita_) added inline comments.