Page MenuHome

Fix T54526: Data Transfer Max Distance field working strangely
Needs ReviewPublic

Authored by Philipp Oeser (lichtwerk) on Jul 9 2019, 12:02 PM.

Details

Summary

BVHTreeNearest index wasnt reset to -1.
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.

Note that it might very well possible to reset 'nearest->index' to -1 in
'BLI_bvhtree_find_nearest_ex' even (since most code I've checked does
this prior to calling anyways) but being close to RC it might be better
to play safe for now...

Diff Detail

Repository
rB Blender
Branch
T54526 (branched from master)
Build Status
Buildable 4000
Build 4000: arc lint + arc unit

Event Timeline

Alexander Gavrilov (angavrilov) added inline comments.
source/blender/blenkernel/intern/mesh_remap.c
64

Considering the purpose of the heuristic, I actually think the problem is this min_ff here. Without it, the search would find the actual nearest point with the actual distance (or keep this one if there is nothing), and would then fail on the dist_sq check in the if below. This min basically causes the default result to lie about its distance to the check.

The idea was probably something along the lines of avoiding to look through points that are closer than the initial point but further than the distance limit, but since this actually means the initial point is invalid too. I'd rather do something like:

nearest->dist_sq = len_squared_v3v3(co, nearest->co);

if (nearest->dist_sq > max_dist_sq) {
  nearest->dist_sq = max_dist_sq;
  nearest->index = -1;
}

Considering ideas for redesign of the API, this use of dist_sq to impose a distance limit is unclear (feels like a hack) and possibly should be replaced with a separate explicit parameter to find_nearest, so that the 'nearest' structure always represents a valid result when filled. With a new BVH_NEAREST_REUSE_RESULT flag to control it, this whole bit of code at the start of this function could then potentially be folded into BLI_bvhtree_find_nearest_ex.