blender
/
blender
136
411
Fork 595
Code Issues 6.3k Pull Requests 770 Projects 19 Wiki Activity

Geometry Node: Index of Nearest #104619

Merged
Jacques Lucke merged 31 commits from mod_moder/blender:index_of_nearest into main 2023-04-22 13:12:03 +02:00
Conversation 7 Commits 31 Files Changed 8 +5 -5
2 changed files with 5 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit a5272482aa - Show all commits

8
source/blender/blenlib/BLI_kdtree_impl.h
View File

@ -107,10 +107,10 @@ inline void BLI_kdtree_nd_(range_search_cb_cpp)(const KDTree *tree,
}
template<typename Fn>
inline int BLI_kdtree_nd_(find_nearest_cb)(const KDTree *tree,
const float co[KD_DIMS],
KDTreeNearest *r_nearest,
const Fn &fn)
inline int BLI_kdtree_nd_(find_nearest_cb_cpp)(const KDTree *tree,
mod_moder marked this conversation as resolved Outdated
Hans Goudey commented 2023-04-04 23:38:52 +02:00
Outdated
Review
Copy Link

Might as well be consistent with the other function here and use the _cpp suffix too.

Might as well be consistent with the other function here and use the `_cpp` suffix too.
😆 1
const float co[KD_DIMS],
KDTreeNearest *r_nearest,
const Fn &fn)
Jacques Lucke commented 2023-04-21 13:05:21 +02:00
Outdated
Review
Copy Link

Can just use Fn &&fn, then you also don't need the const cast below I think.

Can just use `Fn &&fn`, then you also don't need the const cast below I think.
Iliya Katushenock commented 2023-04-21 13:50:20 +02:00
Outdated
Review
Copy Link

Should i also change one other _cpp wrapper above?

Should i also change one other _cpp wrapper above?
Jacques Lucke commented 2023-04-21 13:55:44 +02:00
Outdated
Review
Copy Link

Just keep other code as is right now.

Just keep other code as is right now.
👍 1
{
return BLI_kdtree_nd_(find_nearest_cb)(
tree,

2
source/blender/nodes/geometry/nodes/node_geo_index_of_nearest.cc
View File

@ -111,7 +111,7 @@ class IndexOfNearestFieldInput final : public bke::GeometryFieldInput {
protected:
mod_moder marked this conversation as resolved Outdated
Jacques Lucke commented 2023-04-21 13:15:33 +02:00
Outdated
Review
Copy Link

While forest is used as a technical term for a graph containing multiple trees, I don't think the term should be used for a collection of multiple independent kd trees. Just use kdtrees.

While forest is used as a technical term for a graph containing multiple trees, I don't think the term should be used for a collection of multiple independent kd trees. Just use `kdtrees`.
👍 1
Iliya Katushenock commented 2023-04-21 13:39:08 +02:00
Outdated
Review
Copy Link

If i do merge parallel_fors below, i can delete this vector.

If i do merge `parallel_for`s below, i can delete this vector.
int kdtree_find_neighboard(KDTree_3d *tree, const float3 &position, int index) const
{
return BLI_kdtree_3d_find_nearest_cb(
return BLI_kdtree_3d_find_nearest_cb_cpp(
mod_moder marked this conversation as resolved Outdated
Hans Goudey commented 2023-04-21 14:54:21 +02:00
Outdated
Review
Copy Link

I think this should be !mask_is_cheap

I think this should be `!mask_is_cheap`
Iliya Katushenock commented 2023-04-21 14:58:40 +02:00
Outdated
Review
Copy Link

I was isn't invented the best name, mask_is_cheap == true if computing of indices make sense and cheap mask is used.

I was isn't invented the best name, `mask_is_cheap` == true if computing of indices make sense and cheap mask is used.
tree,
Hans Goudey commented 2023-04-21 14:21:21 +02:00
Outdated
Review
Copy Link

TBH I think it's better to just use int64_t here to avoid duplicating the functions above. Eventually when IndexMask is refactored, these could benefit from using that, and the int64_t will make that more clear too.

TBH I think it's better to just use `int64_t` here to avoid duplicating the functions above. Eventually when `IndexMask` is refactored, these could benefit from using that, and the `int64_t` will make that more clear too.
position,
mod_moder marked this conversation as resolved Outdated
Hans Goudey commented 2023-04-21 14:18:39 +02:00
Outdated
Review
Copy Link

Vector -> Array here

`Vector` -> `Array` here
😆 1
0,