Fix animation try failures for layers-free

RESOLVED FIXED in mozilla57

Status

()

Core
Graphics: WebRender
P1
normal
RESOLVED FIXED
9 months ago
7 months ago

People

(Reporter: pchang, Assigned: pchang)

Tracking

(Blocks: 1 bug)

Trunk
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 unaffected, firefox57 unaffected)

Details

(Whiteboard: [wr-mvp] gfx-noted)

MozReview Requests

()

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

Attachments

(2 attachments)

(Assignee)

Description

9 months ago
file:///home/worker/workspace/build/tests/reftest/tests/layout/reftests/transform/animate-layer-scale-inherit-1.html == file:///home/worker/workspace/build/tests/reftest/tests/layout/reftests/transform/animate-layer-scale-inherit-1-ref.html
(Assignee)

Updated

9 months ago
Blocks: 1389000
(Assignee)

Updated

9 months ago
Whiteboard: gfx-noted
(Assignee)

Updated

9 months ago
Assignee: nobody → howareyou322
(Assignee)

Comment 1

9 months ago
After considering the inherited scale from ancestors, the failures could be passed in my local. Later I will upload the patch for review.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Priority: -- → P3

Updated

9 months ago
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: gfx-noted → [wr-mvp] gfx-noted
Target Milestone: --- → mozilla57
status-firefox56: --- → unaffected
status-firefox57: affected → unaffected

Comment 4

8 months ago
mozreview-review
Comment on attachment 8905343 [details]
Bug 1394308 - Apply inherited scale for OMTA to support layers-free,

https://reviewboard.mozilla.org/r/177130/#review183282

::: gfx/layers/wr/StackingContextHelper.h:91
(Diff revision 1)
> +  // Export the inherited scale
> +  gfx::Size GetInheritedScale() const {
> +    return gfx::Size(mXScale, mYScale);
> +  }
> +
> +  // Provide interface to setup the inhertied scale to support

s/inhertied/inherited/

::: layout/painting/nsDisplayList.cpp:7841
(Diff revision 1)
> +    // Since we pass nullptr transform to stacking contenxt helper for OMTA,
> +    // therefore, we have to setup correct inherited scale for this
> +    // stacking context

I think it would be clearer if this comment read like so:

Since we passed a nullptr transformForSC to the StackingContextHelper, we now set up the correct inherited scale for the stacking context.

(Note that you have a typo "contenxt" in your existing comment also)

::: layout/painting/nsDisplayList.cpp:7844
(Diff revision 1)
> +    aManager->WrBridge()->AddWebRenderParentCommand(anim);
> +
> +    // Since we pass nullptr transform to stacking contenxt helper for OMTA,
> +    // therefore, we have to setup correct inherited scale for this
> +    // stacking context
> +    newTransformMatrix.PostScale(scale.width,scale.height, 1.0f);

nit: space after comma
Attachment #8905343 - Flags: review?(bugmail) → review+

Comment 5

8 months ago
mozreview-review
Comment on attachment 8905344 [details]
Bug 1394308 - Pass perspective transform attribute to compositor for OMTA,

https://reviewboard.mozilla.org/r/177132/#review183290

::: gfx/layers/wr/StackingContextHelper.h:100
(Diff revision 1)
>      mYScale = aScale.height;
>    }
>  
>    bool IsBackfaceVisible() const { return mTransform.IsBackfaceVisible(); }
>  
> +  bool HasPerspectiveTransform() const { return mHasPerspectiveTransform; }

nit: Move this up one line so that it's next to the IsBackfaceVisible() and there's a blank line between it and "private:"
Attachment #8905344 - Flags: review?(bugmail) → review+
(Assignee)

Comment 6

8 months ago
mozreview-review-reply
Comment on attachment 8905343 [details]
Bug 1394308 - Apply inherited scale for OMTA to support layers-free,

https://reviewboard.mozilla.org/r/177130/#review183282

> s/inhertied/inherited/

Fixed

> I think it would be clearer if this comment read like so:
> 
> Since we passed a nullptr transformForSC to the StackingContextHelper, we now set up the correct inherited scale for the stacking context.
> 
> (Note that you have a typo "contenxt" in your existing comment also)

Fixed

> nit: space after comma

Fixed
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 9

8 months ago
mozreview-review-reply
Comment on attachment 8905344 [details]
Bug 1394308 - Pass perspective transform attribute to compositor for OMTA,

https://reviewboard.mozilla.org/r/177132/#review183290

> nit: Move this up one line so that it's next to the IsBackfaceVisible() and there's a blank line between it and "private:"

Fixed

Comment 10

8 months ago
Pushed by pchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f5ae22906a1
Apply inherited scale for OMTA to support layers-free, r=kats
https://hg.mozilla.org/integration/autoland/rev/4b3ef99fb8ca
Pass perspective transform attribute to compositor for OMTA, r=kats
You need to log in before you can comment on or make changes to this bug.