Add approximate comparing static methods to Animation and make Animation code use them

[TokageItLab]

• Fixes AnimationMixer signal don't emit when using custom timeline and time scale stretch #94459

The root cause was a mismatch in scale between prev time and current time due to the lack of the following multiplication:

animation_blend_tree.cpp

	// 3. Progress for Animation.
	double prev_playback_time = prev_time + start_offset;
	double cur_playback_time = cur_time + start_offset;
	if (stretch_time_scale) {
		double mlt = anim_size / cur_len;
		prev_playback_time *= mlt; // <- Added by this PR
		cur_playback_time *= mlt;
		cur_delta *= mlt;
	}

However, there are cases where adding multiplication alone will not solve the problem due to floating point error.

Since the animation uses double-valued time, an approximation is always needed when comparing them, except when an exact match is needed. As I remember, exact matches are only used in part for editors, so unless you are an editor, you can replace most of them.

[RandomShaper]

I was passing by and came up with an idea:

Instead of having all those very verbose method names, having certain C++ constructs that allowed you to write code like this:

double a, b;
// ...
if (a < Approx(0)) { // Var to constant.
// ...
if (a == Approx(b)) { // Var to var.
// ...

[TokageItLab]

@RandomShaper That sounds good, do you mean something that is almost the same as a double type, only overriding the comparison operator?

[RandomShaper]

@RandomShaper That sounds good, do you mean something that is almost the same as a double type, only overriding the comparison operator?

Exactly!

I don't know what to do with GDScript, though. It could still get the long-named functions or maybe defer the decision until we know there's actual demand.

[TokageItLab]

For now, I would like to fix the bug.

We may be able to avoid binding to gdscript until a direction is decided, but given the possibility that it may be difficult to expose Approx types to gdscript in the future, then we may need to eventually expose a Wrapper method that casts to Approx and does the comparison as:

bool VariantUtilityFunctions::is_less_or_equal_approx(double x, double y) {
    return x <= Approx(y);
}

In any case, considering that is_equal_approx() is already exposed, I suppose it is okay to expose the method current this PR's way for now for consistency.

[fire]

Given floating point error, "an approximation is always needed when comparing" time values.

I am not sure the approx object idea is developed enough to use and it would require extra bindings, probably need to separately implement with a design proposal.

[akien-mga]

I have some reservations regarding adding all those APIs to Math and the scripting API for a single use case in the engine. There hasn't been significant demand for it AFAIK, and the functions are quite verbose and IMO difficult to parse (it's also a problem of is_equal_approx and is_zero_approx, but since there's only two of those, it's not too bad as one knows it's either equal or zero).

So I'd prefer if this followed the "keep solutions local" guideline and add those helpers only to animation code for now. We can always refactor and move them to core eventually if there's a need.

I'm also dubious of whether functions like is_less_approx and is_greater_approx make sense. I'm afraid this might introduce more bugs than it will fix when e.g. small positive numbers are treated as passing is_less_approx(num, 0.0) which might lead to some weird wraparound. Overall the fix seems very heavy handed for this issue so it raises some flags IMO - but TIWAGOS, I haven't looked at either the code nor the bug in depth and you obviously know the constraints better than me.

[TokageItLab]

I moved it to Animation instead of Math.

I think this is a reasonable fix, not a hack, as we have done several fixes similar to this before. Also it is needed to fix a bug in #94459 and to reduce the delay in #94372.

I wonder if there is a need for a case that distinguishes between 0 and less than CMP_EPSILON, which might be a problem in a case that exceeds 100,000 FPS, but since that is unlikely, it shouldn't be a problem.

As I said above, the only part I remember needing an explicit approx/exact comparison is the part about track key editor and seek; The find_mode argument of track_find_key distinguishes them and this PR does not change it. So paradoxically, we can almost assume that where the difference is not explicit, the equal operator represents an Approx comparison.

[akien-mga]

Thanks!