lightningcss icon indicating copy to clipboard operation
lightningcss copied to clipboard

CSS transform optimisation breaks animation

Open shish opened this issue 3 years ago • 13 comments

🐛 bug report

I'm animating some things falling down the screen and rotating, but my code:

        @keyframes rain {
            0% {transform: translateY(0vh) rotate(0deg);}
            100% {transform: translateY(120vh) rotate(360deg);}
        }

gets optimised into:

        @keyframes rain {
            0% {transform: matrix(1, 0, 0, 1, 0, 0);}
            100% {transform: translateY(120vh) rotate(360deg);}
        }

matrix(1, 0, 0, 1, 0, 0) is mathematically equivalent to translate(0)+rotate(0)... but when we're using keyframe animation, each of the keyframes needs to manipulate the same properties - we can animate from rotate to rotate, but can't animate from matrix to rotate...

Notably this only happens with parcel build, not parcel run

🤔 Expected Behavior

Leave transforms alone if they're being animated

😯 Current Behavior

one transform is optimised into a matrix, the other is left alone

🌍 Your Environment

Software Version(s)
Parcel 2.7.0

shish avatar Sep 14 '22 22:09 shish

I'm having the same issue, my source SCSS:

@keyframes saveArticle {
  from {
    transform-origin: 50% 1400px;
    transform: translateY(0) translateX (0) rotateX(0) scale(1);
  }

  to {
    transform-origin: 50% 100%;
    transform: translateY(-600px) translateX(50%) rotateX(-30deg) scale(0);
  }
}

gets converted to this (only in production builds):

@keyframes saveArticle {
    0% {
        transform-origin: 50% 1400px;
        transform: matrix(1, 0, 0, 1, 0, 0)
    }

    to {
        transform-origin: 50% 100%;
        transform: translateY(-600px)translate(50%)rotateX(-30deg)scale(0)
    }
}

which breaks the animation. Still looking for a solution/workaround.

josip avatar Nov 09 '22 15:11 josip

FWIW my workaround was to avoid starting the animation with values at 0 -- since my animation loops I went from eg -180deg to +180deg instead of 0deg to 360deg. It's still spinning in a circle so it looks the same, but the minimiser doesn't break it :)

(Aside - was reading more about lightningcss and lol'ed at this from the README - "Note that some [other tools] perform unsafe optimizations that may change the behavior of the original CSS in favor of smaller file size. Lightning CSS does not do this – the output CSS should always behave identically to the input. Keep this in mind when comparing file sizes between tools.")

shish avatar Nov 10 '22 18:11 shish

Did either of you find a generic workaround for this? Here's the nightmare I'm dealing with...

@keyframes miniLoader-1 {
  0% {
    transform: translate3d(0, 0, 0) scale(0);
    opacity: 1;
  }
  to {
    transform: translate3d(0, 0, 0) scale(1.5);
    opacity: 0;
  }
}

into

@keyframes g5nwzq_miniLoader-1 {
  0% {
    opacity: 1;
    transform: matrix(0, 0, 0, 0, 0, 0);
  }
  to {
    opacity: 0;
    transform: scale(1.5);
  }
}

As far as I can tell, I don't really have the option to use degrees for this.

Edit: I can actually just use scale instead of transform for this, since I no longer need the translate3d. Still would be nice to be able to ignore a block or something, since we didn't have this problem with parcel v1.

aminomancer avatar Apr 05 '23 02:04 aminomancer

We lost a good chunk of time to this on a project this week as it was a nasty cause to pin down. Is there a suggested approach that someone could work on a PR for? Would it be OK to stop minifying all transforms down to matrix() or does it just need to get smarter and detect when it's being used in a keyframe animation?

(we did a scale(0.01) trick to fix this).

jalada avatar Aug 18 '23 15:08 jalada

I have the same problem. Is it possible to disable this behavior with a configuration option? Or ideally prevent the optimization if the property is inside a keyframes at-rule.

oscarotero avatar Jan 17 '24 10:01 oscarotero

In keyframes.rs:

impl<'i> KeyframesRule<'i> {
  pub(crate) fn minify(&mut self, context: &mut MinifyContext<'_, 'i>) {
    context.handler_context.context = DeclarationContext::Keyframes;

    for keyframe in &mut self.keyframes {
      keyframe
        .declarations
        .minify(context.handler, context.important_handler, &mut context.handler_context)
    }

    context.handler_context.context = DeclarationContext::None;
  }

I think we can just cut out the call to minify here? I know this won't minimize 'from' to '0%' anymore, but should keep it from breaking. I don't think there is any safe way to determine which properties that are being animated can be collapsed / simplified.

DanielJoyce avatar Jan 19 '24 00:01 DanielJoyce

Hmm, I think on the other end, we can look at the context in Transform and not minify if the context is DeclarationContext::KeyFrames?

DanielJoyce avatar Jan 19 '24 00:01 DanielJoyce

In general, I also think some kind of ignore facility, whether global, or comment based would be good too.

DanielJoyce avatar Jan 19 '24 00:01 DanielJoyce

The actual problem begins at line 60 of transform.rs, which will ignore context and always collapse to a transform if minify is true.

DanielJoyce avatar Jan 19 '24 00:01 DanielJoyce

Minification should be moved to 1688 of transform.rs instead of being done in TransformList toCss. I think.

In there we can look at the context and check if it is a KeyFrame context and not minimize, or otherwise minimize the transformlist.

DanielJoyce avatar Jan 19 '24 04:01 DanielJoyce

I have a draft PR #665 which is mostly to get the discussion going.

This disables minification when emitting declarations of keyframes. It's a hack, but it's to see if this fixes the issue, and discuss what the real solution should be.

DanielJoyce avatar Jan 19 '24 04:01 DanielJoyce

Anoither option is to just not minify TransformList for now. Or only minify white space, and do no other conversion.

DanielJoyce avatar Jan 19 '24 05:01 DanielJoyce

I guess we should just disable the part of the code that converts transforms to matrices and back again: https://github.com/parcel-bundler/lightningcss/blob/0c05ba8620f427e4a68bff05cfebe77bd35eef6f/src/properties/transform.rs#L60-L117 Maybe we can just comment that out for now?

Unfortunate that this optimization breaks animations. I don't think disabling it inside keyframes would be enough because this would also affect CSS transitions. Perhaps there is a way to to this more safely somehow, or make it opt in, but for now we can just disable it.

devongovett avatar Jan 28 '24 22:01 devongovett