Animate transform-origin on the compositor

NEW
Assigned to

Status

()

defect
7 years ago
7 years ago

People

(Reporter: dzbarsky, Assigned: dzbarsky)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

7 years ago
Posted patch Patch (obsolete) — Splinter Review
This should work, but this is getting very silly because of the animations/transitions split.
Assignee

Updated

7 years ago
Depends on: 755084
Assignee

Comment 1

7 years ago
Posted patch Patch (obsolete) — Splinter Review
Attachment #640772 - Attachment is obsolete: true
Assignee

Comment 2

7 years ago
Posted patch PatchSplinter Review
Attachment #641734 - Attachment is obsolete: true
Attachment #641736 - Flags: review?(roc)
Assignee

Updated

7 years ago
Attachment #641736 - Flags: review?(dbaron)
Comment on attachment 641736 [details] [diff] [review]
Patch

Review of attachment 641736 [details] [diff] [review]:
-----------------------------------------------------------------

r+ on the non-style-system stuff

::: gfx/layers/ipc/CompositorParent.cpp
@@ +438,5 @@
>      const_cast<InfallibleTArray<AnimData>&>(aLayer->GetAnimationData());
> +  gfxPoint3D interpolatedTransformOrigin;
> +
> +  //It is important that we iterate over the animations in reverse because we
> +  //added transform-origin animations after transform animations.

Space after //

::: layout/base/nsDisplayList.cpp
@@ +357,5 @@
> +                                                scale);
> +
> +        gfxPoint3D endOrigin = PointFromValue(segment->mToValue,
> +                                              frame->GetStyleContext(),
> +                                              frame->PresContext(),

No need to call GetStyleContext and PresContext twice
Attachment #641736 - Flags: review?(roc) → review+
Could you explain the approach you're taking for handling the different combinations of:
A:
  1. transform is animating
  2. transform is specified but not animating
  3. transform is not specified
B:
  1. transform-origin is animating
  2. transform-origin is specified but not animating
  3. transform-origin is not specified

Looking at SampleAnimations makes me think some of these aren't going to work, but I think there's a bit of complexity elsewhere that I'm not following that will make them work.  Could you explain the design here?
Assignee

Comment 5

7 years ago
A1. We add the keyframes to the transformSegments array, either add the transform-origin segments to the transformOriginSegments array or just take the single transform-origin and set one segment with that as both the start and end state, and animate both.

A2. If transform-origin is animating, we build the transform-origin segments and add the transform as start and end state.

A3. No animation occurs, because we will have a mSpecifiedTransform of null, so when we call AddTransformFunctions, we end up with an empty list of transforms as both the start and end state, which will get converted to an identity transform matrix.

B1. Same as case A1.

B2. We build the transformSegments, then add the transform-origin as a segment with start and end.

B3. Build the transformSegments, then add a transform-origin segment with a start and end state of GetDeltaToMozzTransformOrigin, which will put it at 50%, 50%.
Assignee

Comment 6

7 years ago
This will get even more interesting once I finish my patch to animate perspective and perspective-origin ;)
I was more interested in how each case from (A) interacted with each case from (B), though I think you described parts of that.

How do you deal with animations on more than one of the transform properties, but with different segment, duration, or start time data between properties?

Do you think there's a better way to do this that will scale better once you're animating all the transform properties?
Assignee

Comment 8

7 years ago
If there are keyframe rules for both different animation properties, I build up arrays of animation segments for each, and they get sampled independently on the compositor.  As long as the animation for transform-origin is sampled before transform, we will have the correct interpolated origin to use.

I haven't finished perspective yet so it's hard to tell how ugly it would be.  The issue is that perspective and perspective-origin are specified on the parent, so what I actually do is add perspective and perspective origin (and fill in a default transform and transform-origin), and then if there is also a transform or transform-origin animation, I remove the previously added animation and add the new one.  I have a 50 line comment describing the whole setup.

I'm going to have to think some more about how I can do this better.
Comment on attachment 641736 [details] [diff] [review]
Patch

I think we shouldn't move forward on this until we have tests for it, so marking review-.
Attachment #641736 - Flags: review?(dbaron) → review-
You need to log in before you can comment on or make changes to this bug.