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

Problem using bvhvec4slice with float3 data #97

Open
jkoenen opened this issue Jan 25, 2025 · 1 comment
Open

Problem using bvhvec4slice with float3 data #97

jkoenen opened this issue Jan 25, 2025 · 1 comment

Comments

@jkoenen
Copy link

jkoenen commented Jan 25, 2025

I think the idea of bvhvec4slice is that you can pass in a non-aligned float3 pointer + set stride to 12 - unfortunately this doesn't work because of how bvhvec4slice::operator[] is implemented - it does a reinterpret cast of the data into a bvhvec4 reference - which allows the compiler to create an aligned load (which msvc does) - that unfortunately crashes because it's non-aligned data.

One potential fix is to return a new bvhvec4 by value from bvhvec4slice::operator[]:

bvhvec4 bvhvec4slice::operator[]( size_t i ) const
{
#ifdef PARANOID
	FATAL_ERROR_IF( i >= count, "bvhvec4slice::[..], Reading outside slice." );
#endif
	const float* pData = reinterpret_cast<const float*>(data + stride * i);
	return bvhvec4( pData[ 0u ], pData[ 1u ], pData[ 2u ], 0.0f );
}

I didn't look at the code gen too closely - maybe it's better to call memcpy (and let the compiler figure it out). For my use case I would prefer to use float3 data because of memory sizes - this is more important in this case than the performance benefit. It would be great if the user of tinybvh can choose to use float3 or float4 values.

@jbikker
Copy link
Owner

jbikker commented Jan 27, 2025

The float4 layout is primarily there for the BuildAVX construction code, which really needs data to be aligned to 16 byte boundaries to be efficient. That means that even if you use a slice, each xyz must start on a multiple of 4 elements, which is the case if you have per vertex (x, y, z, u, v, nx, ny, nz).
That being said, the regular builder (and perhaps the HQ builder) could definitely work with float3 data. I will see how this could be incorporated without breaking other things. This will probably require disabling SIMD functionality altogether, so this will not be the default behavior.

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