Bug 1339578 - Remove min active layer size for animations; r=mattwoodrow
authorJamie Nicol <jnicol@mozilla.com>
Thu, 09 Feb 2017 18:00:32 +0000
changeset 373280 0b55ea90ed2aba6c3bc79bd297a3389d8ed3cfdf
parent 373279 735f81d9fd96c9d19d3f2fdfda29a14c1a066d22
child 373281 c1d68f17f3c76513f1913dfdbefa28691eda7f2a
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1339578
milestone54.0a1
Bug 1339578 - Remove min active layer size for animations; r=mattwoodrow Even for very small layers we want to avoid doing work on the main thread. At the same time, however, increase the minimum active layer size for animations which come from restyles. These involve the main thread anyway, so there is less to be gained from using an active layer. Since switching items between active and inactive can have large knock-on effects, we want to make sure it really is worth making the layer active. MozReview-Commit-ID: 8N6xlVW4Dp3
devtools/server/tests/browser/animation.html
dom/animation/AnimationPerformanceWarning.cpp
dom/animation/AnimationPerformanceWarning.h
dom/animation/test/chrome/file_animation_performance_warning.html
dom/locales/en-US/chrome/layout/layout_errors.properties
gfx/thebes/gfxPrefs.h
layout/painting/nsDisplayList.cpp
--- a/devtools/server/tests/browser/animation.html
+++ b/devtools/server/tests/browser/animation.html
@@ -7,18 +7,18 @@
     height: 50px;
     border-radius: 50%;
     background: #eee;
   }
 
   .simple-animation {
     display: inline-block;
 
-    width: 50px;
-    height: 50px;
+    width: 64px;
+    height: 64px;
     border-radius: 50%;
     background: red;
 
     animation: move 200s infinite;
   }
 
   .multiple-animations {
     display: inline-block;
--- a/dom/animation/AnimationPerformanceWarning.cpp
+++ b/dom/animation/AnimationPerformanceWarning.cpp
@@ -28,23 +28,16 @@ AnimationPerformanceWarning::ToLocalized
 
 bool
 AnimationPerformanceWarning::ToLocalizedString(
   nsXPIDLString& aLocalizedString) const
 {
   const char* key = nullptr;
 
   switch (mType) {
-    case Type::ContentTooSmall:
-      MOZ_ASSERT(mParams && mParams->Length() == 2,
-                 "Parameter's length should be 2 for ContentTooSmall");
-
-      return NS_SUCCEEDED(
-        ToLocalizedStringWithIntParams<2>(
-          "CompositorAnimationWarningContentTooSmall", aLocalizedString));
     case Type::ContentTooLarge:
       MOZ_ASSERT(mParams && mParams->Length() == 6,
                  "Parameter's length should be 6 for ContentTooLarge");
 
       return NS_SUCCEEDED(
         ToLocalizedStringWithIntParams<7>(
           "CompositorAnimationWarningContentTooLarge2", aLocalizedString));
     case Type::TransformBackfaceVisibilityHidden:
--- a/dom/animation/AnimationPerformanceWarning.h
+++ b/dom/animation/AnimationPerformanceWarning.h
@@ -15,17 +15,16 @@
 class nsXPIDLString;
 
 namespace mozilla {
 
 // Represents the reason why we can't run the CSS property on the compositor.
 struct AnimationPerformanceWarning
 {
   enum class Type : uint8_t {
-    ContentTooSmall,
     ContentTooLarge,
     TransformBackfaceVisibilityHidden,
     TransformPreserve3D,
     TransformSVG,
     TransformWithGeometricProperties,
     TransformWithSyncGeometricAnimations,
     TransformFrameInactive,
     OpacityFrameInactive,
--- a/dom/animation/test/chrome/file_animation_performance_warning.html
+++ b/dom/animation/test/chrome/file_animation_performance_warning.html
@@ -835,47 +835,45 @@ function testMultipleAnimationsWithGeome
       });
     }, 'Multiple async animations and geometric animation: ' + subtest.desc);
   });
 }
 
 function testSmallElements() {
   [
     {
-      desc: 'opacity on too small element',
+      desc: 'opacity on small element',
       frames: {
         opacity: [0, 1]
       },
       style: { style: 'width: 8px; height: 8px; background-color: red;' +
                       // We need to set transform here to try creating an
                       // individual frame for this opacity element.
                       // Without this, this small element is created on the same
                       // nsIFrame of mochitest iframe, i.e. the document which are
                       // running this test, as a result the layer corresponding
                       // to the frame is sent to compositor.
                       'transform: translateX(100px);' },
       expected: [
         {
           property: 'opacity',
-          runningOnCompositor: false,
-          warning: /Animation cannot be run on the compositor because frame size \(8, 8\) is smaller than \(16, 16\)/
+          runningOnCompositor: true
         }
       ]
     },
     {
-      desc: 'transform on too small element',
+      desc: 'transform on small element',
       frames: {
         transform: ['translate(0px)', 'translate(100px)']
       },
       style: { style: 'width: 8px; height: 8px; background-color: red;' },
       expected: [
         {
           property: 'transform',
-          runningOnCompositor: false,
-          warning: /Animation cannot be run on the compositor because frame size \(8, 8\) is smaller than \(16, 16\)/
+          runningOnCompositor: true
         }
       ]
     },
   ].forEach(subtest => {
     promise_test(function(t) {
     var div = addDiv(t, subtest.style);
     var animation = div.animate(subtest.frames, 100 * MS_PER_SEC);
       return animation.ready.then(function() {
--- a/dom/locales/en-US/chrome/layout/layout_errors.properties
+++ b/dom/locales/en-US/chrome/layout/layout_errors.properties
@@ -6,19 +6,16 @@ ImageMapRectBoundsError=The “coords” attribute of the <area shape="rect"> tag is not in the “left,top,right,bottom” format.
 ImageMapCircleWrongNumberOfCoords=The “coords” attribute of the <area shape="circle"> tag is not in the “center-x,center-y,radius” format.
 ImageMapCircleNegativeRadius=The “coords” attribute of the <area shape="circle"> tag has a negative radius.
 ImageMapPolyWrongNumberOfCoords=The “coords” attribute of the <area shape="poly"> tag is not in the “x1,y1,x2,y2 …” format.
 ImageMapPolyOddNumberOfCoords=The “coords” attribute of the <area shape="poly"> tag is missing the last “y” coordinate (the correct format is “x1,y1,x2,y2 …”).
 
 TablePartRelPosWarning=Relative positioning of table rows and row groups is now supported. This site may need to be updated because it may depend on this feature having no effect.
 ScrollLinkedEffectFound2=This site appears to use a scroll-linked positioning effect. This may not work well with asynchronous panning; see https://developer.mozilla.org/docs/Mozilla/Performance/ScrollLinkedEffects for further details and to join the discussion on related tools and features!
 
-## LOCALIZATION NOTE(CompositorAnimationWarningContentTooSmall):
-## (%1$S, %2$S) is a pair of integer values of the frame size
-CompositorAnimationWarningContentTooSmall=Animation cannot be run on the compositor because frame size (%1$S, %2$S) is smaller than (16, 16)
 ## LOCALIZATION NOTE(CompositorAnimationWarningContentTooLarge2):
 ## (%1$S, %2$S) is a pair of integer values of the frame size
 ## (%3$S, %4$S) is a pair of integer values of a limit based on the viewport size
 ## (%5$S, %6$S) is a pair of integer values of an absolute limit
 CompositorAnimationWarningContentTooLarge2=Animation cannot be run on the compositor because the frame size (%1$S, %2$S) is too large relative to the viewport (larger than (%3$S, %4$S)) or larger than the maximum allowed value (%5$S, %6$S)
 ## LOCALIZATION NOTE(CompositorAnimationWarningTransformBackfaceVisibilityHidden):
 ## 'backface-visibility: hidden' is a CSS property, don't translate it.
 CompositorAnimationWarningTransformBackfaceVisibilityHidden=Animations of ‘backface-visibility: hidden’ transforms cannot be run on the compositor
--- a/gfx/thebes/gfxPrefs.h
+++ b/gfx/thebes/gfxPrefs.h
@@ -551,16 +551,17 @@ private:
   DECL_GFX_PREF(Live, "layout.css.scroll-snap.prediction-max-velocity", ScrollSnapPredictionMaxVelocity, int32_t, 2000);
   DECL_GFX_PREF(Live, "layout.css.scroll-snap.prediction-sensitivity", ScrollSnapPredictionSensitivity, float, 0.750f);
   DECL_GFX_PREF(Live, "layout.css.scroll-snap.proximity-threshold", ScrollSnapProximityThreshold, int32_t, 200);
   DECL_GFX_PREF(Live, "layout.css.touch_action.enabled",       TouchActionEnabled, bool, false);
   DECL_GFX_PREF(Live, "layout.display-list.dump",              LayoutDumpDisplayList, bool, false);
   DECL_GFX_PREF(Live, "layout.display-list.dump-content",      LayoutDumpDisplayListContent, bool, false);
   DECL_GFX_PREF(Live, "layout.event-regions.enabled",          LayoutEventRegionsEnabledDoNotUseDirectly, bool, false);
   DECL_GFX_PREF(Once, "layout.frame_rate",                     LayoutFrameRate, int32_t, -1);
+  DECL_GFX_PREF(Live, "layout.min-active-layer-size",          LayoutMinActiveLayerSize, int, 64);
   DECL_GFX_PREF(Once, "layout.paint_rects_separately",         LayoutPaintRectsSeparately, bool, true);
 
   // This and code dependent on it should be removed once containerless scrolling looks stable.
   DECL_GFX_PREF(Once, "layout.scroll.root-frame-containers",   LayoutUseContainersForRootFrames, bool, true);
   DECL_GFX_PREF(Live, "layout.smaller-painted-layers",         LayoutSmallerPaintedLayers, bool, false);
 
   DECL_GFX_PREF(Once, "media.hardware-video-decoding.force-enabled",
                                                                HardwareVideoDecodingForceEnabled, bool, false);
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -5207,53 +5207,30 @@ nsDisplayOpacity::BuildLayer(nsDisplayLi
  * rendered at a higher (or lower) resolution, affecting the retained layer
  * size --- but this should be good enough.
  */
 static bool
 IsItemTooSmallForActiveLayer(nsIFrame* aFrame)
 {
   nsIntRect visibleDevPixels = aFrame->GetVisualOverflowRectRelativeToSelf().ToOutsidePixels(
           aFrame->PresContext()->AppUnitsPerDevPixel());
-  static const int MIN_ACTIVE_LAYER_SIZE_DEV_PIXELS = 16;
   return visibleDevPixels.Size() <
-    nsIntSize(MIN_ACTIVE_LAYER_SIZE_DEV_PIXELS, MIN_ACTIVE_LAYER_SIZE_DEV_PIXELS);
-}
-
-static void
-SetAnimationPerformanceWarningForTooSmallItem(nsIFrame* aFrame,
-                                              nsCSSPropertyID aProperty)
-{
-  // We use ToNearestPixels() here since ToOutsidePixels causes some sort of
-  // errors. See https://bugzilla.mozilla.org/show_bug.cgi?id=1258904#c19
-  nsIntRect visibleDevPixels = aFrame->GetVisualOverflowRectRelativeToSelf().ToNearestPixels(
-          aFrame->PresContext()->AppUnitsPerDevPixel());
-
-  // Set performance warning only if the visible dev pixels is not empty
-  // because dev pixels is empty if the frame has 'preserve-3d' style.
-  if (visibleDevPixels.IsEmpty()) {
-    return;
-  }
-
-  EffectCompositor::SetPerformanceWarning(aFrame, aProperty,
-      AnimationPerformanceWarning(
-        AnimationPerformanceWarning::Type::ContentTooSmall,
-        { visibleDevPixels.Width(), visibleDevPixels.Height() }));
+    nsIntSize(gfxPrefs::LayoutMinActiveLayerSize(),
+              gfxPrefs::LayoutMinActiveLayerSize());
 }
 
 /* static */ bool
 nsDisplayOpacity::NeedsActiveLayer(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame)
 {
-  if (ActiveLayerTracker::IsStyleAnimated(aBuilder, aFrame,
-                                          eCSSProperty_opacity) ||
-      EffectCompositor::HasAnimationsForCompositor(aFrame,
-                                                   eCSSProperty_opacity)) {
-    if (!IsItemTooSmallForActiveLayer(aFrame)) {
-      return true;
-    }
-    SetAnimationPerformanceWarningForTooSmallItem(aFrame, eCSSProperty_opacity);
+  if (EffectCompositor::HasAnimationsForCompositor(aFrame,
+                                                   eCSSProperty_opacity) ||
+      (ActiveLayerTracker::IsStyleAnimated(aBuilder, aFrame,
+                                           eCSSProperty_opacity) &&
+       !IsItemTooSmallForActiveLayer(aFrame))) {
+    return true;
   }
   return false;
 }
 
 void
 nsDisplayOpacity::ApplyOpacity(nsDisplayListBuilder* aBuilder,
                              float aOpacity,
                              const DisplayItemClipChain* aClip)
@@ -6876,27 +6853,30 @@ already_AddRefed<Layer> nsDisplayTransfo
     container->SetContentFlags(container->GetContentFlags() & ~Layer::CONTENT_MAY_CHANGE_TRANSFORM);
   }
   return container.forget();
 }
 
 bool
 nsDisplayTransform::MayBeAnimated(nsDisplayListBuilder* aBuilder)
 {
-  // Here we check if the *post-transform* bounds of this item are big enough
-  // to justify an active layer.
-  if (ActiveLayerTracker::IsStyleAnimated(aBuilder,
-                                          mFrame,
-                                          eCSSProperty_transform) ||
-      EffectCompositor::HasAnimationsForCompositor(mFrame,
-                                                   eCSSProperty_transform)) {
-    if (!IsItemTooSmallForActiveLayer(mFrame)) {
-      return true;
-    }
-    SetAnimationPerformanceWarningForTooSmallItem(mFrame, eCSSProperty_transform);
+  // If EffectCompositor::HasAnimationsForCompositor() is true then we can
+  // completely bypass the main thread for this animation, so it is always
+  // worthwhile.
+  // For ActiveLayerTracker::IsStyleAnimated() cases the main thread is
+  // already involved so there is less to be gained.
+  // Therefore we check that the *post-transform* bounds of this item are
+  // big enough to justify an active layer.
+  if (EffectCompositor::HasAnimationsForCompositor(mFrame,
+                                                   eCSSProperty_transform) ||
+      (ActiveLayerTracker::IsStyleAnimated(aBuilder,
+                                           mFrame,
+                                           eCSSProperty_transform) &&
+       !IsItemTooSmallForActiveLayer(mFrame))) {
+    return true;
   }
   return false;
 }
 
 nsDisplayItem::LayerState
 nsDisplayTransform::GetLayerState(nsDisplayListBuilder* aBuilder,
                                   LayerManager* aManager,
                                   const ContainerLayerParameters& aParameters) {