Remove the layer dependency in SampleAnimation

RESOLVED FIXED in Firefox 53

Status

()

Core
Graphics: WebRender
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: pchang, Assigned: pchang)

Tracking

(Blocks: 1 bug)

Trunk
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [gfx-noted])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

a year ago
Add sampleanimation support in WebRenderBridgeParent to support OMTA
(Assignee)

Updated

a year ago
Blocks: 1311790
Depends on: 1325022
(Assignee)

Updated

a year ago
Blocks: 1328231
(Assignee)

Comment 1

a year ago
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.
Assignee: nobody → howareyou322
Whiteboard: [gfx-noted]
(Assignee)

Comment 2

a year ago
Created attachment 8825329 [details] [diff] [review]
Remove the layer dependency for SampleAnimation

MozReview-Commit-ID: 78D3c7t8tIi
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8825329 - Attachment is obsolete: true
(Assignee)

Comment 4

a year ago
(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

a year ago
mozreview-review
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?
Attachment #8825623 - Flags: review?(bbirtles) → review+
(Assignee)

Comment 6

a year ago
mozreview-review-reply
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 hidden (mozreview-request)
(Assignee)

Comment 8

a year ago
I would like to land this bug first and create another bug to call new SampleAnimationForEachNode in WebRenderBridgeParent.
No longer depends on: 1325022
Summary: Add sampleanimation support in WebRenderBridgeParent → Remove the layer dependency in SampleAnimation

Comment 9

a year ago
Pushed by pchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/642f5d4cf6fc
Remove the layer dependency for SampleAnimation, r=birtles

Comment 10

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/642f5d4cf6fc
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
(Assignee)

Updated

a year ago
Blocks: 1333355
You need to log in before you can comment on or make changes to this bug.