Split `running_animations` into two lists, one for keyframes and one for transitions

[notriddle]

Instead of this:

#[derive(Clone, Debug)]
pub enum Animation {
    Transition(OpaqueNode, UnsafeNode, f64, AnimationFrame, bool),
    Keyframes(OpaqueNode, Atom, KeyframesAnimationState),
}

// ...

pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
                              script_chan: &IpcSender<ConstellationControlMsg>,
                              running_animations: &mut HashMap<OpaqueNode, Vec<Animation>>,
                              expired_animations: &mut HashMap<OpaqueNode, Vec<Animation>>,
                              new_animations_receiver: &Receiver<Animation>,
                              pipeline_id: PipelineId,
                              timer: &Timer) {

The data should be defined like this:

#[derive(Clone, Debug)]
pub enum Animation {
    Transition(TransitionAnimation),
    Keyframes(KeyframesAnimation),
}
#[derive(Clone, Debug)]
pub struct TransitionAnimation {
    opaque: OpaqueNode,
    node: UnsafeNode,
    start_time: f64,
    frame: AnimationFrame,
    is_expired: bool,
}
#[derive(Clone, Debug)]
pub struct KeyframesAnimation {
    opaque: OpaqueNode,
    name: Atom,
    state: KeyframesAnimationState,
}

// ...

pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
                              script_chan: &IpcSender<ConstellationControlMsg>,
                              running_keyframes_animations: &mut HashMap<OpaqueNode, Vec<KeyframesAnimation>>,
                              running_transition_animations: &mut HashMap<OpaqueNode, Vec<TransitionAnimation>>,
                              expired_keyframes_animations: &mut HashMap<OpaqueNode, Vec<KeyframesAnimation>>,
                              expired_transition_animations: &mut HashMap<OpaqueNode, Vec<TransitionAnimation>>,
                              new_animations_receiver: &Receiver<Animation>,
                              pipeline_id: PipelineId,
                              timer: &Timer) {

Making this change (there are more spots that will need to be changed than the ones shown, of course) should reduce the rightward drift that comes from having to loop and then match (instead, it's just two loops). It should also save RAM, by only allocating the space we need instead of the space of the largest variant (in this case, Animation::Keyframes). The enum can still be used for sending over the animation channel.

[KiChjang]

Perhaps using struct variants would make more sense? I.e.

#[derive(Clone, Debug)]
pub enum Animation {
    Transition {
        opaque: OpaqueNode,
        node: UnsafeNode,
        start_time: f64,
        frame: AnimationFrame,
        is_expired: bool
    },
    Keyframes {
        opaque: OpaqueNode,
        name: Atom,
        state: KeyframesAnimationState
    }
}

[KiChjang]

Oh, I see what's happening here. The only thing that doesn't quite make sense to me is that you're essentially creating 2 different structs that have no relation with each other, except in the case where you have to send data over a channel and you have to wrap them with the Animation enum. If the receiver also handles these two animations differently, you might as well create another channel altogether and keep them completely separate.

[notriddle]

If the receiver also handles these two animations differently, you might as well create another channel altogether and keep them completely separate.

We probably could have two separate channels, since the receiver just runs try_recv after every restyle, it could just as easily run try_recv on two separate channels.

[nox]

Is this still relevant? Given much stuff changed around animations since last year.

[mrobinson]

Transitions and animations are currently not sent over a channel, but they are still represented by an enum. It may make sense to fully separate these into two different structs as the number of situations where animations and transitions need to be handled differently (such as applying them in the proper order or one having precedence over the other) is only going to increase.