Skip to content

Conversation

@oliver-dew
Copy link
Contributor

@oliver-dew oliver-dew commented Dec 17, 2025

Objective

Solution

  • Mesh::take_gpu_data was calling Aabb3d::new and passing it min and max, but the constructor expected center and half size. This caused a panic for meshes where the vertices are all negative on any of the axes. And also means the aabb is incorrect, maybe causing issues like incorrect frustum culling? After a bit of discussion on Discord, the new constructor expecting center & half extents was deemed footgunny. So this PR renames Aabb3d::new to Aabb::from_half_size, and adds a new constructor Aabb::from_min_max. Mesh::take_gpu_data now calls from_min_max, fixing the AABB calculation bug.

Testing

  • I hit this bug with a model where all the vertices were negative in one dimension (so the max is all negative, and when it was wrongly passed as the half size, fails the assertion that half size is >= 0). Confirmed that this change fixes the issue.
  • This PR adds a unit test mimicking the above scenario, that panics without the fix
  • How can other people (reviewers) test your changes? They could test it with a model whose max is less than zero in one dimension, or run the unit test
  • Tested on macOS

@github-actions
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@oliver-dew oliver-dew closed this Dec 17, 2025
@oliver-dew
Copy link
Contributor Author

Closed in favour of #22180

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant