Bug 663776. Part 3: Refactor layer transform snapping to distinguish translation-snapping from rect-snapping, and don't snap translation+scale transforms when we don't know all four edges of the rect that needs to be snapped. r=mattwoodrow

This separates SnapTransform into SnapTransformTranslation, which just snaps translations
and nothing else, and SnapTransform, which snaps translation+scale of rectangles.

gfx/layers/ImageLayers.cpp

@@ -44,7 +44,7 @@ void ImageLayer::ComputeEffectiveTransforms(const gfx3DMatrix& aTransformToSurfa
   // transform, then we'd snap again when compositing the ThebesLayer).
   mEffectiveTransform =
       SnapTransform(local, sourceRect, nullptr) *
-      SnapTransform(aTransformToSurface, gfxRect(0, 0, 0, 0), nullptr);
+      SnapTransformTranslation(aTransformToSurface, nullptr);
   ComputeEffectiveTransformForMaskLayer(aTransformToSurface);
 }
 

gfx/layers/Layers.cpp

@@ -547,6 +547,37 @@ Layer::GetEffectiveVisibleRegion()
   return GetVisibleRegion();
 }
 
+gfx3DMatrix
+Layer::SnapTransformTranslation(const gfx3DMatrix& aTransform,
+                                gfxMatrix* aResidualTransform)
+{
+  if (aResidualTransform) {
+    *aResidualTransform = gfxMatrix();
+  }
+
+  gfxMatrix matrix2D;
+  gfx3DMatrix result;
+  if (mManager->IsSnappingEffectiveTransforms() &&
+      aTransform.Is2D(&matrix2D) &&
+      !matrix2D.HasNonTranslation() &&
+      matrix2D.HasNonIntegerTranslation()) {
+    gfxPoint snappedTranslation(matrix2D.GetTranslation());
+    snappedTranslation.Round();
+    gfxMatrix snappedMatrix = gfxMatrix().Translate(snappedTranslation);
+    result = gfx3DMatrix::From2D(snappedMatrix);
+    if (aResidualTransform) {
+      // set aResidualTransform so that aResidual * snappedMatrix == matrix2D.
+      // (I.e., appying snappedMatrix after aResidualTransform gives the
+      // ideal transform.)
+      *aResidualTransform =
+        gfxMatrix().Translate(matrix2D.GetTranslation() - snappedTranslation);
+    }
+  } else {
+    result = aTransform;
+  }
+  return result;
+}
+
 gfx3DMatrix
 Layer::SnapTransform(const gfx3DMatrix& aTransform,
                      const gfxRect& aSnapRect,
@@ -560,26 +591,18 @@ Layer::SnapTransform(const gfx3DMatrix& aTransform,
   gfx3DMatrix result;
   if (mManager->IsSnappingEffectiveTransforms() &&
       aTransform.Is2D(&matrix2D) &&
-      matrix2D.HasNonIntegerTranslation() &&
-      !matrix2D.IsSingular() &&
-      !matrix2D.HasNonAxisAlignedTransform()) {
-    gfxMatrix snappedMatrix;
-    gfxPoint topLeft = matrix2D.Transform(aSnapRect.TopLeft());
-    topLeft.Round();
-    // first compute scale factors that scale aSnapRect to the snapped rect
-    if (aSnapRect.IsEmpty()) {
-      snappedMatrix.xx = matrix2D.xx;
-      snappedMatrix.yy = matrix2D.yy;
-    } else {
-      gfxPoint bottomRight = matrix2D.Transform(aSnapRect.BottomRight());
-      bottomRight.Round();
-      snappedMatrix.xx = (bottomRight.x - topLeft.x)/aSnapRect.Width();
-      snappedMatrix.yy = (bottomRight.y - topLeft.y)/aSnapRect.Height();
-    }
-    // compute translation factors that will move aSnapRect to the snapped rect
-    // given those scale factors
-    snappedMatrix.x0 = topLeft.x - aSnapRect.X()*snappedMatrix.xx;
-    snappedMatrix.y0 = topLeft.y - aSnapRect.Y()*snappedMatrix.yy;
+      gfxSize(1.0, 1.0) <= aSnapRect.Size() &&
+      matrix2D.PreservesAxisAlignedRectangles()) {
+    gfxPoint transformedTopLeft = matrix2D.Transform(aSnapRect.TopLeft());
+    transformedTopLeft.Round();
+    gfxPoint transformedTopRight = matrix2D.Transform(aSnapRect.TopRight());
+    transformedTopRight.Round();
+    gfxPoint transformedBottomRight = matrix2D.Transform(aSnapRect.BottomRight());
+    transformedBottomRight.Round();
+
+    gfxMatrix snappedMatrix = gfxUtils::TransformRectToRect(aSnapRect,
+      transformedTopLeft, transformedTopRight, transformedBottomRight);
+
     result = gfx3DMatrix::From2D(snappedMatrix);
     if (aResidualTransform && !snappedMatrix.IsSingular()) {
       // set aResidualTransform so that aResidual * snappedMatrix == matrix2D.
@@ -795,7 +818,7 @@ ContainerLayer::DefaultComputeEffectiveTransforms(const gfx3DMatrix& aTransformT
   gfxMatrix residual;
   gfx3DMatrix idealTransform = GetLocalTransform()*aTransformToSurface;
   idealTransform.ProjectTo2D();
-  mEffectiveTransform = SnapTransform(idealTransform, gfxRect(0, 0, 0, 0), &residual);
+  mEffectiveTransform = SnapTransformTranslation(idealTransform, &residual);
 
   bool useIntermediateSurface;
   if (GetMaskLayer()) {

gfx/layers/Layers.h

@@ -1077,15 +1077,42 @@ protected:
   const float GetLocalOpacity();
 
   /**
-   * Computes a tweaked version of aTransform that snaps a point or a rectangle
-   * to pixel boundaries. Snapping is only performed if this layer's
-   * layer manager has enabled snapping (which is the default).
+   * We can snap layer transforms for two reasons:
+   * 1) To avoid unnecessary resampling when a transform is a translation
+   * by a non-integer number of pixels.
+   * Snapping the translation to an integer number of pixels avoids
+   * blurring the layer and can be faster to composite.
+   * 2) When a layer is used to render a rectangular object, we need to
+   * emulate the rendering of rectangular inactive content and snap the
+   * edges of the rectangle to pixel boundaries. This is both to ensure
+   * layer rendering is consistent with inactive content rendering, and to
+   * avoid seams.
+   * This function implements type 1 snapping. If aTransform is a 2D
+   * translation, and this layer's layer manager has enabled snapping
+   * (which is the default), return aTransform with the translation snapped
+   * to nearest pixels. Otherwise just return aTransform. Call this when the
+   * layer does not correspond to a single rectangular content object.
+   * This function does not try to snap if aTransform has a scale, because in
+   * that case resampling is inevitable and there's no point in trying to
+   * avoid it. In fact snapping can cause problems because pixel edges in the
+   * layer's content can be rendered unpredictably (jiggling) as the scale
+   * interacts with the snapping of the translation, especially with animated
+   * transforms.
+   * @param aResidualTransform a transform to apply before the result transform
+   * in order to get the results to completely match aTransform.
+   */
+  gfx3DMatrix SnapTransformTranslation(const gfx3DMatrix& aTransform,
+                                       gfxMatrix* aResidualTransform);
+  /**
+   * See comment for SnapTransformTranslation.
+   * This function implements type 2 snapping. If aTransform is a translation
+   * and/or scale, transform aSnapRect by aTransform, snap to pixel boundaries,
+   * and return the transform that maps aSnapRect to that rect. Otherwise
+   * just return aTransform.
    * @param aSnapRect a rectangle whose edges should be snapped to pixel
-   * boundaries in the destination surface. If the rectangle is empty,
-   * then the snapping process should preserve the scale factors of the
-   * transform matrix
-   * @param aResidualTransform a transform to apply before mEffectiveTransform
-   * in order to get the results to completely match aTransform
+   * boundaries in the destination surface.
+   * @param aResidualTransform a transform to apply before the result transform
+   * in order to get the results to completely match aTransform.
    */
   gfx3DMatrix SnapTransform(const gfx3DMatrix& aTransform,
                             const gfxRect& aSnapRect,
@@ -1177,14 +1204,12 @@ public:
 
   virtual void ComputeEffectiveTransforms(const gfx3DMatrix& aTransformToSurface)
   {
-    // The default implementation just snaps 0,0 to pixels.
     gfx3DMatrix idealTransform = GetLocalTransform()*aTransformToSurface;
     gfxMatrix residual;
-    mEffectiveTransform = SnapTransform(idealTransform, gfxRect(0, 0, 0, 0),
+    mEffectiveTransform = SnapTransformTranslation(idealTransform,
         mAllowResidualTranslation ? &residual : nullptr);
-    // The residual can only be a translation because ThebesLayer snapping
-    // only aligns a single point with the pixel grid; scale factors are always
-    // preserved exactly
+    // The residual can only be a translation because SnapTransformTranslation
+    // only changes the transform if it's a translation
     NS_ASSERTION(!residual.HasNonTranslation(),
                  "Residual transform can only be a translation");
     if (!residual.GetTranslation().WithinEpsilonOf(mResidualTranslation, 1e-3f)) {
@@ -1411,9 +1436,8 @@ public:
 
   virtual void ComputeEffectiveTransforms(const gfx3DMatrix& aTransformToSurface)
   {
-    // Snap 0,0 to pixel boundaries, no extra internal transform.
     gfx3DMatrix idealTransform = GetLocalTransform()*aTransformToSurface;
-    mEffectiveTransform = SnapTransform(idealTransform, gfxRect(0, 0, 0, 0), nullptr);
+    mEffectiveTransform = SnapTransformTranslation(idealTransform, nullptr);
     ComputeEffectiveTransformForMaskLayer(aTransformToSurface);
   }
 
@@ -1524,7 +1548,7 @@ public:
     mEffectiveTransform =
         SnapTransform(GetLocalTransform(), gfxRect(0, 0, mBounds.width, mBounds.height),
                       nullptr)*
-        SnapTransform(aTransformToSurface, gfxRect(0, 0, 0, 0), nullptr);
+        SnapTransformTranslation(aTransformToSurface, nullptr);
     ComputeEffectiveTransformForMaskLayer(aTransformToSurface);
   }
 

gfx/layers/ReadbackLayer.h

@@ -79,7 +79,7 @@ public:
     mEffectiveTransform =
         SnapTransform(GetLocalTransform(), gfxRect(0, 0, mSize.width, mSize.height),
                       nullptr)*
-        SnapTransform(aTransformToSurface, gfxRect(0, 0, 0, 0), nullptr);
+        SnapTransformTranslation(aTransformToSurface, nullptr);
   }
 
   /**

gfx/layers/basic/BasicContainerLayer.h

@@ -147,9 +147,9 @@ ContainerComputeEffectiveTransforms(const gfx3DMatrix& aTransformToSurface,
   }
 
   aContainer->mEffectiveTransform =
-    aContainer->SnapTransform(idealTransform, gfxRect(0, 0, 0, 0), &residual);
+    aContainer->SnapTransformTranslation(idealTransform, &residual);
   // We always pass the ideal matrix down to our children, so there is no
-  // need to apply any compensation using the residual from SnapTransform.
+  // need to apply any compensation using the residual from SnapTransformTranslation.
   aContainer->ComputeEffectiveTransformsForChildren(idealTransform);
 
   aContainer->ComputeEffectiveTransformForMaskLayer(aTransformToSurface);