godot
godot copied to clipboard
Add ability to get barycentric coordinates from ray
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
Is this implemented in Godot 4 RC1?
Calls to 'get_triangle_barycentric_coords' stills don't appear to work.
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 I can't find a related/similar feature in the RC1 release notes. The barycentric coordinate function still works for me.
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.
Dangit. Oh well, thanks a bunch for implementing it - it will be very useful once merged!
@adamscott Woah, I think I accidentally nuked this commit...
How do I fix this? As you can see, I can't use Git. 😂😵
@PrecisionRender See this documentation article, or for a GUI option, this video.
@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.
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
)
@aaronfranke @RedworkDE I'm not sure what's causing these checks to fail? I can build from source with no problems.
@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 @aaronfranke @RedworkDE Thanks for the help, everyone. 🙌
Yay! No nuclear commits this time! 😂
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);
Seems like the error is a code error and not github flakey-ness.
Fixed, thanks.
bump, i need this :)
Any chance we'll see this in 4.0.2? :-)
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.
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)
Has this PR fallen under the radar?
Has this PR fallen under the radar?
4.1 is in feature freeze, so this can only be merged for 4.2 at the earliest.
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?
I want Array.size() to have 64 bit lengths, I don't know why it isn't the case too.
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.
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.
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.
This should finally be ready to merge now.
Not sure what's going on with the cpp extension.
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!