godot icon indicating copy to clipboard operation
godot copied to clipboard

Add ability to get barycentric coordinates from ray

Open PrecisionRender opened this issue 1 year ago • 13 comments

Implements https://github.com/godotengine/godot-proposals/issues/4663, #684, #1903, portion of https://github.com/godotengine/godot/issues/17665

Closes https://github.com/godotengine/godot-proposals/issues/4663

This PR implements a way to get the barycentric coordinates of a collision object from a ray. To do this, we return the intersected face index from a ConcaveCollisionShape3D and expose a barycentric coordinate function. This allows for the user to get much more detail from a ray than simply the collision point and normal. Check the surface alignment example below.

Alignment using RayCast normal:

https://user-images.githubusercontent.com/89754713/211886287-06162053-b5a9-4a5b-a488-5a5b89ed827c.mp4

Alignment using barycentric coordinate interpolation:

https://user-images.githubusercontent.com/89754713/211886330-a0e8c9bd-4db1-4d65-b5a0-9e08c32fc169.mp4

Test project: Barycentric Coords Test.zip

PrecisionRender avatar Jan 11 '23 18:01 PrecisionRender

Is this implemented in Godot 4 RC1?

Calls to 'get_triangle_barycentric_coords' stills don't appear to work.

thismarty avatar Feb 08 '23 20:02 thismarty

Darn, I just noticed that it is still pending review before merging.

Sure hope it gets merged soon. But I guess it's going to have to wait for Godot 4.1 now?

thismarty avatar Feb 08 '23 20:02 thismarty

@thismarty I can't find a related/similar feature in the RC1 release notes. The barycentric coordinate function still works for me.

PrecisionRender avatar Feb 08 '23 20:02 PrecisionRender

Sure hope it gets merged soon. But I guess it's going to have to wait for Godot 4.1 now?

Unfortunately so, I guess. Kind of a shame, though. This has been open and ready to merge for a while now. But I did open this after the feature freeze.

PrecisionRender avatar Feb 08 '23 20:02 PrecisionRender

Dangit. Oh well, thanks a bunch for implementing it - it will be very useful once merged!

thismarty avatar Feb 08 '23 20:02 thismarty

@adamscott Woah, I think I accidentally nuked this commit...

PrecisionRender avatar Mar 05 '23 22:03 PrecisionRender

How do I fix this? As you can see, I can't use Git. 😂😵

PrecisionRender avatar Mar 05 '23 22:03 PrecisionRender

@PrecisionRender See this documentation article, or for a GUI option, this video.

aaronfranke avatar Mar 05 '23 22:03 aaronfranke

@aaronfranke Thank you. I'm aware of rebasing and such, but I don't know exactly what I need to do to undo all of the commits I accidentally squashed into this PR.

PrecisionRender avatar Mar 05 '23 22:03 PrecisionRender

Use git reflog to find the commit id of the last commit from before you messed things up and then git reset --hard <commitid> to go back there and then do whatever you were doing again, but correctly. (From githubs force push log, it probably is 0b4dc1ae33513e531f407067f52bff9ff54e05ad)

RedworkDE avatar Mar 05 '23 22:03 RedworkDE

@aaronfranke @RedworkDE I'm not sure what's causing these checks to fail? I can build from source with no problems.

PrecisionRender avatar Mar 05 '23 23:03 PrecisionRender

@PrecisionRender thanks for rebasing and cleaning up the history with reflog. Your git work looks good.

I think you got unlucky and the builds are failing for some unrelated reason. In a few days or after it gets reviewed, try git fetch and git rebase origin/master and see if that issue goes away.

