-
-
Notifications
You must be signed in to change notification settings - Fork 135
Open
Description
Hya!
While playing around with this awesome little 3D crate (it's gotta be my favorite in the whole Rust ecosystem for how beautifully stupid simple the interface is), I came across the fact that InstancedMesh::aabb() (
| fn aabb(&self) -> AxisAlignedBoundingBox { |
If you then do something with transparency, you trigger this code:
///
/// Compare function for sorting objects based on distance from the viewer.
/// The order is opaque objects from nearest to farthest away from the viewer,
/// then transparent objects from farthest away to closest to the viewer.
///
pub fn cmp_render_order(
viewer: impl Viewer,
obj0: impl Object,
obj1: impl Object,
) -> std::cmp::Ordering {
if obj0.material_type() == MaterialType::Transparent
&& obj1.material_type() != MaterialType::Transparent
{
std::cmp::Ordering::Greater
} else if obj0.material_type() != MaterialType::Transparent
&& obj1.material_type() == MaterialType::Transparent
{
std::cmp::Ordering::Less
} else {
let distance_a = viewer.position().distance2(obj0.aabb().center());
let distance_b = viewer.position().distance2(obj1.aabb().center());
if distance_a.is_nan() || distance_b.is_nan() {
distance_a.is_nan().cmp(&distance_b.is_nan()) // whatever - just save us from panicing on unwrap below
} else if obj0.material_type() == MaterialType::Transparent {
distance_b.partial_cmp(&distance_a).unwrap()
} else {
distance_a.partial_cmp(&distance_b).unwrap()
}
}
}
That is: the full InstancedMesh aabb is recomputed, for potentially thousands of instances, O(n log n) times in that sorting operation... It turned out to be a bit of a bottleneck 😅
Adding caching for the AABB sped my code up significantly:
pub struct InstancedMesh {
// ... existing fields ...
aabb: AxisAlignedBoundingBox,
cached_instances_aabb: RwLock<Option<AxisAlignedBoundingBox>>, // <- new addition
// ... rest of fields ...
}
I dunno if that fits your architecture so I'm not creating a PR just yet, but it seems to at least be a very useful patch on my end
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels