Implement basic ray intersection testing#8
Conversation
|
Hi, thanks for the kind words and for this PR! Sorry it took me that long to respond, had a lot on my plate lately. I did a quick review and I'm impressed, great job! Major issue:
error[E0599]: no method named `min_position` found for struct `Vec3` in the current scope
--> voxelis/src/world/voxchunk.rs:514:31
|
514 | let i = next_dist.min_position();
| ^^^^^^^^^^^^ method not found in `Vec3`
error[E0308]: mismatched types
--> voxelis/src/world/voxchunk.rs:523:31
|
422 | impl<T: VoxelTrait> VoxOpsRayIntersection<T> for VoxChunk<T> {
| - expected this type parameter
...
523 | if voxel_pos[i] < 0 || voxel_pos[i] >= voxels_per_side {
| ^ expected type parameter `T`, found integer
|
= note: expected type parameter `T`
found type `{integer}`
error[E0308]: mismatched types
--> voxelis/src/world/voxchunk.rs:523:52
|
422 | impl<T: VoxelTrait> VoxOpsRayIntersection<T> for VoxChunk<T> {
| - expected this type parameter
...
523 | if voxel_pos[i] < 0 || voxel_pos[i] >= voxels_per_side {
| ^^^^^^^^^^^^^^^ expected type parameter `T`, found `i32`
|
= note: expected type parameter `T`
found type `i32`Minor issues:
After fixing the glam issue I can merge it as is, or maybe you want to fix minor issues too? :) |
|
Thank you for the review! No worries about the delay, the end of the year is always extremely busy. I think it will be easier to iterate on the patch in place, given that most of the minor issues are small.
As to the glam build errors, you are quite correct! At some point I got tired of adding |
|
Okay, so using a version dep for glam of ">= 0.29" does allow users to select a higher version; however, since vtm-viewer is in the same workspace, it breaks again if you ever run a I've seen two different mechanisms in the wild for how projects support multiple bevy versions, either of which we could adopt pretty easily.
My preference would be option (1), but I'd be happy to implement any of them. @aljen, which of those options would the project prefer? Regardless, for now, I think the first step is going to be to land this patch against bevy 0.16 and glam 0.29 and upgrade from there. I'll get to work on fixing up the minor nits. |
This should further minimize precision issues and is cheaper to boot.
|
I see what you mean now about fixing the precision issue: if we accumulate on And now that I've pointed that out, I think the |
|
The test was (as always) a good idea: I found an edge case I hadn't thought about and manually verified that t_hit is correct. I think I'm happy with the current state now and am ready to land. Thanks again for the helpful review! |
|
Nice progress! I'll try to review it today :) And sorry about the glam confusion. Instead of worrying about it breaking the build, I should just port to bevy 0.17 and move on. Lost track that it was released, my bad 😅 |
|
Oh! I probably would have just updated vtm-viewer in this patch if I'd known. I assumed you had other code behind the scenes that was on 0.16 still. I've had a couple "I'll just bump Bevy's version this morning before I get to the real work" turn into weeks long slogs before, so I'd hate to pressure someone to rush an update! |
Thank you for building and publishing Voxelis: it's a fantastic piece of technology! I saw in the PDF that ray-intersection is missing, so went ahead and built it. The new ray_cast method is now successfully powering visibility testing to load an infinite world in my bevy project.
The code is currently a very basic implementation of Amanatides-Woo ray/voxel intersection. I left it basic for the first pass to get some feedback before pushing further. The singular new VoxChunk method is the most flexible potential variant, supporting arbitrary rays and returning full collision data suitable for physics or rendering.
Currently, testing a ray against a MaxDepth(5)/LOD(0) chunk takes about 90ns on my machine and scales to 160ns for a MaxDepth(6)/LOD(0) chunk. If we modify the algorithm to only support internal rays and return only a hit boolean, we can shave about 20ns off of those numbers. Further optimization could potentially take advantage of the LOD tree directly, however, given that the current algorithm has a max of O(3*64) steps, it's hard to imagine that more complexity will pay for itself easily.
I would appreciate any feedback and am more than happy to make improvements to get this merged.