Forceflow / cuda_voxelizer

CUDA Voxelizer to convert polygon meshes into annotated voxel grids
MIT License
595 stars 98 forks source link

Is it a bug in src/main.cpp ? #59

Closed ndroi closed 2 years ago

ndroi commented 3 years ago

size_t vtable_size = static_cast<size_t>(ceil(static_cast<size_t>(voxelization_info.gridsize.x) * static_cast<size_t>(voxelization_info.gridsize.y) * static_cast<size_t>(voxelization_info.gridsize.z)) / 8.0f)

Should we ceil it after '/ 8.0f' ?

Forceflow commented 3 years ago

Yeah, I kind of implicitly expect the grid size to be a power of 2.

But yes, in order to avoid invalid memory writes in case the model fills the last byte, a ceil() would definitely be safest here

ndroi commented 3 years ago

Thanks for answering me. I found another bug when gridsize.x, gridsize.y and gridsize.z unequal . About the loaction expr, size_t location = x + (y*gridsize.y) + (z*gridsize.y*gridsize.z);, I guess it should be size_t location = x + (y*gridsize.x) + (z*gridsize.x*gridsize.y);

Forceflow commented 3 years ago

Do you want to bundle these changes into a PR or should I incorporate them?

Thanks a bunch

ndroi commented 3 years ago

I will make a PR.

Forceflow commented 2 years ago

Merged in e778b122a42f06a320fd353e337acb7002ac4dea

Thanks! Will be included in next non-dev release.