WIP of T92157 (seems like I changed more in this node than I originally intended)
Wanted to further discuss. Since we're only using real geometry for min/max output, what's the expected values for incoming geometry that only contains instances?
Details
Details
- Reviewers
- None
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- bb_instances (branched from master)
- Build Status
Buildable 17899 Build 17899: arc lint + arc unit
Event Timeline
Comment Actions
I think the node should only add an instance to the output for the first level of a geometry's instances, rather than recursively like it does currently.
What I'm not sure about is whether it should consider the instances of those instances in the bounding box. I think it would be most intuitive/useful if it did.
Let me know if I should clarify that.
Since we're only using real geometry for min/max output, what's the expected values for incoming geometry that only contains instances?
I think it's 0,0 and a node warning, if we want to be consistent. I think with this behavior it will be easiest when we support attributes on instances and want to update the node to output an anonymous attribute with the values for each instance.
| source/blender/nodes/geometry/nodes/node_geo_bounding_box.cc | ||
|---|---|---|
| 40 | I would rather the code didn't make this sort of abbrieviations-- "trs", "bb". It may be a bit quicker to write, but IMO it's harder to read. | |
| 50 | blender::Vector should almost always be used instead of std::vector in Blender code. There are more details in its header file. | |
| 127 | This could be a bit simpler, using the constructor of InstanceReference right here: const int handle = instances.add_reference(GeometrySet::create_with_mesh(mesh)); | |
Comment Actions
Thanks for the patch Jarrett. I think the approach here is a little more complicated than necessary, and I think you mentioned you wouldn't have time to continue this in chat, so I continued work on this task here: D12951