Details

Reviewers

Brecht Van Lommel (brecht)
Bastien Montagne (mont29)
Sergey Sharybin (sergey)
Dalai Felinto (dfelinto)

Maniphest Tasks

T62595: Select co-planar faces fails

Summary

This avoids copy-pasting the BLI_kdtree API for a 4D version (needed to fix T62595).

The way macros are used here isn't great, alternatives could be to duplicate the code, make the dimensions a property of the tree (at the cost of efficiency) or move it to C++ and use a C-API wrapper.

This includes a fix for T62595 (to show how it works).
BLI_kdtree is currently being used for 2D trees for select similar and edge-net-filling, so the ability to have use different dimensions isn't limited to this case.

Details:

If this is included BLI_kdtree_* should be renamed to BLI_kdtree_3d_* since it's strange to have both BLI_kdtree_new & BLI_kdtree_4d_new.
Could rename 3d/4d -> v3/v4 (named this way because BLI API's have 3d and 2d versions of functions).

Diff Detail

Repository

rB Blender

Branch

TEMP-FIX-T62595-v2 (branched from master)

Build Status

Buildable 3161
Build 3161: arc lint + arc unit

Event Timeline

Campbell Barton (campbellbarton) created this revision.Mar 18 2019, 1:32 AM

Harbormaster completed remote builds in B3149: Diff 14229.Mar 18 2019, 1:32 AM

Correct comment

Harbormaster completed remote builds in B3150: Diff 14230.Mar 18 2019, 1:36 AM

Campbell Barton (campbellbarton) retitled this revision from Shared code for 3D and 4D KD trees to Shared code for BLI_kdtree (3D and 4D).Mar 18 2019, 1:43 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)

Campbell Barton (campbellbarton) added reviewers: Brecht Van Lommel (brecht), Bastien Montagne (mont29), Sergey Sharybin (sergey), Dalai Felinto (dfelinto).

Campbell Barton (campbellbarton) added a project: BF Blender.

Campbell Barton (campbellbarton) added a task: T62595: Select co-planar faces fails.

Remove unused macro

Harbormaster completed remote builds in B3151: Diff 14231.Mar 18 2019, 1:46 AM

Harbormaster completed remote builds in B3151: Diff 14231.

undef utility macros

Harbormaster completed remote builds in B3152: Diff 14232.Mar 18 2019, 1:47 AM

Harbormaster completed remote builds in B3152: Diff 14232.

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Mar 18 2019, 1:48 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Mar 18 2019, 1:55 AM

Correct dimension use in header

Harbormaster completed remote builds in B3153: Diff 14233.Mar 18 2019, 1:58 AM

Harbormaster completed remote builds in B3153: Diff 14233.

Relocate math functions

Harbormaster completed remote builds in B3154: Diff 14234.Mar 18 2019, 2:04 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Mar 18 2019, 2:05 AM

Rename v3/v4 -> 3d/4d

Harbormaster completed remote builds in B3155: Diff 14235.Mar 18 2019, 3:04 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Mar 18 2019, 3:04 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)

Since we don't need to inline the functions, it is probably easier write the kdtree in C++. That would also make it easier to use from other C++ code.

But since the C API will probably stay the same, this can be decided later.

source/blender/editors/mesh/editmesh_select_similar.c
150	Why `mul_transposed_mat3_m4_v3(ob->imat, ...)`. Looks like a double inversion to me (transpose + using `imat`). I think the result should be the same when you use the non transposed version with `ob->obmat`.

Campbell Barton (campbellbarton) added inline comments.Mar 18 2019, 2:26 PM

source/blender/editors/mesh/editmesh_select_similar.c
150	This is needed to get the normal in worldspace. `mul_mat3_m4_v3(obmat)` fails when the matrix has a non-uniform scale. see: `calc_point_from_barycentric_extrusion`, `snapMesh` which does this too.