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

Update Geometry3D.get_triangle_barycentric_coords() docs #80269

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PrecisionRender
Copy link
Contributor

@PrecisionRender PrecisionRender commented Aug 4, 2023

Resolves comment #71233 (comment)

Updates the Geometry3D.get_triangle_barycentric_coords() documentation to explain what happens when the parameters are invalid.

doc/classes/Geometry3D.xml Outdated Show resolved Hide resolved
Copy link
Member

@kleonc kleonc left a comment

Choose a reason for hiding this comment

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

Added some suggestions, feedback welcomed as always. 🙂

@@ -81,6 +81,33 @@
<param index="3" name="c" type="Vector3" />
<description>
Returns a [Vector3] containing weights based on how close a 3D position ([param point]) is to a triangle's different vertices ([param a], [param b] and [param c]). This is useful for interpolating between the data of different vertices in a triangle. One example use case is using this to smoothly rotate over a mesh instead of relying solely on face normals.
[param a], [param b] and [param c] must form a valid triangle and [param point] must be coplanar with the [param a][param b][param c] triangle. Returns an empty [Vector3] if [param a][param b][param c] does not form a valid triangle. Returns invalid weights if [param point] is not coplanar with the [param a][param b][param c] triangle.
Copy link
Member

Choose a reason for hiding this comment

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

Coplanar / not coplanar is kinda fluid using floats. This should clarify this a little:

Suggested change
[param a], [param b] and [param c] must form a valid triangle and [param point] must be coplanar with the [param a][param b][param c] triangle. Returns an empty [Vector3] if [param a][param b][param c] does not form a valid triangle. Returns invalid weights if [param point] is not coplanar with the [param a][param b][param c] triangle.
[param a], [param b] and [param c] must form a valid triangle and [param point] must be coplanar with the [param a][param b][param c] triangle. Returns an empty [Vector3] if [param a][param b][param c] does not form a valid triangle. Returns invalid weights if [param point] is not coplanar with the [param a][param b][param c] triangle. However, the closer [param point] is to the triangle's plane, the smaller the error is. Hence floating-point precision errors and alike should not affect the correctness of the result much.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add.

var w = get_triangle_barycentric_coords(point, a, b, c)
# w is Vector3(-3, 1, 3)
# w contains invalid weights
[/codeblock]
[url=https://en.wikipedia.org/wiki/Barycentric_coordinate_system]Here is a more detailed explanation of barycentric coordinates.[/url]
Copy link
Member

Choose a reason for hiding this comment

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

Moved the link into the description.

Suggested change
[url=https://en.wikipedia.org/wiki/Barycentric_coordinate_system]Here is a more detailed explanation of barycentric coordinates.[/url]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fairly certain the way it is now is more consistent with the rest of the API docs.

@@ -81,6 +81,33 @@
<param index="3" name="c" type="Vector3" />
<description>
Returns a [Vector3] containing weights based on how close a 3D position ([param point]) is to a triangle's different vertices ([param a], [param b] and [param c]). This is useful for interpolating between the data of different vertices in a triangle. One example use case is using this to smoothly rotate over a mesh instead of relying solely on face normals.
Copy link
Member

@kleonc kleonc Aug 5, 2023

Choose a reason for hiding this comment

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

I think this equation should be mentioned somewhere. Also reworded this a little to move the wiki link in here (and link directly to the section relevant for triangles):

Suggested change
Returns a [Vector3] containing weights based on how close a 3D position ([param point]) is to a triangle's different vertices ([param a], [param b] and [param c]). This is useful for interpolating between the data of different vertices in a triangle. One example use case is using this to smoothly rotate over a mesh instead of relying solely on face normals.
Returns a [Vector3] containing a 3D [param point]'s [url=https://en.wikipedia.org/wiki/Barycentric_coordinate_system#Barycentric_coordinates_on_triangles]barycentric coordinates within a triangle[/url] formed by [param a], [param b], [param c] vertices (such that [code]result.x * a + result.y * b + result.z * c[/code] approximately equals [param point]). This is useful for interpolating between the data of different vertices in a triangle. One example use case is using this to smoothly rotate over a mesh instead of relying solely on face normals.

Edit: Oh, now I see it was intentionally written like that (#71233 (comment)). Oh, well. 🙃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll incorporate something like this.

Comment on lines +89 to +92
var point = Vector3(1.5, 1, 0.5)
var w = get_triangle_barycentric_coords(point, a, b, c)
# w is Vector3(0, 0.5, 0.5)
# w contains valid weights
Copy link
Member

Choose a reason for hiding this comment

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

WDYT about something like:

Suggested change
var point = Vector3(1.5, 1, 0.5)
var w = get_triangle_barycentric_coords(point, a, b, c)
# w is Vector3(0, 0.5, 0.5)
# w contains valid weights
var point = Vector3(1.5, 1, 0.5) # Coplanar with abc triangle (valid input)
var w = get_triangle_barycentric_coords(point, a, b, c) # w is Vector3(0, 0.5, 0.5)
point.is_equal_approx(w.x * a + w.y * b + w.z * c) # True, w contains valid weights

?
(if it's good then applicable to the other examples)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestions!

When I have time, I'll add them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I like this.

@YuriSizov YuriSizov modified the milestones: 4.2, 4.3 Nov 13, 2023
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.

None yet

4 participants