1258904 - Set a performance warning when transform or opacity layers are too small

Status: RESOLVED FIXED

(Core :: DOM: Animation, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

We need to set it here.
https://dxr.mozilla.org/mozilla-central/rev/ea6298e1b4f7e22ce2311b2b6a918822f0adb112/layout/base/nsDisplayList.cpp#5890

Comment 1

[Hiroyuki Ikezoe (:hiro)]

This size restriction seems not to work for compositor animations.

Bas, you were involved in bug 678859, do you think the size restriction should be effective for compositor?  If not, this bug will be invalid.

Comment 2

[Bas Schouten (:bas.schouten)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)
> This size restriction seems not to work for compositor animations.
> 
> Bas, you were involved in bug 678859, do you think the size restriction
> should be effective for compositor?  If not, this bug will be invalid.

That is probably better for performance and memory usage, yes.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Thank you, Bas! Then I will fix it in this bug.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

GetVisibleRect() which is used in IsItemTooSmallForActiveLayer returns (0, 0) if the frame has 'preserve-3d' style.

https://dxr.mozilla.org/mozilla-central/rev/ea6298e1b4f7e22ce2311b2b6a918822f0adb112/layout/base/nsDisplayList.cpp#4211

On the other hand aFrame->GetVisualOverflowRectRelativeToSelf() in ShouldPrerenderTransformedContent() seems to return a correct value even if 'preserve-3d' is there.

https://dxr.mozilla.org/mozilla-central/rev/ea6298e1b4f7e22ce2311b2b6a918822f0adb112/layout/base/nsDisplayList.cpp#5710

Matt, should we also use GetVisualOverflowRectRelativeToSelf in IsItemTooSmallForActiveLayer?

Comment 5

[Matt Woodrow (:mattwoodrow)]

Don't we always force an active layer for preserve-3d anyway?

We also disable OMTA for preserve-3d, so I don't think this matters.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: MozReview Request: Bug 1258904 - Part 1: Don't create layers for compositor animations if the layer size is less than 16x16. r?mattwoodrow

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

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: MozReview Request: Bug 1258904 - Part 3: Factor out ToLocalizedStringForKey. r?birtles

This function will be used for the warning of small content.

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

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: MozReview Request: Bug 1258904 - Part 4: Set performance warning for small content. r?birtles

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

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: MozReview Request: Bug 1258904 - Part 5: Remove redundant tests for animation performance warnings. r?birtles

There were two places run gAnimationsTests in
test_animation_performance_warning.html.

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

Comment 10

[Hiroyuki Ikezoe (:hiro)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> Don't we always force an active layer for preserve-3d anyway?
> 
> We also disable OMTA for preserve-3d, so I don't think this matters.

Thank you, Matt.  I have another question about GetVisibleRect().
In a test case in part 3 patch, an element whose width and height are 8px with transform animation is created.  In that case GetVisibleRect().ToOutsidePixels(AppUnitsPerDevPixel()) sometimes returns 9px.  I don't investigate what is the trigger of the wrong size yet.  As far as I can tell,  only 'width' seems to be wrong.  Is this a known issue or should I file it as a new bug?

Comment 11

[Matt Woodrow (:mattwoodrow)]

Check what the top/left values are in app units?

If it's 8px, but the top/left isn't at a pixel edge (multiple of AppUnitsPerDevPixel()), then rounding out to pixels might make it 9.

Comment 12

[Bas Schouten (:bas.schouten)]

Comment on attachment 8739831 [details]
MozReview Request: Bug 1258904 - Part 1: Don't create layers for compositor animations if the layer size is less than 16x16. r?mattwoodrow

I feel Matt is better qualified to review this even though I feel this should be fine.

Comment 13

[Hiroyuki Ikezoe (:hiro)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #11)
> Check what the top/left values are in app units?
> 
> If it's 8px, but the top/left isn't at a pixel edge (multiple of
> AppUnitsPerDevPixel()), then rounding out to pixels might make it 9.

The width in app units was 481 in case when we get '9px'.  left was 1260, top was 20880 at that time. AppUnitsPerDevPixel is 60.

Comment 14

[Hiroyuki Ikezoe (:hiro)]

A try result, there were two problems.  One is new test cases for this bug failed on Android.  The other is the new test cases broke all subsequent test cases on Linux non-E10S.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae0e27e02b91a15e5d3da90225d8633300d1147c

The failure reason on Android is related device DPI.  We should set layout.css.devPixelsPerPx to 1 in test.  I confirmed that setting the preference fixes the failure on Android locally.

The failure reason of the second one is not yet know.

Comment 15

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8739831 [details]
MozReview Request: Bug 1258904 - Part 1: Don't create layers for compositor animations if the layer size is less than 16x16. r?mattwoodrow

https://reviewboard.mozilla.org/r/45389/#review42143

Comment 16

[Matt Woodrow (:mattwoodrow)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #13)
 
> The width in app units was 481 in case when we get '9px'.  left was 1260,
> top was 20880 at that time. AppUnitsPerDevPixel is 60.

So we're a single app unit bigger than 8px.

Sounds like a rounding bug, float precision maybe?

You'll have to debug where it comes from.

Comment 17

[Hiroyuki Ikezoe (:hiro)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #16)
> (In reply to Hiroyuki Ikezoe (:hiro) from comment #13)
>  
> > The width in app units was 481 in case when we get '9px'.  left was 1260,
> > top was 20880 at that time. AppUnitsPerDevPixel is 60.
> 
> So we're a single app unit bigger than 8px.
> 
> Sounds like a rounding bug, float precision maybe?

It might not be a float precision problem.  aProperties.mTransformList has non-zero value, I see 0.00928551611 now there, this small value causes the error. 

https://hg.mozilla.org/mozilla-central/file/tip/layout/base/nsDisplayList.cpp#l5533

Comment 18

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #17)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #16)
> > (In reply to Hiroyuki Ikezoe (:hiro) from comment #13)
> >  
> > > The width in app units was 481 in case when we get '9px'.  left was 1260,
> > > top was 20880 at that time. AppUnitsPerDevPixel is 60.
> > 
> > So we're a single app unit bigger than 8px.
> > 
> > Sounds like a rounding bug, float precision maybe?
> 
> It might not be a float precision problem.  aProperties.mTransformList has
> non-zero value, I see 0.00928551611 now there, this small value causes the
> error. 

To be more precise aProperties.mTransformList->..->Item(1) has the non-zero value. All of other values are zeo.  I am now tracking where the non-zero comes from.

Comment 19

[Hiroyuki Ikezoe (:hiro)]

Ah, I did not know that the value is the transform specified in test case itself.  That means the error comes from the difference NSToIntFloor and NSToIntCeil in nsRect::ScaleToOutsidePixels.

https://dxr.mozilla.org/mozilla-central/rev/e847cfcb315f511f4928b03fd47dcf57aad05e1e/gfx/src/nsRect.h#243

Comment 20

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

Comment on attachment 8739832 [details]
MozReview Request: Bug 1258904 - Part 3: Factor out ToLocalizedStringForKey. r?birtles

https://reviewboard.mozilla.org/r/45391/#review42255

r=birtles with comments addressed

::: dom/animation/AnimationPerformanceWarning.cpp:13
(Diff revision 1)
> +nsresult
> +AnimationPerformanceWarning::ToLocalizedStringForKey(
> +  const char* aKey, nsXPIDLString& aLocalizedString) const

Maybe we should call this ToLocalizedStringWithIntParams?

(I was thinking that we could actually templatize this and make the number of parameters a template argument. Then we wouldn't need kMaxParamsForLocalization, I think. However, it would introduce greater code bloat, I guess because we'd end up generating two versions of this function. We'd also have to pass mParams by const ref but I think that's ok? It's up to you if you think that's worthwhile.)

::: dom/animation/AnimationPerformanceWarning.cpp:17
(Diff revision 1)
>  
> +nsresult
> +AnimationPerformanceWarning::ToLocalizedStringForKey(
> +  const char* aKey, nsXPIDLString& aLocalizedString) const
> +{
> +  // We can pass an array of parameters whose length is greater than 7 to

Is '7' still significant here?

::: dom/animation/AnimationPerformanceWarning.cpp:43
(Diff revision 1)
>        MOZ_ASSERT(mParams->Length() <= kMaxParamsForLocalization,
>                   "Parameter's length should be less than "
>                   "kMaxParamsForLocalization");

I think we should move this assertion to ToLocalizedStringForKey?

Comment 21

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

Comment on attachment 8739833 [details]
MozReview Request: Bug 1258904 - Part 4: Set performance warning for small content. r?birtles

https://reviewboard.mozilla.org/r/45393/#review42257

::: dom/animation/AnimationPerformanceWarning.cpp:43
(Diff revision 1)
> +      MOZ_ASSERT(mParams->Length() <= kMaxParamsForLocalization,
> +                 "Parameter's length should be less than "
> +                 "kMaxParamsForLocalization");

As with the previous patch, I think we can move this to ToLocalizedStringForKey (unless we templatize that method, in which case we might not need this at all).

::: dom/animation/test/chrome/test_animation_performance_warning.html:504
(Diff revision 1)
> +                    // We need to set transform here to try creating an
> +                    // individual layer for this opacity element.
> +                    'transform: translateX(100px);' },

Why is this needed? I don't quite follow.

::: dom/animation/test/chrome/test_animation_performance_warning.html:511
(Diff revision 1)
> +                    'transform: translateX(100px);' },
> +    expected: [
> +      {
> +        property: 'opacity',
> +        runningOnCompositor: false,
> +        warning: /Async animation disabled because frame size \(8, 8\) is smaller than \(16, 16\)/

I thought 'warning' could be a string too? That would save escaping the braces?

Comment 22

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

Comment on attachment 8739834 [details]
MozReview Request: Bug 1258904 - Part 5: Remove redundant tests for animation performance warnings. r?birtles

https://reviewboard.mozilla.org/r/45395/#review42259

Comment 23

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #14)
> A try result, there were two problems.  One is new test cases for this bug
> failed on Android.  The other is the new test cases broke all subsequent
> test cases on Linux non-E10S.
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=ae0e27e02b91a15e5d3da90225d8633300d1147c
> 
> The failure reason on Android is related device DPI.  We should set
> layout.css.devPixelsPerPx to 1 in test.  I confirmed that setting the
> preference fixes the failure on Android locally.
> 
> The failure reason of the second one is not yet know.

I've finally found the reason why tests which are subsequent to small element test cases fails.

With part 1 patch, once a small element is created, NS_FRAME_NO_COMPONENT_ALPHA is set against container frame in FrameLayerBuilder::BuildContainerLayerFor.  As a result, elements created in the subsequent tests are not sent to the compositor at all.

According to bug 741682#c9 and bug 741682#c7, NS_FRAME_NO_COMPONENT_ALPHA remains while the container frame is alive.  And it's hard to tell when it should be cleared (I think).  So now I am inclined to introduce a new flag named isTooSmallForActiveLayer into nsDisplayItem, check it and set mDisableFlattening if it's true.

Comment 24

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #23)
>  So now I am inclined to introduce a new flag
> named isTooSmallForActiveLayer into nsDisplayItem, check it and set
> mDisableFlattening if it's true.

I am going to upload whole patches including this part as part 2 from now on, I am really hoping MozReview does not break anything.

Comment 25

[Hiroyuki Ikezoe (:hiro)]

Attachment: MozReview Request: Bug 1258904 - Part 2: nsDisplay(Opacity|Transform)::GetLayerState should return LAYER_ACTIVE_FORCE if they have async animations to avoid flattening the layers.

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

Comment 26

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739831 [details]
MozReview Request: Bug 1258904 - Part 1: Don't create layers for compositor animations if the layer size is less than 16x16. r?mattwoodrow

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

Comment 27

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739832 [details]
MozReview Request: Bug 1258904 - Part 3: Factor out ToLocalizedStringForKey. r?birtles

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

Comment 28

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739833 [details]
MozReview Request: Bug 1258904 - Part 4: Set performance warning for small content. r?birtles

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

Comment 29

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739834 [details]
MozReview Request: Bug 1258904 - Part 5: Remove redundant tests for animation performance warnings. r?birtles

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

Comment 30

[Hiroyuki Ikezoe (:hiro)]

https://reviewboard.mozilla.org/r/45391/#review42255

> Maybe we should call this ToLocalizedStringWithIntParams?
> 
> (I was thinking that we could actually templatize this and make the number of parameters a template argument. Then we wouldn't need kMaxParamsForLocalization, I think. However, it would introduce greater code bloat, I guess because we'd end up generating two versions of this function. We'd also have to pass mParams by const ref but I think that's ok? It's up to you if you think that's worthwhile.)

Thanks!  I made the function template function.

Comment 31

[Hiroyuki Ikezoe (:hiro)]

https://reviewboard.mozilla.org/r/45393/#review42257

> I thought 'warning' could be a string too? That would save escaping the braces?

Unfortunetely it can't because 'string' is used to convert the string to localized string there.

Comment 32

[Matt Woodrow (:mattwoodrow)]

I don't understand part 2.

mDisableFlattening is part of a BasicCompositor specific optimization, where we 'flatten' active layers together to prevent us ending up with subpixel-AA over a transparent background in the final layer tree.

We don't want to block this flattening because we have a small display item somewhere in the display list.

mIsTooSmallForActiveLayer also appears to be the reverse of what the name suggests, since it's set to true when we're *not* too small.

Comment 33

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8742628 [details]
MozReview Request: Bug 1258904 - Part 2: nsDisplay(Opacity|Transform)::GetLayerState should return LAYER_ACTIVE_FORCE if they have async animations to avoid flattening the layers.

(In reply to Matt Woodrow (:mattwoodrow) from comment #32)
> I don't understand part 2.
> 
> mDisableFlattening is part of a BasicCompositor specific optimization, where
> we 'flatten' active layers together to prevent us ending up with subpixel-AA
> over a transparent background in the final layer tree.
> 
> We don't want to block this flattening because we have a small display item
> somewhere in the display list.

Thank you, Matt.  I will consider other approach here.

> mIsTooSmallForActiveLayer also appears to be the reverse of what the name
> suggests, since it's set to true when we're *not* too small.

This was totally wrong. The test case I checked must be passed accidentally.

Comment 34

[Matt Woodrow (:mattwoodrow)]

What are you trying to solve? I can try suggest something.

Comment 35

[Hiroyuki Ikezoe (:hiro)]

What I am trying to fix is an issue in comment 23 which I've been seeing on non-E10S.  The issue is that layers for small display items are flatten in the failure case.  

I am now looking at mPropagateComponentAlphaFlattening, it is set to false if the display item is transfrom  at https://dxr.mozilla.org/mozilla-central/rev/67ac40fb8f680ea5e03805552187ba1b5e8392a1/layout/base/FrameLayerBuilder.cpp#4125 .  But it seems to me that aHasComponentAlphaChildren is set to true in ContainerState::Finish() if there is at least one layer whose mPropagateComponentAlphaFlattening is true despite other layer's mPropagateComponentAlphaFlattening state.  I might be missing something though.

Comment 36

[Matt Woodrow (:mattwoodrow)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #35)
> What I am trying to fix is an issue in comment 23 which I've been seeing on
> non-E10S.  The issue is that layers for small display items are flatten in
> the failure case.  
> 
> I am now looking at mPropagateComponentAlphaFlattening, it is set to false
> if the display item is transfrom  at
> https://dxr.mozilla.org/mozilla-central/rev/
> 67ac40fb8f680ea5e03805552187ba1b5e8392a1/layout/base/FrameLayerBuilder.
> cpp#4125 .  But it seems to me that aHasComponentAlphaChildren is set to
> true in ContainerState::Finish() if there is at least one layer whose
> mPropagateComponentAlphaFlattening is true despite other layer's
> mPropagateComponentAlphaFlattening state.  I might be missing something
> though.

You can return LAYER_ACTIVE_FORCE from GetLayerState if you feel that the layer being active is more important than getting subpixel-AA for the text (which I feel like it is for this case).

Comment 37

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739831 [details]
MozReview Request: Bug 1258904 - Part 1: Don't create layers for compositor animations if the layer size is less than 16x16. r?mattwoodrow

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

Comment 38

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8742628 [details]
MozReview Request: Bug 1258904 - Part 2: nsDisplay(Opacity|Transform)::GetLayerState should return LAYER_ACTIVE_FORCE if they have async animations to avoid flattening the layers.

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

Comment 39

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739832 [details]
MozReview Request: Bug 1258904 - Part 3: Factor out ToLocalizedStringForKey. r?birtles

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

Comment 40

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739833 [details]
MozReview Request: Bug 1258904 - Part 4: Set performance warning for small content. r?birtles

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

Comment 41

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739834 [details]
MozReview Request: Bug 1258904 - Part 5: Remove redundant tests for animation performance warnings. r?birtles

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

Comment 42

[Hiroyuki Ikezoe (:hiro)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #36)
> (In reply to Hiroyuki Ikezoe (:hiro) from comment #35)
> > What I am trying to fix is an issue in comment 23 which I've been seeing on
> > non-E10S.  The issue is that layers for small display items are flatten in
> > the failure case.  
> > 
> > I am now looking at mPropagateComponentAlphaFlattening, it is set to false
> > if the display item is transfrom  at
> > https://dxr.mozilla.org/mozilla-central/rev/
> > 67ac40fb8f680ea5e03805552187ba1b5e8392a1/layout/base/FrameLayerBuilder.
> > cpp#4125 .  But it seems to me that aHasComponentAlphaChildren is set to
> > true in ContainerState::Finish() if there is at least one layer whose
> > mPropagateComponentAlphaFlattening is true despite other layer's
> > mPropagateComponentAlphaFlattening state.  I might be missing something
> > though.
> 
> You can return LAYER_ACTIVE_FORCE from GetLayerState if you feel that the
> layer being active is more important than getting subpixel-AA for the text
> (which I feel like it is for this case).

Thank you, Matt!  Firstly I did not notice that CanUseAsyncAnimations has to return false in case of LAYER_ACTIVE_FORCE.  Now returning LAYER_ACTIVE_FORCE works fine.

Comment 43

[Matt Woodrow (:mattwoodrow)]

Sorry, I meant to return LAYER_ACTIVE_FORCE from the MayBeAnimated() test in GetLayerState, that should be all you need.

Full explanation:

When we get a layer that contains text without an opaque background behind it, we can't easily render the text using subpixel-AA (since subpixel-AA needs to know the partial pixel colours behind the text to blend correctly).

For accelerated layers we render the layer twice (once on a black background, once on a white). The compositor can then use the two textures to recover the alpha channel and get the correct text. We call this 'component alpha' (since we're effectively retaining an alpha value for each r/g/b channel instead of only one for the pixel).

For BasicCompositor/BasicLayerManager this recovery process is too slow, so we don't bother. If we detect a layer that would require component alpha, we restart Layer building, but we convert LAYER_ACTIVE -> LAYER_INACTIVE (but not LAYER_ACTIVE_FORCE), and also disable active geometry roots. Hopefully this results in the component alpha layer no longer existing, but if it's still there then we just have to render the text without subpixel-AA.

In your broken test case, small (inactive?) transforms are the content that causes the layer to have component alpha, and thus trigger flattening. There are however, many other way to trigger flattening, background-attachment:fixed on the page background is the most common.

I think the right solution is to make sure that flattening doesn't prevent async animations from working (by 'forcing' an unflattenable layer using LAYER_ACTIVE_FORCE), rather than trying to stop the flattening from triggering (since there's too many ways it can happen, and it serves a useful purpose).

As for why this only happens with non-e10s, with e10s we decided that async scrolling was more valuable than subpixel-AA, so flattening never happens when APZ is enabled.

Comment 44

[Hiroyuki Ikezoe (:hiro)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #43)
> Sorry, I meant to return LAYER_ACTIVE_FORCE from the MayBeAnimated() test in
> GetLayerState, that should be all you need.

Thanks for the detailed explanation!  If I understand correctly the explanation, we need to return LAYER_ACTIVE_FORCE from NeedsActiveLayer() test in nsDisplayOpacity::GetLayerState() as well.  Right?

Comment 45

[Matt Woodrow (:mattwoodrow)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #44)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #43)
> > Sorry, I meant to return LAYER_ACTIVE_FORCE from the MayBeAnimated() test in
> > GetLayerState, that should be all you need.
> 
> Thanks for the detailed explanation!  If I understand correctly the
> explanation, we need to return LAYER_ACTIVE_FORCE from NeedsActiveLayer()
> test in nsDisplayOpacity::GetLayerState() as well.  Right?

Yes indeed!

Comment 46

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739831 [details]
MozReview Request: Bug 1258904 - Part 1: Don't create layers for compositor animations if the layer size is less than 16x16. r?mattwoodrow

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45389/diff/3-4/

Comment 47

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8742628 [details]
MozReview Request: Bug 1258904 - Part 2: nsDisplay(Opacity|Transform)::GetLayerState should return LAYER_ACTIVE_FORCE if they have async animations to avoid flattening the layers.

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

Comment 48

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739832 [details]
MozReview Request: Bug 1258904 - Part 3: Factor out ToLocalizedStringForKey. r?birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45391/diff/3-4/

Comment 49

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739833 [details]
MozReview Request: Bug 1258904 - Part 4: Set performance warning for small content. r?birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45393/diff/3-4/

Comment 50

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8739834 [details]
MozReview Request: Bug 1258904 - Part 5: Remove redundant tests for animation performance warnings. r?birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45395/diff/3-4/

Comment 51

[Hiroyuki Ikezoe (:hiro)]

Thank you, Matt. It seems to work fine now.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b02fd42f2b32

Comment 52

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8742628 [details]
MozReview Request: Bug 1258904 - Part 2: nsDisplay(Opacity|Transform)::GetLayerState should return LAYER_ACTIVE_FORCE if they have async animations to avoid flattening the layers.

https://reviewboard.mozilla.org/r/47383/#review47933

Comment 53

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2c119fc3431b
https://hg.mozilla.org/integration/mozilla-inbound/rev/05cb31c61bf2
https://hg.mozilla.org/integration/mozilla-inbound/rev/156b9e7e21c7
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4f185cfc034
https://hg.mozilla.org/integration/mozilla-inbound/rev/246fdb220ef3

Comment 54

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2c119fc3431b
https://hg.mozilla.org/mozilla-central/rev/05cb31c61bf2
https://hg.mozilla.org/mozilla-central/rev/156b9e7e21c7
https://hg.mozilla.org/mozilla-central/rev/f4f185cfc034
https://hg.mozilla.org/mozilla-central/rev/246fdb220ef3