Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BHV::Optimize is leaky? #130

Open
DavidPeicho opened this issue Feb 28, 2025 · 2 comments
Open

BHV::Optimize is leaky? #130

DavidPeicho opened this issue Feb 28, 2025 · 2 comments

Comments

@DavidPeicho
Copy link
Contributor

Hey! I have seen that BVH::Optimize is now a thing to shortcut the creation of verbose/optimization/conversion back.

The verbose pointer is allocated but never freed. I think it's probably fair to allocate verbose on the stack, convert, and drop it? If someone wants to keep the verbose layout around, they will need to write the conversion themselves (using Optimize, ConvertFrom, etc...).

@DavidPeicho
Copy link
Contributor Author

I feel like the conversion API is missing a slight differentiation between:

  • Moved data: The original BVH can be dropped after the converson
  • Referenced data

This could be "as simple" as having a ConvertFrom that works on r-value references and one on references:

// The verbose one takes ownership
BVH_Verbose::ConvertFrom(BVH&& original);
// Not owned, but then `BVH` must live as long as `BVH_Verbose`.
BVH_Verbose::ConvertFrom(const BVH& original);

@jbikker
Copy link
Owner

jbikker commented Mar 3, 2025

How would the user differentiate between the two? E.g., what if a conversion for one mesh needs to happen every frame, while another mesh is constructed (and converted) only once? This is expected behavior for GPU BVHs.

Right now I was assuming the 'repeated conversion' scenario, with the 'one shot conversion' as exception, indicated by e.g. a call to 'Compact' (not implemented properly yet). This would also trim all buffers, sort node data for cache coherence and so on.

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

No branches or pull requests

2 participants