added `Keyframe::new_dynamic` and a matching From implementation

As discussed in #6 having a constructor that takes a boxed EasingFunction directly makes sense, as they are saved as Boxes internally anyway.

With the from implementation this new constructor should also support the keyframes! macro.

let easing_box = match easing_enum {
  VisualizerExample::LinearTwoPoint => Box::new(Linear),
  VisualizerExample::EaseInOutFourPoint => Box::new(EaseInOut),
  _ => panic!(),
}

keyframes![
  ([0.0, 0.0].into(), 0.0, easing_box),
  ([1.0, 1.0].into(), 1.0)
]

You said that you'd like to be able to use the macro without needing to create a Box explicitly. I'm wondering: what exactly would be the use case of that? I mean, when working with variables that are set at runtime containing EasingFunctions, the compiler still forces you to specify a size, which means you'll have to create a Box anyway.

With the from implementation this new constructor should also support the keyframes! macro.

While it does work it runs into the same problem as when I tried to get it to work earlier.

This code compiles without issues:

let seq1 = keyframes![(0.0, 0.0, Linear)];
let seq2 = keyframes![(0.0, 0.0, Box::new(Linear) as Box<dyn EasingFunction + Send + Sync>)];

But this one does not:

let seq3 = keyframes![(0.0, 0.0, Box::new(Linear))];
// error[E0277]: the trait bound `Box<keyframe::functions::Linear>: EasingFunction` is not satisfied

It would be nice if you could pass it a Box<EasingFunction> without having to cast it, but I'm not sure if Rust's type system is capable of figuring that out. It's been a while since I used Rust so maybe I'm missing something.

You said that you'd like to be able to use the macro without needing to create a Box explicitly. I'm wondering: what exactly would be the use case of that?

What I meant is I would be able to write keyframes![(0.0, 0.0, Linear)] rather than keyframes![(0.0, 0.0, Box::new(Linear))] when using the macro. No need to call the constructor on your own because that's done internally in Keyframe::new.

If the from implementation for Keyframe::new was changed to the following:

impl<V, T: Float, F: EasingFunction + 'static + Send + Sync> From<(V, T, Box<F>)> for Keyframe<V> {
    #[inline]
    fn from(tuple: (V, T, Box<F>)) -> Self { Keyframe::new(tuple.0, as_f64(tuple.1), tuple.2) }
}

then you could pass it any type of Box<EasingFunction> and the compiler can figure it out but the old syntax would no longer work. You would always have to type Box::new when using the macro.

Keeping both From<(V, T, F)> and From<(V, T, Box<F>)> gives me a compiler error about a conflicting implementation.

This code compiles without issues:
let seq1 = keyframes![(0.0, 0.0, Linear)];
let seq2 = keyframes![(0.0, 0.0, Box::new(Linear) as Box<dyn EasingFunction + Send + Sync>)];
But this one does not:
let seq3 = keyframes![(0.0, 0.0, Box::new(Linear))];
// error[E0277]: the trait bound `Box<keyframe::functions::Linear>: EasingFunction` is not satisfied
It would be nice if you could pass it a Box<EasingFunction> without having to cast it, but I'm not sure if Rust's type system is capable of figuring that out. It's been a while since I used Rust so maybe I'm missing something.

You said that you'd like to be able to use the macro without needing to create a Box explicitly. I'm wondering: what exactly would be the use case of that?

What I meant is I would be able to write keyframes![(0.0, 0.0, Linear)] rather than keyframes![(0.0, 0.0, Box::new(Linear))] when using the macro. No need to call the constructor on your own because that's done internally in Keyframe::new.

I agree that such a cast is ugly, but I think it's also unneccesary in any real situation. This is because you never actually need to write Box::new(SomeEasingFunction): In all situations that I can imagine one does either know the type of EasingFunction at compile time and can just type it in without boxing it, or one doesn't, which leads to having to box it beforehand anyway, just to be able to handle it. In the second case such a Box will always be a Box<dyn EasingFunction + ...> as the type is only known at runtime.

If the from implementation for Keyframe::new was changed to the following:
impl<V, T: Float, F: EasingFunction + 'static + Send + Sync> From<(V, T, Box<F>)> for Keyframe<V> {
  #[inline]
  fn from(tuple: (V, T, Box<F>)) -> Self { Keyframe::new(tuple.0, as_f64(tuple.1), tuple.2) }
}
then you could pass it any type of Box<EasingFunction> and the compiler can figure it out but the old syntax would no longer work. You would always have to type Box::new when using the macro.

Keeping both From<(V, T, F)> and From<(V, T, Box<F>)> gives me a compiler error about a conflicting implementation.

Getting something like keyframes![(0.0, 0.0, Box::new(Linear))] to work (additionally) might be nice (because why not), but as far as I understand the problem seems to be that downstream users might implement EasingFunction for Box<F> themselves, leading to ambiguity, as there are now two implementations of From that might be used, one taking the Box<F> as a Box<F: EasingFunction + 'static + Send + Sync> and one taking it as a valid implementor of EasingFunction itself. At least that's how I interpret the note that the error comes with:

note: downstream crates may implement trait `easing::EasingFunction` for type `std::boxed::Box<_>`

The only way around this that I found was implementing EasingFunction for Box like this:

impl<F: EasingFunction + 'static + Send + Sync> EasingFunction for Box<F> {
    fn y(&self, x: f64) -> f64 { (**self).y(x) }
}

Now one can remove From<(V, T, Box<F>)>, and everything works fine. Both keyframes![(0.0, 0.0, Linear)] and keyframes![(0.0, 0.0, Box::new(Linear))] work now. But while this solution adds support for the second case it's also inherintly silly, as it leads to double boxing.

Therefore I'd argue for dropping the idea of keyframes![(0.0, 0.0, Box::new(Linear))], because I just can't see how anyone would ever get into a situation where it's neccesary or actually helpful.

In all situations that I can imagine one does either know the type of EasingFunction at compile time and can just type it in without boxing it, or one doesn't, which leads to having to box it beforehand anyway, just to be able to handle it. In the second case such a Box will always be a Box<dyn EasingFunction + ...> as the type is only known at runtime.

Therefore I'd argue for dropping the idea of keyframes![(0.0, 0.0, Box::new(Linear))], because I just can't see how anyone would ever get into a situation where it's neccesary or actually helpful.

That does make sense, since the only real reason why you would need a boxed EasingFunction is when the type is unknown at compile-time. Since you need a Box<dyn EasingFunction> as soon as there's any ambiguity (like in your example with the match statement or when passing it as a function argument) having a boxed EasingFunction with a known type fail isn't a big deal.

I'll merge this and release 1.0.4. :+1:

Closed the PR on accident. My bad!

hannesmann / keyframe

added `Keyframe::new_dynamic` and a matching From implementation #7