Bug 1368551 - Don't send transforms to WR in push_stacking_context if they are identity. r=pchang
authorKartikaya Gupta <kgupta@mozilla.com>
Thu, 01 Jun 2017 09:17:17 -0400
changeset 361814 67f689dd202dca5fe70622a131b087408f7119f2
parent 361813 85e598624e0ffb8ed7532e2f1495dcf3e591e918
child 361815 db1f0de460e574f9fc9a0fd0fdf5e880df109896
push id31944
push userryanvm@gmail.com
push dateThu, 01 Jun 2017 16:44:22 +0000
treeherdermozilla-central@0e9853e31da9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspchang
bugs1368551, 1345577
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1368551 - Don't send transforms to WR in push_stacking_context if they are identity. r=pchang If we set a transform in push_stacking_context, it changes the internal WebRender behaviour to make that stacking context a reference frame, and things inside it are positioned differently. This is true even if the transform is an identity transform. In most cases we are hitting this and sending an identity transform through, when in fact we want to be sending a None value to WebRender so that it doesn't create reference frames. This is a partial fix, a proper fix will be done in bug 1345577 by separating the CSS transform from the other transforms that FrameLayerBuilder invents. MozReview-Commit-ID: ElSs3hFMD2D
gfx/layers/wr/StackingContextHelper.cpp
gfx/layers/wr/WebRenderContainerLayer.cpp
gfx/webrender_bindings/src/bindings.rs
--- a/gfx/layers/wr/StackingContextHelper.cpp
+++ b/gfx/layers/wr/StackingContextHelper.cpp
@@ -22,17 +22,18 @@ StackingContextHelper::StackingContextHe
                                              WebRenderLayer* aLayer,
                                              const Maybe<gfx::Matrix4x4>& aTransform)
   : mBuilder(&aBuilder)
 {
   WrRect scBounds = aParentSC.ToRelativeWrRect(aLayer->BoundsForStackingContext());
   Layer* layer = aLayer->GetLayer();
   mTransform = aTransform.valueOr(layer->GetTransform());
   float opacity = 1.0f;
-  mBuilder->PushStackingContext(scBounds, 0, &opacity, &mTransform,
+  mBuilder->PushStackingContext(scBounds, 0, &opacity,
+                                mTransform.IsIdentity() ? nullptr : &mTransform,
                                 wr::ToWrMixBlendMode(layer->GetMixBlendMode()));
   mOrigin = aLayer->Bounds().TopLeft();
 }
 
 StackingContextHelper::StackingContextHelper(const StackingContextHelper& aParentSC,
                                              wr::DisplayListBuilder& aBuilder,
                                              WebRenderLayer* aLayer,
                                              uint64_t aAnimationsId,
--- a/gfx/layers/wr/WebRenderContainerLayer.cpp
+++ b/gfx/layers/wr/WebRenderContainerLayer.cpp
@@ -102,16 +102,23 @@ WebRenderContainerLayer::RenderLayer(wr:
 
     EnsureAnimationsId();
     animationsId = GetCompositorAnimationsId();
     // We need to set the transform in the stacking context to null for it to
     // pick up and install the animation id.
     transformForSC = nullptr;
   }
 
+  if (transformForSC && transform.IsIdentity()) {
+    // If the transform is an identity transform, strip it out so that WR
+    // doesn't turn this stacking context into a reference frame, as it
+    // affects positioning. Bug 1345577 tracks a better fix.
+    transformForSC = nullptr;
+  }
+
   ScrollingLayersHelper scroller(this, aBuilder, aSc);
   StackingContextHelper sc(aSc, aBuilder, this, animationsId, opacityForSC, transformForSC);
 
   LayerRect rect = Bounds();
   DumpLayerInfo("ContainerLayer", rect);
 
   Maybe<WrImageMask> mask = BuildWrMaskLayer(&sc);
   aBuilder.PushClip(sc.ToRelativeWrRect(rect), mask.ptrOr(nullptr));
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -1256,26 +1256,29 @@ pub extern "C" fn wr_dp_push_stacking_co
         if *opacity < 1.0 {
             filters.push(FilterOp::Opacity(PropertyBinding::Value(*opacity)));
         }
     } else {
         filters.push(FilterOp::Opacity(PropertyBinding::Binding(PropertyBindingKey::new(animation_id))));
     }
 
     let transform = unsafe { transform.as_ref() };
-    let transform_binding = match transform {
-        Some(transform) => PropertyBinding::Value(transform.into()),
-        None => PropertyBinding::Binding(PropertyBindingKey::new(animation_id)),
+    let transform_binding = match animation_id {
+        0 => match transform {
+            Some(transform) => Some(PropertyBinding::Value(transform.into())),
+            None => None,
+        },
+        _ => Some(PropertyBinding::Binding(PropertyBindingKey::new(animation_id))),
     };
 
     state.frame_builder
          .dl_builder
          .push_stacking_context(webrender_traits::ScrollPolicy::Scrollable,
                                 bounds,
-                                Some(transform_binding),
+                                transform_binding,
                                 TransformStyle::Flat,
                                 None,
                                 mix_blend_mode,
                                 filters);
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_pop_stacking_context(state: &mut WrState) {