terrain.get_intersection() returns wrong result first but then correct result on second try

[ell1e]

Terrain3D version

v0.9.2

System information

Godot Engine v4.3.stable.flathub [77dcf97d8]

Is the issue reproducable in the demo?

No

Issue description

(Note: I saw #498 but since in our test project the position is only wrong for the first ray and not for the second one, it's probably a different issue. But sorry if this is a duplicate)

We are seeing weird issues with terrain.get_intersection(), where it first returns an invalid ray that is too long as if the terrain wasn't properly detected, but then the second attempt is correct again. We couldn't quite figure out what the problem was, so we made an isolated test project where the error can be seen.

The test project is here: https://github.com/alf632/terrain3dglitch

Steps to reproduce:

1. git clone https://github.com/alf632/terrain3dglitch
2. Import and open the project in Godot. You'll probably get a few parse and import errors, close godot again and reopen, which you may repeat twice, to get rid of them.
3. Click godot's run arrow ▶️ in the top-right of the project view
4. ALT+LEFT_CLICK on the unit that shows on the center of the screen.
5. ALT+RIGHT_CLICK somewhere to send the unit there. You'll notice that the path that is shown as gray cubes will go to a slightly wrong target sometimes. ALT+RIGHT_CLICK again at the exact same spot right after (don't wait for the unit to go there) and the path will be correct. This is the intersection first returning a wrong target point that deeply penetrates the terrain, then the correct one.
6. Repeat step 5 with a different spot somewhere on the screen on the terrain if you didn't see much of a target difference. It should most of the time return an invalid spot first, then on second target try a correct point.

(The project itself also has a README with the reproduction steps)

We narrowed this down to the Terrain3D extension by outputting the coordinates that we get in source/match/maps/3Dmap/3Dmap.gd in line 20: https://github.com/alf632/terrain3dglitch/blob/main/source/match/maps/3Dmap/3Dmap.gd#L20

We do use terrain.set_camera() in DemoMatch.gd in line 10: https://github.com/alf632/terrain3dglitch/blob/main/DemoMatch.gd#L10

If you set a breakpoint in 3Dmap.gd in line 20, you can check the position yourself once you click on the terrain, or just read the debug output in the "Output" tab while the test project is running to see the coordinates and that they are varying wildly, while the camera_pos and camera_dir which are used for the ray are seemingly quite consistent.

I hope this demo works well enough for you to reproduce it, I know that I fell short with that for my previous ticket. Let me know if you can reproduce the issue at all, if you do get around to looking at this. My apologies for the inconvenience, and sorry if we made an obvious mistake here.

Logs

Terrain3D#6610:_notification: NOTIFICATION_EDITOR_PRE_SAVE
Terrain3DStorage#2973:save: Save requested, but not modified. Skipping
Terrain3DMaterial#1217:save: Generating parameter list from shaders
Terrain3DMaterial#1217:save: Attempting to save material to external file: res://assets/maps/demo/material.tres
Terrain3DMaterial#1217:save: ResourceSaver return error (0 is OK): 0
Terrain3DMaterial#1217:save: Finished saving material
Terrain3DAssets#1924:save: Attempting to save texture list to external file: res://assets/assets.tres
Terrain3DAssets#1924:save: ResourceSaver return error (0 is OK): 0
Terrain3DAssets#1924:save: Finished saving texture list
Godot Engine v4.3.stable.flathub.77dcf97d8 - https://godotengine.org
Vulkan 1.3.289 - Forward+ - Using Device #0: AMD - AMD Custom GPU 0405 (RADV VANGOGH)

setting up unit properties for res://source/match/units/Tank.gd
sight_range8
hp10
hp_max10
attack_damage2
projectile_speed10
movement_speed100
attack_interval0.75
attack_range5
attack_aim0.6
attack_domains[1]

clickPos: (247, 265)
camera_pos: (238.757, 14.072, 237.332)
camera_dir: (-0.772096, -0.328108, -0.544255)
target_point: (340282346638528859811704183484516925440, 340282346638528859811704183484516925440, 340282346638528859811704183484516925440)
height: nan
target_point not valid

clickPos: (247, 265)
camera_pos: (238.757, 14.072, 237.332)
camera_dir: (-0.772096, -0.328108, -0.544255)
target_point: (205.5991, -0.018707, 213.9588)
height: 0.00000744257977
--- Debugging process stopped ---

[MilesRomanum]

Hi, I saw my issue mentioned and decided to do a test. If I do the second click (on the same location), the intersection indeed becomes accurate. so I can confirm that it is (probably) the same bug

[TokisanGames]

The GPU mouse reads from a viewport. It's probable the viewport isn't updated by the renderer soon enough before it's read.

[ell1e]

Is there something that should be done from our end to rectify that? My apologies if that's an obvious or superfluous question.

[TokisanGames]

You could intentionally read it twice, or try await RenderingServer.frame_post_draw before the second read. You can read the RenderingServer and Viewport docs to learn of the caveats of viewports. You could also look through the renderingserver options and the settings chosen in terrain3d.cpp:get_intersection() and offer a patch to fix the problem. I won't look at this until we finalize some PRs we're working on so we can get the next release out.

[TokisanGames]

Thanks for the detailed report and project.
Using this workaround does indeed allow it to work properly. The first call captures the image, the second gets the result.

var target_point = terrain.get_intersection(camera_pos,camera_dir)
await RenderingServer.frame_post_draw
target_point = terrain.get_intersection(camera_pos,camera_dir)

@MilesRomanum Does this also fix the near horizontal camera issue? I have no issue in ell1e's demo using a near horizontal camera and this workaround.

As noted before the issue is get_intersection() tells the renderingserver to draw a frame, then requests that frame. C++ can't await, so it returns an invalid picture until a subsequent call. I'm not sure how to fix that as there's no current facility Godot to draw the frame right now. We might just have to document this workaround.

Maybe we could add another parameter where if enabled works as is using GPU mode. If false, it uses an iterative method.

[TokisanGames]

Fixed by #567. Either using the documented workaround for gpu mode. Or a raymarched mode without the workaround.

[MilesRomanum]

hi, yeah that makes sense. Pretty sure both the await and the raymarched mode would work. for my personal use case I will probably use raymarched because it might be slightly faster, but really appreciate the work around