Bug 1339578 - Remove min active layer size for css animations; r?mattwoodrow draft
authorJamie Nicol <jnicol@mozilla.com>
Thu, 09 Feb 2017 18:00:32 +0000
changeset 487097 b9fe3a504ea2db9bb4489c7c171e75302406fb99
parent 487087 d0462b0948e0b1147dcce615bddcc46379bdadb2
child 546382 a1fc8eed61f686cfa2abdbdff4a4a31252af5edc
push id46132
push userbmo:jnicol@mozilla.com
push dateMon, 20 Feb 2017 18:34:46 +0000
reviewersmattwoodrow
bugs1339578
milestone54.0a1
Bug 1339578 - Remove min active layer size for css 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
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/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
@@ -8,17 +8,18 @@ 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)
+## (%3$S, %4$S) is a pair of integer values of the minimum frame size for animations to run on the compositor
+CompositorAnimationWarningContentTooSmall=Animation cannot be run on the compositor because frame size (%1$S, %2$S) is smaller than (%3$S, %4$S)
 ## 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
@@ -547,16 +547,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
@@ -5039,19 +5039,19 @@ 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);
+    nsIntSize(gfxPrefs::LayoutMinActiveLayerSize(),
+              gfxPrefs::LayoutMinActiveLayerSize());
 }
 
 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
@@ -5062,26 +5062,28 @@ SetAnimationPerformanceWarningForTooSmal
   // 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() }));
+        { visibleDevPixels.Width(), visibleDevPixels.Height(),
+          gfxPrefs::LayoutMinActiveLayerSize(), gfxPrefs::LayoutMinActiveLayerSize() }));
 }
 
 /* static */ bool
 nsDisplayOpacity::NeedsActiveLayer(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame)
 {
-  if (ActiveLayerTracker::IsStyleAnimated(aBuilder, aFrame,
-                                          eCSSProperty_opacity) ||
-      EffectCompositor::HasAnimationsForCompositor(aFrame,
+  if (EffectCompositor::HasAnimationsForCompositor(aFrame,
                                                    eCSSProperty_opacity)) {
+    return true;
+  } else if (ActiveLayerTracker::IsStyleAnimated(aBuilder, aFrame,
+                                                 eCSSProperty_opacity)) {
     if (!IsItemTooSmallForActiveLayer(aFrame)) {
       return true;
     }
     SetAnimationPerformanceWarningForTooSmallItem(aFrame, eCSSProperty_opacity);
   }
   return false;
 }
 
@@ -6708,23 +6710,26 @@ 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,
+  // Try to avoid doing work on the main thread for this animation.
+  if (EffectCompositor::HasAnimationsForCompositor(mFrame,
                                                    eCSSProperty_transform)) {
+    return true;
+  } else if (ActiveLayerTracker::IsStyleAnimated(aBuilder,
+                                                 mFrame,
+                                                 eCSSProperty_transform)) {
+    // Work is being done on the main thread anyway in this case,
+    // so check if the *post-transform* bounds of this item are big enough
+    // to justify an active layer.
     if (!IsItemTooSmallForActiveLayer(mFrame)) {
       return true;
     }
     SetAnimationPerformanceWarningForTooSmallItem(mFrame, eCSSProperty_transform);
   }
   return false;
 }