Page MenuHome
Maniphest T54526

Data Transfer modifier's Max Distance field working strangely
Open, Confirmed, MediumPublic
Actions

Description

Using Blender 2.89b. EVGA GTX1070. Win10. All non-default add-ons disabled and new scene loaded.

Attached demonstration file contains a cube using a data transfer with max distance enabled to copy weights from a nearby plane, and an armature for that cube. Max Distance settings should put four verts in the domain of the modifier; as you can see, seven verts are affected.

Problem encountered in less abstract situations as well, this is just my simplest reduction of the problem in order to verify that it's an issue and make it plain to see and reproduce.

For reference, official documentation reads, "This allows to transfer a small sub-detailed mesh onto a more complete one (e.g. from a “hand” mesh towards a “full body” one)," which would be useful if it were working properly.

DataDistance1.blend472 KBDownload

Details

Type
Bug

Related Objects

Event Timeline

Philipp Oeser (lichtwerk) triaged this task as Confirmed, Medium priority.Apr 5 2018, 10:41 AM
Philipp Oeser (lichtwerk) added a subscriber: Philipp Oeser (lichtwerk).

Confirming on first sight, having a closer look now...

Philipp Oeser (lichtwerk) claimed this task.Apr 9 2018, 10:16 AM
Philipp Oeser (lichtwerk) added a subscriber: Bastien Montagne (mont29).

I'll claim for the time being and do some further investiagtion. If that doesnt succeed, I'll get @Bastien Montagne (mont29) on board to help me out...

Nathan Vasil (vasiln) added a comment.Jan 30 2019, 6:23 PM

Coming back to add that there's a reasonable workaround, which is to use a vertex weight proximity modifier to determine the distance from each vertex before the weight transfer, use a custom (constant) curve to modulate that vertex group, and then use it to modulate the data transfer. It's a bit more work for the end user, but perfectly reasonable, and it wouldn't seem unreasonable to just drop support for max distance from the data transfer.

Philipp Oeser (lichtwerk) added a subscriber: Alexander Gavrilov (angavrilov).Tue, May 7, 7:53 PM

Had a look at this again and here are some findings:

  • BKE_mesh_remap_calc_verts_from_mesh / mesh_remap_bvhtree_query_nearest is where things happen
  • in this loop, code keeps the previous "nearest" [but doesnt reset nearest.index to -1]
  • if BLI_bvhtree_find_nearest() is called with a "nearest" provided, code will not update anything on "nearest" (esp. not index, not dist_sq) if it fails to find something.
  • so the comment here is actually misleading I think, index is not neccessarily -1 if it fails to find something.
  • if we have previously found something [making nearest.index non-zero], and then for the next vertex dont find anything, we still have a non-zero index leading to false positives in the following code.

It actually works fine if the index is reset to -1 for each vert.
Could be like this (would have to be done for a couple of other loops as well):

P967 T54526_snippet_1
1
2
3diff --git a/source/blender/blenkernel/intern/mesh_remap.c b/source/blender/blenkernel/intern/mesh_remap.c
4index db158ca8fb2..c1dd62d2719 100644
5--- a/source/blender/blenkernel/intern/mesh_remap.c
6+++ b/source/blender/blenkernel/intern/mesh_remap.c
7@@ -526,9 +526,12 @@ void BKE_mesh_remap_calc_verts_from_mesh(const int mode,
8
9 if (mode == MREMAP_MODE_VERT_NEAREST) {
10 BKE_bvhtree_from_mesh_get(&treedata, me_src, BVHTREE_FROM_VERTS, 2);
11- nearest.index = -1;
12+ //nearest.index = -1;
13
14 for (i = 0; i < numverts_dst; i++) {
15+ /* resetting index here would work */
16+ nearest.index = -1;
17+
18 copy_v3_v3(tmp_co, verts_dst[i].co);
19
20 /* Convert the vertex to tree coordinates, if needed. */

Or more general like:
P968 T54526_snippet_2
1
2
3diff --git a/source/blender/blenkernel/intern/mesh_remap.c b/source/blender/blenkernel/intern/mesh_remap.c
4index db158ca8fb2..fb4ce68fed3 100644
5--- a/source/blender/blenkernel/intern/mesh_remap.c
6+++ b/source/blender/blenkernel/intern/mesh_remap.c
7@@ -67,6 +67,7 @@ static bool mesh_remap_bvhtree_query_nearest(BVHTreeFromMesh *treedata,
8 nearest->dist_sq = max_dist_sq;
9 }
10 /* Compute and store result. If invalid (-1 index), keep FLT_MAX dist. */
11+ nearest->index = -1;
12 BLI_bvhtree_find_nearest(treedata->tree, co, nearest, treedata->nearest_callback, treedata);
13
14 if ((nearest->index != -1) && (nearest->dist_sq <= max_dist_sq)) {

@Alexander Gavrilov (angavrilov), @Bastien Montagne (mont29): what do you think?

Bastien Montagne (mont29) added a comment.Wed, May 8, 3:58 PM

@Philipp Oeser (lichtwerk) nice catch. Wouldn’t it be even simpler to always reset that nearest->index directly in BLI_bvhtree_find_nearest_ex()? Keeping previous nearest distance there if provided makes sense, but I think item index should be always reset, at least I cannot see a case where keeping previous value would be useful?