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

Handle corner cases for curve baking #69726

Merged
merged 1 commit into from Dec 14, 2022

Conversation

xiongyaohua
Copy link
Contributor

@xiongyaohua xiongyaohua commented Dec 7, 2022

When control point and in/out point have equal position, the derivative of bezier curve is 0 vector, which cause error message in Basis::look_at().

ERROR: The target vector can't be zero.
at: looking_at (core/math/basis.cpp:1041)

Also cause #69789

This commit handles this corner case.
Fix #69789

Copy link
Member

@TokageItLab TokageItLab left a comment

Choose a reason for hiding this comment

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

Corner cases should be handled (checked) in the Curve class, not in the Math class.

@xiongyaohua
Copy link
Contributor Author

xiongyaohua commented Dec 13, 2022

To be more precise, they are the corner cases of how to correctly calculate tangent for one bezier segment. And like Vector2D/3D::bezier_derivative(), they can be used more generally than only in Curve2D/3D. So I feel it could also be put on Vector2D/3D instead of Curve2D/3D.

@TokageItLab
Copy link
Member

TokageItLab commented Dec 13, 2022

I don't think so. There are several ways to think about the treatment of end/sharp points, and the error caused by the Curve class calculation using look_at() should be handled in the Curve class.

@TokageItLab
Copy link
Member

TokageItLab commented Dec 13, 2022

I also think it is not a good thing to insist on the word bezier in naming for function that is not actually a bezier curve. If you want to make it generic for some places, the biggest compromise is to expose a static method called Curve2D/3D::calculate_tangent(p_from, p_to, p_pre, p_post, p_weight) in a similar way to PathFollow3D::correct_posture(). Then, there is no meaning there that the Bezier end/sharp points must be treated in a specific way mathematically, and it makes it clear that Godot's curve implies that the end/sharp points have to be treated in that specific way.

When control point and point have equal position,
the derivative is 0 vector, which cause error message in Basis::look_at().
This commit handles this case.
@xiongyaohua
Copy link
Contributor Author

Thanks for the explanation. I think you are right. I moved these methods to Curve2D/3D as internal methods, to keep them as implementation detail.

@akien-mga akien-mga merged commit 45edf35 into godotengine:master Dec 14, 2022
@akien-mga
Copy link
Member

Thanks!

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.

Curve2D and Curve3D is baked incorrectly at sharp control points(in/out handles are 0 length).
4 participants