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

BVH: Optimize preparation of geometry data. #19

Closed
Glidias opened this issue Feb 18, 2020 · 3 comments
Closed

BVH: Optimize preparation of geometry data. #19

Glidias opened this issue Feb 18, 2020 · 3 comments

Comments

@Glidias
Copy link

Glidias commented Feb 18, 2020

There are some minor side issues like the constructor parameters documentation ordering for BVH.js doesn't match the ordering of the function parameters.

Also, from fromMeshGeometry() method could consider additional approaches (in the event of already referencing non-indexed geometry for the parameter, to avoid cloning the Meshgeometry via the toPolygonSoup() call). I understand that cloning does provide stability in case the original mesh geometry may had been altered via some other apps, but I guess seperating out fromMeshGeometry() into two seperate functions (.. ie. another function that avoids calling toPolygonSoup() cloning/conversion process, may be considered to allow bypassing additional cloning for certain application cases.

@Mugen87
Copy link
Owner

Mugen87 commented Feb 18, 2020

There are some minor side issues like the constructor parameters documentation ordering for BVH.js doesn't match the ordering of the function parameters.

You're right, primitivesPerNode and depth are mixed. Fixed here 96e6e9f. I'll updating the website later.

MeshGeometry.toTriangleSoup() is currently only called in BVH. How about BVH.fromMeshGeometry() only performs the conversion if the geometry is indexed. In this way, the caller can pass in a non-indexed geometry and does not trigger further processing overhead. Something like:

if ( geometry.indices !== null )  geometry = geometry.toTriangleSoup();

I would not worry about the fact that other app logic might change the vertex data. I don't think this is a problem here.

@Mugen87 Mugen87 changed the title BVH minor issues for parameters documentation/options BVH: Optimize preparation of geometry data. Feb 18, 2020
@Glidias
Copy link
Author

Glidias commented Feb 19, 2020

That's what i did on my end as well. Oh, it looks like you are refering a new primitives = new Array() clone anyway. So, it wouldn't matter even if the original data was mutated..

Anyway, for real-time performance critical stuff, I actually use another BVH implementation (benraziel's) that uses only a single densely packed monolith typed array on the tree itself, while each BVHNOde only uses startIndex to endIndex approach.

@Mugen87
Copy link
Owner

Mugen87 commented Feb 19, 2020

Solved via 328e3f1.

@Mugen87 Mugen87 closed this as completed Feb 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants