Bug 1453747 - Use rounded dest rect in simplified image decode size calculations for WebRender. r=mstange
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 04 Jan 2019 09:28:49 -0500
changeset 511131 70f87448b9e46e5d4ca7c2965d210ec2f1d5acd3
parent 511130 d6d74968b2741243af6942cc09b66dfea5456865
child 511132 62ce57cf81571a894c42c91199aa1379d4a15e5b
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1453747
milestone66.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 1453747 - Use rounded dest rect in simplified image decode size calculations for WebRender. r=mstange We are using the unrounded dest rect to calculate the image decode size in ComputeImageContainerDrawingParameters, while passing the rounded dest rect to WebRender. This mismatch causes images to be decoded to one size and display at another, cause some visual distortions. Using the correct rect seems to allow us to remove the extra snapping logic added to work around this. At this time, how we snap is different between WebRender and non-WebRender in general. This patch will likely morph again once we bring the two models closer together. Differential Revision: https://phabricator.services.mozilla.com/D15739
gfx/layers/wr/StackingContextHelper.h
layout/base/nsLayoutUtils.cpp
layout/generic/nsBulletFrame.cpp
layout/generic/nsImageFrame.cpp
layout/painting/nsCSSRenderingBorders.cpp
layout/painting/nsImageRenderer.cpp
layout/xul/nsImageBoxFrame.cpp
layout/xul/reftest/reftest.list
--- a/gfx/layers/wr/StackingContextHelper.h
+++ b/gfx/layers/wr/StackingContextHelper.h
@@ -50,20 +50,16 @@ class MOZ_RAII StackingContextHelper {
   StackingContextHelper();
 
   // Pops the stacking context, if one was pushed during the constructor.
   ~StackingContextHelper();
 
   // Export the inherited scale
   gfx::Size GetInheritedScale() const { return mScale; }
 
-  const gfx::Matrix& GetInheritedTransform() const {
-    return mInheritedTransform;
-  }
-
   const gfx::Matrix& GetSnappingSurfaceTransform() const {
     return mSnappingSurfaceTransform;
   }
 
   const Maybe<nsDisplayTransform*>& GetDeferredTransformItem() const;
   Maybe<gfx::Matrix4x4> GetDeferredTransformMatrix() const;
 
   bool AffectsClipPositioning() const { return mAffectsClipPositioning; }
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -6356,18 +6356,16 @@ static SnappedImageDrawingParameters Com
 
   gfxMatrix currentMatrix = aCtx->CurrentMatrixDouble();
   gfxRect fill = devPixelFill;
   gfxRect dest = devPixelDest;
   bool didSnap;
   // Snap even if we have a scale in the context. But don't snap if
   // we have something that's not translation+scale, or if the scale flips in
   // the X or Y direction, because snapped image drawing can't handle that yet.
-  // Any changes to this algorithm will need to be reflected in
-  // ComputeImageContainerDrawingParameters.
   if (!currentMatrix.HasNonAxisAlignedTransform() && currentMatrix._11 > 0.0 &&
       currentMatrix._22 > 0.0 && aCtx->UserToDevicePixelSnapped(fill, true) &&
       aCtx->UserToDevicePixelSnapped(dest, true)) {
     // We snapped. On this code path, |fill| and |dest| take into account
     // currentMatrix's transform.
     didSnap = true;
   } else {
     // We didn't snap. On this code path, |fill| and |dest| do not take into
@@ -6747,56 +6745,27 @@ static ImgDrawResult DrawImageInternal(
     CSSIntSize cssViewportSize(viewportSize.width, viewportSize.height);
     if (!aSVGContext) {
       aSVGContext.emplace(Some(cssViewportSize));
     } else {
       aSVGContext->SetViewportSize(Some(cssViewportSize));
     }
   }
 
-  // Attempt to snap pixels, the same as ComputeSnappedImageDrawingParameters.
-  // Any changes to the algorithm here will need to be reflected there.
-  bool snapped = false;
-  gfxSize gfxLayerSize;
-  const gfx::Matrix& itm = aSc.GetInheritedTransform();
-  if (!itm.HasNonAxisAlignedTransform() && itm._11 > 0.0 && itm._22 > 0.0) {
-    gfxRect rect(gfxPoint(aDestRect.X(), aDestRect.Y()),
-                 gfxSize(aDestRect.Width(), aDestRect.Height()));
-
-    gfxPoint p1 = ThebesPoint(itm.TransformPoint(ToPoint(rect.TopLeft())));
-    gfxPoint p2 = ThebesPoint(itm.TransformPoint(ToPoint(rect.TopRight())));
-    gfxPoint p3 = ThebesPoint(itm.TransformPoint(ToPoint(rect.BottomRight())));
-
-    if (p2 == gfxPoint(p1.x, p3.y) || p2 == gfxPoint(p3.x, p1.y)) {
-      p1.Round();
-      p3.Round();
-
-      rect.MoveTo(gfxPoint(std::min(p1.x, p3.x), std::min(p1.y, p3.y)));
-      rect.SizeTo(gfxSize(std::max(p1.x, p3.x) - rect.X(),
-                          std::max(p1.y, p3.y) - rect.Y()));
-
-      // An empty size is unacceptable so we ensure our suggested size is at
-      // least 1 pixel wide/tall.
-      gfxLayerSize =
-          gfxSize(std::max(rect.Width(), 1.0), std::max(rect.Height(), 1.0));
-      snapped = true;
-    }
-  }
-
-  if (!snapped) {
-    // Compute our size in layer pixels.
-    const LayerIntSize layerSize =
-        RoundedToInt(LayerSize(aDestRect.Width() * scaleFactors.width,
-                               aDestRect.Height() * scaleFactors.height));
-
-    // An empty size is unacceptable so we ensure our suggested size is at least
-    // 1 pixel wide/tall.
-    gfxLayerSize =
-        gfxSize(std::max(layerSize.width, 1), std::max(layerSize.height, 1));
-  }
+  // Compute our size in layer pixels. We may need to revisit this for Android
+  // because mobile websites are rarely displayed at a 1:1
+  // LayoutPixel:ScreenPixel ratio and the snapping here may be insufficient.
+  const LayerIntSize layerSize =
+      RoundedToInt(LayerSize(aDestRect.Width() * scaleFactors.width,
+                             aDestRect.Height() * scaleFactors.height));
+
+  // An empty size is unacceptable so we ensure our suggested size is at least
+  // 1 pixel wide/tall.
+  gfxSize gfxLayerSize =
+      gfxSize(std::max(layerSize.width, 1), std::max(layerSize.height, 1));
 
   return aImage->OptimalImageSizeForDest(
       gfxLayerSize, imgIContainer::FRAME_CURRENT, samplingFilter, aFlags);
 }
 
 /* static */ nsPoint nsLayoutUtils::GetBackgroundFirstTilePos(
     const nsPoint& aDest, const nsPoint& aFill, const nsSize& aRepeatSize) {
   return nsPoint(NSToIntFloor(float(aFill.x - aDest.x) / aRepeatSize.width) *
--- a/layout/generic/nsBulletFrame.cpp
+++ b/layout/generic/nsBulletFrame.cpp
@@ -427,16 +427,18 @@ ImgDrawResult BulletRenderer::CreateWebR
   if (aDisplayListBuilder->ShouldSyncDecodeImages()) {
     flags |= imgIContainer::FLAG_SYNC_DECODE;
   }
 
   const int32_t appUnitsPerDevPixel =
       aItem->Frame()->PresContext()->AppUnitsPerDevPixel();
   LayoutDeviceRect destRect =
       LayoutDeviceRect::FromAppUnits(mDest, appUnitsPerDevPixel);
+  destRect.Round();
+
   Maybe<SVGImageContext> svgContext;
   gfx::IntSize decodeSize =
       nsLayoutUtils::ComputeImageContainerDrawingParameters(
           mImage, aItem->Frame(), destRect, aSc, flags, svgContext);
 
   RefPtr<layers::ImageContainer> container;
   ImgDrawResult drawResult = mImage->GetImageContainerAtSize(
       aManager->LayerManager(), decodeSize, svgContext, flags,
@@ -449,17 +451,17 @@ ImgDrawResult BulletRenderer::CreateWebR
       nsLayoutUtils::GetSamplingFilterForFrame(aItem->Frame()));
   gfx::IntSize size;
   Maybe<wr::ImageKey> key = aManager->CommandBuilder().CreateImageKey(
       aItem, container, aBuilder, aResources, rendering, aSc, size, Nothing());
   if (key.isNothing()) {
     return drawResult;
   }
 
-  wr::LayoutRect dest = wr::ToRoundedLayoutRect(destRect);
+  wr::LayoutRect dest = wr::ToLayoutRect(destRect);
 
   aBuilder.PushImage(dest, dest, !aItem->BackfaceIsHidden(), rendering,
                      key.value());
 
   return drawResult;
 }
 
 bool BulletRenderer::CreateWebRenderCommandsForPath(
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -1613,18 +1613,20 @@ ImgDrawResult nsImageFrame::DisplayAltFe
       nsCOMPtr<imgIContainer> imgCon;
       request->GetImage(getter_AddRefs(imgCon));
       MOZ_ASSERT(imgCon, "Load complete, but no image container?");
 
       nsRect dest(flushRight ? inner.XMost() - size : inner.x, inner.y, size,
                   size);
 
       const int32_t factor = PresContext()->AppUnitsPerDevPixel();
-      const LayoutDeviceRect destRect(
+      LayoutDeviceRect destRect(
           LayoutDeviceRect::FromAppUnits(dest, factor));
+      destRect.Round();
+
       Maybe<SVGImageContext> svgContext;
       IntSize decodeSize =
           nsLayoutUtils::ComputeImageContainerDrawingParameters(
               imgCon, this, destRect, aSc, aFlags, svgContext);
       RefPtr<ImageContainer> container;
       result = imgCon->GetImageContainerAtSize(
           aManager->LayerManager(), decodeSize, svgContext, aFlags, getter_AddRefs(container));
       if (container) {
@@ -1891,18 +1893,20 @@ bool nsDisplayImage::CreateWebRenderComm
   if (aDisplayListBuilder->IsPaintingToWindow()) {
     flags |= imgIContainer::FLAG_HIGH_QUALITY_SCALING;
   }
   if (aDisplayListBuilder->ShouldSyncDecodeImages()) {
     flags |= imgIContainer::FLAG_SYNC_DECODE;
   }
 
   const int32_t factor = mFrame->PresContext()->AppUnitsPerDevPixel();
-  const LayoutDeviceRect destRect(
+  LayoutDeviceRect destRect(
       LayoutDeviceRect::FromAppUnits(GetDestRect(), factor));
+  destRect.Round();
+
   Maybe<SVGImageContext> svgContext;
   IntSize decodeSize = nsLayoutUtils::ComputeImageContainerDrawingParameters(
       mImage, mFrame, destRect, aSc, flags, svgContext);
 
   RefPtr<layers::ImageContainer> container;
   ImgDrawResult drawResult = mImage->GetImageContainerAtSize(
       aManager->LayerManager(), decodeSize, svgContext, flags,
       getter_AddRefs(container));
--- a/layout/painting/nsCSSRenderingBorders.cpp
+++ b/layout/painting/nsCSSRenderingBorders.cpp
@@ -3548,17 +3548,18 @@ ImgDrawResult nsCSSBorderImageRenderer::
   NS_FOR_CSS_SIDES(i) {
     slice[i] = (float)(mSlice.Side(i)) / appUnitsPerDevPixel;
     widths[i] = (float)(mWidths.Side(i)) / appUnitsPerDevPixel;
     outset[i] = (float)(mImageOutset.Side(i)) / appUnitsPerDevPixel;
   }
 
   LayoutDeviceRect destRect =
       LayoutDeviceRect::FromAppUnits(mArea, appUnitsPerDevPixel);
-  wr::LayoutRect dest = wr::ToRoundedLayoutRect(destRect);
+  destRect.Round();
+  wr::LayoutRect dest = wr::ToLayoutRect(destRect);
 
   wr::LayoutRect clip = dest;
   if (!mClip.IsEmpty()) {
     LayoutDeviceRect clipRect =
         LayoutDeviceRect::FromAppUnits(mClip, appUnitsPerDevPixel);
     clip = wr::ToRoundedLayoutRect(clipRect);
   }
 
--- a/layout/painting/nsImageRenderer.cpp
+++ b/layout/painting/nsImageRenderer.cpp
@@ -562,16 +562,19 @@ ImgDrawResult nsImageRenderer::BuildWebR
       CSSIntSize imageSize(nsPresContext::AppUnitsToIntCSSPixels(mSize.width),
                            nsPresContext::AppUnitsToIntCSSPixels(mSize.height));
       Maybe<SVGImageContext> svgContext(Some(SVGImageContext(Some(imageSize))));
 
       const int32_t appUnitsPerDevPixel =
           mForFrame->PresContext()->AppUnitsPerDevPixel();
       LayoutDeviceRect destRect =
           LayoutDeviceRect::FromAppUnits(aDest, appUnitsPerDevPixel);
+      auto stretchSize = wr::ToLayoutSize(destRect.Size());
+      destRect.Round();
+
       gfx::IntSize decodeSize =
           nsLayoutUtils::ComputeImageContainerDrawingParameters(
               mImageContainer, mForFrame, destRect, aSc, containerFlags,
               svgContext);
 
       RefPtr<layers::ImageContainer> container;
       drawResult = mImageContainer->GetImageContainerAtSize(
           aManager->LayerManager(), decodeSize, svgContext, containerFlags,
@@ -595,18 +598,17 @@ ImgDrawResult nsImageRenderer::BuildWebR
       nsPoint firstTilePos = nsLayoutUtils::GetBackgroundFirstTilePos(
           aDest.TopLeft(), aFill.TopLeft(), aRepeatSize);
       LayoutDeviceRect fillRect = LayoutDeviceRect::FromAppUnits(
           nsRect(firstTilePos.x, firstTilePos.y, aFill.XMost() - firstTilePos.x,
                  aFill.YMost() - firstTilePos.y),
           appUnitsPerDevPixel);
       wr::LayoutRect fill = wr::ToRoundedLayoutRect(fillRect);
 
-      wr::LayoutRect roundedDest = wr::ToRoundedLayoutRect(destRect);
-      auto stretchSize = wr::ToLayoutSize(destRect.Size());
+      wr::LayoutRect roundedDest = wr::ToLayoutRect(destRect);
 
       // WebRender special cases situations where stretchSize == fillSize to
       // infer that it shouldn't use repeat sampling. This makes sure
       // we hit those special cases when not repeating.
 
       switch (mExtendMode) {
         case ExtendMode::CLAMP:
           fill = roundedDest;
--- a/layout/xul/nsImageBoxFrame.cpp
+++ b/layout/xul/nsImageBoxFrame.cpp
@@ -393,16 +393,18 @@ ImgDrawResult nsImageBoxFrame::CreateWeb
   }
   if (aFlags & nsImageRenderer::FLAG_SYNC_DECODE_IMAGES) {
     containerFlags |= imgIContainer::FLAG_SYNC_DECODE;
   }
 
   const int32_t appUnitsPerDevPixel = PresContext()->AppUnitsPerDevPixel();
   LayoutDeviceRect fillRect =
       LayoutDeviceRect::FromAppUnits(dest, appUnitsPerDevPixel);
+  fillRect.Round();
+
   Maybe<SVGImageContext> svgContext;
   gfx::IntSize decodeSize =
       nsLayoutUtils::ComputeImageContainerDrawingParameters(
           imgCon, aItem->Frame(), fillRect, aSc, containerFlags, svgContext);
 
   RefPtr<layers::ImageContainer> container;
   result = imgCon->GetImageContainerAtSize(aManager->LayerManager(), decodeSize,
                                            svgContext, containerFlags,
@@ -415,17 +417,17 @@ ImgDrawResult nsImageBoxFrame::CreateWeb
   mozilla::wr::ImageRendering rendering = wr::ToImageRendering(
       nsLayoutUtils::GetSamplingFilterForFrame(aItem->Frame()));
   gfx::IntSize size;
   Maybe<wr::ImageKey> key = aManager->CommandBuilder().CreateImageKey(
       aItem, container, aBuilder, aResources, rendering, aSc, size, Nothing());
   if (key.isNothing()) {
     return result;
   }
-  wr::LayoutRect fill = wr::ToRoundedLayoutRect(fillRect);
+  wr::LayoutRect fill = wr::ToLayoutRect(fillRect);
 
   LayoutDeviceSize gapSize(0, 0);
   aBuilder.PushImage(fill, fill, !BackfaceIsHidden(),
                      wr::ToLayoutSize(fillRect.Size()),
                      wr::ToLayoutSize(gapSize), rendering, key.value());
 
   return result;
 }
--- a/layout/xul/reftest/reftest.list
+++ b/layout/xul/reftest/reftest.list
@@ -1,9 +1,9 @@
 fails-if(Android) == textbox-multiline-noresize.xul textbox-multiline-ref.xul # reference is blank on Android (due to no native theme support?)
 != textbox-multiline-resize.xul textbox-multiline-ref.xul
 == popup-explicit-size.xul popup-explicit-size-ref.xul
-random-if(Android) fuzzy-if(webrender,128-128,168-168) == image-size.xul image-size-ref.xul
+random-if(Android) == image-size.xul image-size-ref.xul
 == image-scaling-min-height-1.xul image-scaling-min-height-1-ref.xul
 == textbox-text-transform.xul textbox-text-transform-ref.xul
 
 == checkbox-dynamic-change.xul checkbox-dynamic-change-ref.xul
 == radio-dynamic-change.xul radio-dynamic-change-ref.xul