1328229 - Remove the layer dependency in SampleAnimation

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Peter Chang[:pchang]]

Add sampleanimation support in WebRenderBridgeParent to support OMTA

Comment 1

[Peter Chang[:pchang]]

Inside AsyncCompositionManager.cpp, we need to remove the layer dependency from the static function 'SampleAnimation' and 'ApplyAnimatedValue'. After we doing this, we can share these logic with CompositorBridgeParent and WebRenderBridgeParent.

Comment 2

[Peter Chang[:pchang]]

Attachment: Remove the layer dependency for SampleAnimation

MozReview-Commit-ID: 78D3c7t8tIi

Comment 3

[Peter Chang[:pchang]]

Attachment: Bug 1328229 - Remove the layer dependency for SampleAnimation,

Review commit: https://reviewboard.mozilla.org/r/103740/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/103740/

Comment 4

[Peter Chang[:pchang]]

(In reply to Peter Chang[:pchang] from comment #1)
> Inside AsyncCompositionManager.cpp, we need to remove the layer dependency
> from the static function 'SampleAnimation' and 'ApplyAnimatedValue'. After
> we doing this, we can share these logic with CompositorBridgeParent and
> WebRenderBridgeParent.

Brian, as I mentioned above, we would like to support OMTA for Quantum Render which doesn't have layer information in the Parent side(WebRenderBridgeParent). Therefore, I have to do the refactor for the SampleAnimation function.

Comment 5

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Comment on attachment 8825623 [details]
Bug 1328229 - Remove the layer dependency for SampleAnimation,

https://reviewboard.mozilla.org/r/103740/#review104462

::: gfx/layers/composite/AsyncCompositionManager.cpp:778
(Diff revision 1)
> -                                       computedTiming.mCurrentIteration,
> +                                 computedTiming.mCurrentIteration,
> -                                       animationValue);
> -          hasInEffectAnimations = true;
> +                                 aAnimationValue);
> +    aHasInEffectAnimations = true;
> -        }
> +  }
>  
> -#ifdef DEBUG
> + #ifdef DEBUG

MozReview is suggesting this line and the following #endif are indented a space. Is that intentional?

::: gfx/layers/composite/AsyncCompositionManager.cpp:811
(Diff revision 1)
> +{
> +  bool activeAnimations = false;
> +
> +  ForEachNode<ForwardIterator>(
> +      aLayer,
> +      [&activeAnimations, aPoint] (Layer* layer)

Is dropping the reference semantics for aPoint intentional?

Comment 6

[Peter Chang[:pchang]]

Comment on attachment 8825623 [details]
Bug 1328229 - Remove the layer dependency for SampleAnimation,

https://reviewboard.mozilla.org/r/103740/#review104462

> MozReview is suggesting this line and the following #endif are indented a space. Is that intentional?

The space should be removed.

> Is dropping the reference semantics for aPoint intentional?

This should be typo error. Fixed.

Comment 7

[Peter Chang[:pchang]]

Comment on attachment 8825623 [details]
Bug 1328229 - Remove the layer dependency for SampleAnimation,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/103740/diff/1-2/

Comment 8

[Peter Chang[:pchang]]

I would like to land this bug first and create another bug to call new SampleAnimationForEachNode in WebRenderBridgeParent.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/642f5d4cf6fc
Remove the layer dependency for SampleAnimation, r=birtles

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/642f5d4cf6fc