Scons cache enabled... (path: 'D:\a\godot\godot/.scons-cache/') 42 TypeError: can only concatenate deque (not "list") to deque: 43 File "D:\a\godot\godot\SConstruct", line 927: 44 methods.generate_vs_project(env, GetOption("num_jobs"),

lyuma avatar Mar 05 '23 23:03 lyuma

@lyuma @aaronfranke @RedworkDE Thanks for the help, everyone. 🙌

PrecisionRender avatar Mar 05 '23 23:03 PrecisionRender

Yay! No nuclear commits this time! 😂

PrecisionRender avatar Apr 12 '23 15:04 PrecisionRender

Seems like the error is a code error and not github flakey-ness.

servers/physics_3d/godot_body_pair_3d.cpp:171:16: error: unused variable 'shape_B_ptr' [-Werror,-Wunused-variable]
        GodotShape3D *shape_B_ptr = p_B->get_shape(p_shape_B);

fire avatar Apr 12 '23 19:04 fire

Seems like the error is a code error and not github flakey-ness.

Fixed, thanks.

PrecisionRender avatar Apr 12 '23 20:04 PrecisionRender

bump, i need this :)

yesitsfebreeze avatar Apr 24 '23 15:04 yesitsfebreeze

Any chance we'll see this in 4.0.2? :-)

thismarty avatar Apr 29 '23 18:04 thismarty

Any chance we'll see this in 4.0.2? :-)

4.0.2 is already released, and we don't cherry-pick new features to 4.0.x patch releases as per the release policy. The first release that could feature this PR is 4.1 at the earliest – we can't guarantee this will be merged for 4.1.

Calinou avatar Apr 29 '23 20:04 Calinou

So cool to see that Barycentric Coordinates is being implemented, this was after I saw a youtube video that demonstrates the use of Barycentric Coordinates in F-Zero GX... Additionally I think BC was also used in other games such as Super Mario Galaxy (A.K.A. Planets Platforming)

Corruptinator avatar May 16 '23 04:05 Corruptinator

Has this PR fallen under the radar?

PrecisionRender avatar Jun 09 '23 16:06 PrecisionRender

We should think a bit about what to (eventually) do for heightmap shapes, to avoid having to change the API again later. Maybe the face index should be a 64-bit integer?

rburing avatar Jun 11 '23 15:06 rburing

I want Array.size() to have 64 bit lengths, I don't know why it isn't the case too.

fire avatar Jun 11 '23 15:06 fire

We should think a bit about what to (eventually) do for heightmap shapes, to avoid having to change the API again later. Maybe the face index should be a 64-bit integer?

The physics backend uses regular integers. I think changing the types is outside the scope of this PR. I'm only exposing the backend.

PrecisionRender avatar Jul 18 '23 16:07 PrecisionRender

We should think a bit about what to (eventually) do for heightmap shapes, to avoid having to change the API again later. Maybe the face index should be a 64-bit integer?

@rburing Can you provide more data about your usecase in which a single submesh needs more than 2 billion vertices?

Almost no GPU api supports 64-bit indices (or even if it does support it, it won't be efficient). I think we should keep mesh data at 32-bit since Godot stores mesh data on the GPU (including vertices and face indices).

If someone can provide a really good example for needing more than this in a surface, then it will make it easier to understand the tradeoffs.

@fire, I agree about Array.size() in general, but in this case we're talking about Mesh apis so that won't be an issue here as far as I can tell.

lyuma avatar Jul 19 '23 04:07 lyuma

My point was if there are at most 2147483648 indices then a square heightmap (with two triangles for each square face) can have at most sqrt(2147483648/2) = 32768 squares along each axis (i.e. much less than a heightmap's theoretical limit), if the triangle indices are to be somehow uniquely defined based on the information x, y, and which of the two triangles of the square. To be honest I didn't run the numbers before and it actually seems reasonable, not very limiting in practice, so I am fine with using int.

There is still one uninitialized int face_index; in the PR.

rburing avatar Jul 19 '23 05:07 rburing

This should finally be ready to merge now.

PrecisionRender avatar Jul 25 '23 02:07 PrecisionRender

Not sure what's going on with the cpp extension.

PrecisionRender avatar Jul 27 '23 23:07 PrecisionRender

Not sure what's going on with the cpp extension.

I just restarted the build that failed, we had issues with our cache storage, so it shouldn't be linked to your PR.

Hopefully, the test will pass and we'll merge (finally) your PR!

adamscott avatar Jul 28 '23 21:07 adamscott