Add unit structs for each ease function

[greeble-dev]

Objective

Allow users to directly call ease functions rather than going through the EaseFunction struct. This is less verbose and more efficient when the user doesn't need the data-driven aspects of EaseFunction.

Background

EaseFunction is a flexible and data-driven way to apply easing. But that has a price when a user just wants to call a specific ease function:

EaseFunction::SmoothStep.sample(t);

This is a bit verbose, but also surprisingly inefficient. It calls the general EaseFunction::eval, which won't be inlined and adds an unnecessary branch. It can also increase code size since it pulls in all ease functions even though the user might only require one. As far as I can tell this is true even with opt-level = 3 and lto = "fat".

; EaseFunction::SmoothStep.sample_unchecked(t)
lea rcx, [rip + __unnamed_2] ; Load the disciminant for EaseFunction::SmoothStep.
movaps xmm1, xmm0
jmp bevy_math::curve::easing::EaseFunction::eval    

Solution

This PR adds a struct for each ease function. Most are unit structs, but a couple have parameters:

SmoothStep.sample(t);

Elastic(50.0).sample(t);

Steps(4, JumpAt::Start).sample(t)

The structs implement the Curve<f32> trait. This means they fit into the broader Curve system, and the user can choose between sample, sample_unchecked, and sample_clamped. The internals are a simple function call so the compiler can easily estimate the cost of inlining:

; SmoothStep.sample_unchecked(t)
movaps xmm1, xmm0
addss xmm1, xmm0
movss xmm2, dword ptr [rip + __real@40400000] ; 3.0
subss xmm2, xmm1
mulss xmm2, xmm0
mulss xmm0, xmm2

In a microbenchmark this is around 4x faster. If inlining permits auto-vectorization then it's 20-50x faster, but that's a niche case.

Adding unit structs is also a small boost to discoverability - the unit struct can be found in VS Code via "Go To Symbol" -> "smoothstep", which doesn't find EaseFunction::SmoothStep.

Concerns

• While the unit structs have advantages, they add a lot of API surface area.
• Another option would have been to expose the underlying functions.
  • But functions can't implement the Curve trait.
  • And the underlying functions are unclamped, which could be a footgun.
  • Or there have to be three functions to cover unchecked/checked/clamped variants.
• The unit structs can't be used with EasingCurve, which requires EaseFunction.
  • This might confuse users and limit optimisation.
    • Wrong: EasingCurve::new(a, b, SmoothStep).
    • Right: EasingCurve::new(a, b, EaseFunction::SmoothStep).
  • In theory EasingCurve could be changed to support any Curve<f32> or a more limited trait.
  • But that's likely to be a breaking change and raises questions around reflection and reliability.
• The unit structs don't have serialization.
  • I don't know much about the motivations/requirements for serialization.
• Each unit struct duplicates the documentation of EaseFunction.
  • This is convenient for the user, but awkward for anyone updating the code.
  • Maybe better if each unit struct points to the matching EaseFunction.
  • Might also make the module page less intimidating (see screenshot).

Testing

cargo test -p bevy_math

[ElliottjPierce]

This is pretty much exactly what I've been hoping for. Thanks for doing it!

As for the extra API surface, this is 100% worth it. It gives significant performance wins in my noise lib, basically for free. Also note that not all easing functions are zero-sized, differentiable, etc. We could build on this with enums like SmallEaseFunction, DifferentiableEaseFunction, and SmoothEaseFunction (where the derivative is 0 at t=0 and t=1). That's what I'd use ultimately for my noise lib to allow users to change the ease at runtime. It would also open up the possibility for a Smooth trait (WIP name).

So the API surface is not only worth it, but it's a blocker for these tools.

As for the docs, for now, I see no problem duplicating it. But if we add the other 3 enums above, this would become annoying. It doesn't have to be this pr, but I would either have the enum docs point back to the unit struct docs, or create a macro for each function that expands to the desired docs. Or just dump it all in module docs and refer to that. I've seen other crates do that sometimes.

Anyway, that's my thoughts. Excited to see this land!

[ElliottjPierce]

This isn't a bad thing to have, but all the functions just call each other, so I don't really see the need for this.

[greeble-dev]

I added it to defend against copy and paste errors, although in retrospect the test was vulnerable to the same error.

I've updated with a macro based test that should be less vulnerable. Also open to removing it if there's a consensus that it's unnecessary.

[mweatherley]

I'm happy with this :)

In general, I might prefer if the unit structs actually had Curve appended to their names, but I could probably be convinced otherwise. (I recall trying to be pretty consistent about having Curve in the name of curve types.)

In any case, thanks!

[greeble-dev]

I'm open to adding Curve to the name - I don't feel strongly either way. I'm gonna leave it for now and see if other reviewers or discord have a take.

My personal take is that I like the current solution's brevity and guess most people will recognise the names as curves. But adding Curve is clearer and more consistent.

[Aceeri]

Unfortunate that the compiler can't inline this...

Does adding #[inline] to the EasingFunction::eval/Curve::sample/sample_unchecked do anything to help?

Could we make EasingCurve take an impl Into<EasingFunction> and have each of the unit structs impl their respective intos? Not a full solution imo, but it'd reduce some user error here.

[alice-i-cecile]

I would prefer the FooCurve names here. I'd also like to warn on the old struct / approach, and probably at the module docs level too. We need better guidance on when each approach should be preferred.

I don't think we should deprecated EaseFunction, since it allows for runtime flexibility, but please correct me if I'm wrong.

[benfrankel]

EaseFunction is convenient so I can switch between ease functions in a config file without having to recompile my game. I can see something similar to the enum Color { Srgba(Srgba), ... } pattern making sense here?

[greeble-dev]

Does adding #[inline] to the EasingFunction::eval/Curve::sample/sample_unchecked do anything to help?

Nope - I'm guessing the compiler stops as soon as it sees the size of EaseFunction::eval, even though most of the function would be optimised away later. I think it could also be risky - in the data-driven case inlining EaseFunction::eval would usually be bad.

Could we make EasingCurve take an impl Into<EasingFunction> and have each of the unit structs impl their respective intos? Not a full solution imo, but it'd reduce some user error here.

That's a good idea. I think it's worth exploring, but might also be a bit more controversial. So I'll leave it out of this PR unless other reviewers think it should be tried now.

[greeble-dev]

I've done a new version of the documentation that tries to be clearer about differences and user choices. The PR description might have been misleading - the intention is to offer more options, not replace EaseFunction.

I went with a more beginner-y focus (e.g. "zero to one" over "unit interval") as I guess easing functions are often used by people who are focused on visuals rather than math. Not sure if this was the right choice.

Also added Curve to the names. This meant dropping the macro magic as the identifiers no longer match EaseFunction, so the test is a bit more tenuous.

[alice-i-cecile]

This meant dropping the macro magic as the identifiers no longer match EaseFunction, so the test is a bit more tenuous.

Can't you use macro magic to just append / strip Curve?

[greeble-dev]

Can't you use macro magic to just append / strip Curve?

I might be wrong as I'm still new to Rust, but as far I can tell that requires a proc macro or an external crate like paste. Unless Bevy already has something I can use?

EDIT: Should have said "requires a proc macro or external crate, which seems excessive for this case".

[alice-i-cecile]

Sounds good :) Not worth increasing complexity here. I haven't done a ton with macros myself, so I wasn't sure.

[alice-i-cecile]

Very nice docs, thank you!