Add ability to get face index and barycentric coordinates from raycast

[PrecisionRender]

Implements godotengine/godot-proposals#4663, #684, #1903, portion of #17665

Closes godotengine/godot-proposals#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:

wo_bary_coords.mp4

Alignment using barycentric coordinate interpolation:

w_bary_coords.mp4

Test project: Barycentric Coords Test.zip

[thismarty]

Is this implemented in Godot 4 RC1?

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

[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?

[PrecisionRender]

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

[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.

[thismarty]

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

[adamscott]

This looks fine to me. Can you rebase and test just to make sure it works with the latest code?

[AThousandShips]

LGTM, haven't tested so TIWAGOS but looks correct

[kleonc]

This should likely mention:

• what's a valid input (a, b, c forming a triangle and point being coplanar with the abc triangle),
• what's the behavior for an invalid input (a, b, c not forming a valid triangle, or point being not coplanar with the abc triangle).

(But it's a non-blocker, can be improved later, separately)

[YuriSizov]

Let's delegate this to a follow-up. This has been cooking long enough :)

[PrecisionRender]

I can open a new PR for this. Does an issue need to be created?

[paddy-exe]

@PrecisionRender No, just link back to this comment discussion and the PR

[YuriSizov]

Thanks a lot!

[Zireael07]

Yay I can already see uses for this <3

[Corruptinator]

AWESOME! Can't wait so see this feature implemented in Godot. Now it's just a matter of time before someone recreates an F-Zero style game in Godot.

[Zireael07]

I am rather thinking of doing shadow detection this way (instead of raycasting towards the sun) :)

[PrecisionRender]

AWESOME! Can't wait so see this feature implemented in Godot. Now it's just a matter of time before someone recreates an F-Zero style game in Godot.

@Corruptinator Already implemented. It should be fully ready for the 4.2 release. 😁

[Corruptinator]

I am rather thinking of doing shadow detection this way (instead of raycasting towards the sun) :)

Oh, that is a good point @Zireael07. You're right. So many uses for Barycentric Coordinates.

[mihe]

@YuriSizov

This PR seems to have introduced a breaking change to GDExtension without anyone realizing.

Any changes to PhysicsDirectSpaceState3D::RayResult needs to have the same change made to the struct bindings in register_server_types.cpp:

godot/servers/physics_server_3d.h

Lines 151 to 159 in f7bc653

	struct RayResult {
	Vector3 position;
	Vector3 normal;
	int face_index = -1;
	RID rid;
	ObjectID collider_id;
	Object *collider = nullptr;
	int shape = 0;
	};

godot/servers/register_server_types.cpp

Line 160 in f7bc653

GDREGISTER_NATIVE_STRUCT(PhysicsServer3DExtensionRayResult, "Vector3 position;Vector3 normal;RID rid;ObjectID collider_id;Object *collider;int shape");

... which would likely trigger a --validate-extension-api failure with how RayResult is laid out right now.

I'm not sure there's a perfect way to solve this, since you're changing the format/size of the struct no matter which way you turn it, which will likely upset --validate-extension-api, but moving face_index to the end of RayResult would at least preserve backwards compatibility as far as intersect_ray is concerned I guess.

[YuriSizov]

@mihe Are you saying that GDExtension API is sensitive to the order of elements we return in Dictionary? Or is this exposed in some other way to GDExtension? And if so, how come it's not validated on the CI right now, how does it fall through the cracks? I don't quite understand.

[mihe]

There is no Dictionary involved. RayResult is a struct on both ends, but called PhysicsServer3DExtensionRayResult in GDExtension, and its binary layout is described through GDREGISTER_NATIVE_STRUCT.

As of right now, the RayResult struct layout differs from the struct layout of PhysicsServer3DExtensionRayResult after the normal field, leading to anyone writing to (for example) the rid field from GDExtension to unknowingly write over face_index instead, and so forth.

And if so, how come it's not validated on the CI right now, how does it fall through the cracks?

It is, but only if you actually make the corresponding change to the GDREGISTER_NATIVE_STRUCT binding mentioned previously, in which case you'll be met with:

Validate extension JSON: Error: Field 'native_structures/PhysicsServer3DExtensionRayResult': format changed value in new API, from "Vector3 position;Vector3 normal;RID rid;ObjectID collider_id;Object *collider;int shape" to "Vector3 position;Vector3 normal;int face_index;RID rid;ObjectID collider_id;Object *collider;int shape".

This should obviously/ideally be dealt with in a less brittle and error-prone way, maybe using some macro magic, as opposed to typing in the layout into a string somewhere. It's not the first time this has happened, and probably won't be the last, unless this is changed.

[YuriSizov]

@mihe Could you create an issue for this so we don't lose track of it?

[mihe]

Issue created, with MRP: #80444

[Corruptinator]

Understood. Sorry for the non-related message.