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

[GodotPhysics] Fix raycasts don't reliably collide with HeightMapShape3D #97471

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

kevinkuo52
Copy link
Contributor

@kevinkuo52 kevinkuo52 commented Sep 26, 2024

Tested this fix on both of the MRPs shared and it seems to work
heightmap_raycast_repro.zip
raycastHeightmapBug.zip

If there's cleaner way to fix this, or if I missed something, let me know thanks

Details:
Raycast with issue fall under this case where ray is long so it does query on higher level grid first

return _intersect_grid_segment(_heightmap_chunk_cull_segment, bounds_from, bounds_to, bounds_grid_width, bounds_grid_depth, bounds_offset, r_point, r_normal);

And issue looks to be in the loop here, it exit before it got to the chunk where the ray actually intersect
if ((x < 0) || (z < 0) || (x >= p_width - 1) || (z >= p_depth - 1)) {
break;

p_width and p_depth is the chunk's width and depth for this flow
using heightmap_raycast_repro.zip as example
p_width = ceil( width/chunk size) = ceil( (21/16) = 2
p_depth = ceil(depth/chunk size) = ceil( (21/16) = 2
x = 1 or z = 1 should still be valid chunk, but it will exit here.

But can't directly change that line to if ((x < 0) || (z < 0) || (x >= p_width) || (z >= p_depth))
because _intersect_grid_segment() is used by cell level too, where in _heightmap_cell_cull_segment:

p_params.heightmap->_get_point(p_state.x + 1, p_state.z, p_params.face->vertex[1]);
p_params.heightmap->_get_point(p_state.x, p_state.z + 1, p_params.face->vertex[2]);

it performs get point on the triangle with p_state.x + 1, p_state.z + 1 etc so it will hit out of range on the edge cell if we change that line

This PR passes bounds_grid_width + 1, bounds_grid_depth + 1 to _intersect_grid_segment() with a comment to explain this,
if there's cleaner way let me know 😅

…h HeightMapShape3D, by fixing the value bounds_grid_width and bounds_grid_depth passed
@kevinkuo52 kevinkuo52 marked this pull request as draft September 26, 2024 01:15
@kevinkuo52 kevinkuo52 marked this pull request as ready for review September 26, 2024 01:47
@Chaosus Chaosus changed the title Fix #68238 [GodotPhysics] Raycasts don't reliably collide with HeightMapShape3D [GodotPhysics] Fix raycasts don't reliably collide with HeightMapShape3D Sep 26, 2024
@Chaosus Chaosus added this to the 4.4 milestone Sep 26, 2024
@AThousandShips AThousandShips requested a review from a team September 26, 2024 08:27
Copy link
Member

@rburing rburing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks a lot for your contribution!

@Repiteo Repiteo merged commit 6577ed2 into godotengine:master Nov 11, 2024
19 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 11, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GodotPhysics] Raycasts don't reliably collide with HeightMapShape3D
4 participants