Bug 1717556 - Give gfxContext::UserToDevicePixelSnapped an option to prioritize the rect dimensions over snapping each individual edge, and use this for GTK widget painting. r=karlt

Differential Revision: https://phabricator.services.mozilla.com/D119885

gfx/thebes/gfxContext.cpp

@@ -247,7 +247,7 @@ void gfxContext::Rectangle(const gfxRect& rect, bool snapToPixels) {
 
   if (snapToPixels) {
     gfxRect newRect(rect);
-    if (UserToDevicePixelSnapped(newRect, true)) {
+    if (UserToDevicePixelSnapped(newRect, SnapOption::IgnoreScale)) {
       gfxMatrix mat = ThebesMatrix(mTransform);
       if (mat.Invert()) {
         // We need the user space rect.
@@ -277,7 +277,7 @@ void gfxContext::SnappedClip(const gfxRect& rect) {
   Rect rec = ToRect(rect);
 
   gfxRect newRect(rect);
-  if (UserToDevicePixelSnapped(newRect, true)) {
+  if (UserToDevicePixelSnapped(newRect, SnapOption::IgnoreScale)) {
     gfxMatrix mat = ThebesMatrix(mTransform);
     if (mat.Invert()) {
       // We need the user space rect.
@@ -347,8 +347,10 @@ gfxRect gfxContext::UserToDevice(const gfxRect& rect) const {
 }
 
 bool gfxContext::UserToDevicePixelSnapped(gfxRect& rect,
-                                          bool ignoreScale) const {
-  if (mDT->GetUserData(&sDisablePixelSnapping)) return false;
+                                          SnapOptions aOptions) const {
+  if (mDT->GetUserData(&sDisablePixelSnapping)) {
+    return false;
+  }
 
   // if we're not at 1.0 scale, don't snap, unless we're
   // ignoring the scale.  If we're not -just- a scale,
@@ -356,9 +358,11 @@ bool gfxContext::UserToDevicePixelSnapped(gfxRect& rect,
   const gfxFloat epsilon = 0.0000001;
 #define WITHIN_E(a, b) (fabs((a) - (b)) < epsilon)
   Matrix mat = mTransform;
-  if (!ignoreScale && (!WITHIN_E(mat._11, 1.0) || !WITHIN_E(mat._22, 1.0) ||
-                       !WITHIN_E(mat._12, 0.0) || !WITHIN_E(mat._21, 0.0)))
+  if (!aOptions.contains(SnapOption::IgnoreScale) &&
+      (!WITHIN_E(mat._11, 1.0) || !WITHIN_E(mat._22, 1.0) ||
+       !WITHIN_E(mat._12, 0.0) || !WITHIN_E(mat._21, 0.0))) {
     return false;
+  }
 #undef WITHIN_E
 
   gfxPoint p1 = UserToDevice(rect.TopLeft());
@@ -371,22 +375,41 @@ bool gfxContext::UserToDevicePixelSnapped(gfxRect& rect,
   // define an axis-aligned rectangle whose other corners are p2 and p4.
   // We actually only need to check one of p2 and p4, since an affine
   // transform maps parallelograms to parallelograms.
-  if (p2 == gfxPoint(p1.x, p3.y) || p2 == gfxPoint(p3.x, p1.y)) {
+  if (!(p2 == gfxPoint(p1.x, p3.y) || p2 == gfxPoint(p3.x, p1.y))) {
+    return false;
+  }
+
+  if (aOptions.contains(SnapOption::PrioritizeSize)) {
+    // Snap the dimensions of the rect, to minimize distortion; only after that
+    // will we snap its position. In particular, this guarantees that a square
+    // remains square after snapping, which may not be the case if each edge is
+    // independently snapped to device pixels.
+
+    // Use the same rounding approach as gfx::BasePoint::Round.
+    rect.SizeTo(std::floor(rect.width + 0.5), std::floor(rect.height + 0.5));
+
+    // Find the top-left corner based on the original center and the snapped
+    // size, then snap this new corner to the grid.
+    gfxPoint center = (p1 + p3) / 2;
+    gfxPoint topLeft = center - gfxPoint(rect.width / 2.0, rect.height / 2.0);
+    topLeft.Round();
+    rect.MoveTo(topLeft);
+  } else {
     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()));
-    return true;
   }
 
-  return false;
+  return true;
 }
 
 bool gfxContext::UserToDevicePixelSnapped(gfxPoint& pt,
                                           bool ignoreScale) const {
-  if (mDT->GetUserData(&sDisablePixelSnapping)) return false;
+  if (mDT->GetUserData(&sDisablePixelSnapping)) {
+    return false;
+  }
 
   // if we're not at 1.0 scale, don't snap, unless we're
   // ignoring the scale.  If we're not -just- a scale,
@@ -395,8 +418,9 @@ bool gfxContext::UserToDevicePixelSnapped(gfxPoint& pt,
 #define WITHIN_E(a, b) (fabs((a) - (b)) < epsilon)
   Matrix mat = mTransform;
   if (!ignoreScale && (!WITHIN_E(mat._11, 1.0) || !WITHIN_E(mat._22, 1.0) ||
-                       !WITHIN_E(mat._12, 0.0) || !WITHIN_E(mat._21, 0.0)))
+                       !WITHIN_E(mat._12, 0.0) || !WITHIN_E(mat._21, 0.0))) {
     return false;
+  }
 #undef WITHIN_E
 
   pt = UserToDevice(pt);

gfx/thebes/gfxContext.h

@@ -15,6 +15,7 @@
 #include "gfxPattern.h"
 #include "nsTArray.h"
 
+#include "mozilla/EnumSet.h"
 #include "mozilla/gfx/2D.h"
 
 typedef struct _cairo cairo_t;
@@ -227,11 +228,21 @@ class gfxContext final {
    * fails, the method will return false, and the rect will not be
    * changed.
    *
-   * If ignoreScale is true, then snapping will take place even if
-   * the CTM has a scale applied.  Snapping never takes place if
-   * there is a rotation in the CTM.
+   * aOptions parameter:
+   *   If IgnoreScale is set, then snapping will take place even if the CTM
+   *   has a scale applied. Snapping never takes place if there is a rotation
+   *   in the CTM.
+   *
+   *   If PrioritizeSize is set, the rect's dimensions will first be snapped
+   *   and then its position aligned to device pixels, rather than snapping
+   *   the position of each edge independently.
    */
-  bool UserToDevicePixelSnapped(gfxRect& rect, bool ignoreScale = false) const;
+  enum class SnapOption : uint8_t {
+    IgnoreScale = 1,
+    PrioritizeSize = 2,
+  };
+  using SnapOptions = mozilla::EnumSet<SnapOption>;
+  bool UserToDevicePixelSnapped(gfxRect& rect, SnapOptions aOptions = {}) const;
 
   /**
    * Takes the given point and tries to align it to device pixels.  If

layout/base/nsLayoutUtils.cpp

@@ -5551,7 +5551,10 @@ gfxFloat nsLayoutUtils::GetSnappedBaselineY(nsIFrame* aFrame,
   gfxFloat appUnitsPerDevUnit = aFrame->PresContext()->AppUnitsPerDevPixel();
   gfxFloat baseline = gfxFloat(aY) + aAscent;
   gfxRect putativeRect(0, baseline / appUnitsPerDevUnit, 1, 1);
-  if (!aContext->UserToDevicePixelSnapped(putativeRect, true)) return baseline;
+  if (!aContext->UserToDevicePixelSnapped(
+          putativeRect, gfxContext::SnapOption::IgnoreScale)) {
+    return baseline;
+  }
   return aContext->DeviceToUser(putativeRect.TopLeft()).y * appUnitsPerDevUnit;
 }
 
@@ -5561,7 +5564,8 @@ gfxFloat nsLayoutUtils::GetSnappedBaselineX(nsIFrame* aFrame,
   gfxFloat appUnitsPerDevUnit = aFrame->PresContext()->AppUnitsPerDevPixel();
   gfxFloat baseline = gfxFloat(aX) + aAscent;
   gfxRect putativeRect(baseline / appUnitsPerDevUnit, 0, 1, 1);
-  if (!aContext->UserToDevicePixelSnapped(putativeRect, true)) {
+  if (!aContext->UserToDevicePixelSnapped(
+          putativeRect, gfxContext::SnapOption::IgnoreScale)) {
     return baseline;
   }
   return aContext->DeviceToUser(putativeRect.TopLeft()).x * appUnitsPerDevUnit;
@@ -6201,8 +6205,11 @@ static SnappedImageDrawingParameters ComputeSnappedImageDrawingParameters(
   // 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.
   if (!currentMatrix.HasNonAxisAlignedTransform() && currentMatrix._11 > 0.0 &&
-      currentMatrix._22 > 0.0 && aCtx->UserToDevicePixelSnapped(fill, true) &&
-      aCtx->UserToDevicePixelSnapped(dest, true)) {
+      currentMatrix._22 > 0.0 &&
+      aCtx->UserToDevicePixelSnapped(fill,
+                                     gfxContext::SnapOption::IgnoreScale) &&
+      aCtx->UserToDevicePixelSnapped(dest,
+                                     gfxContext::SnapOption::IgnoreScale)) {
     // We snapped. On this code path, |fill| and |dest| take into account
     // currentMatrix's transform.
     didSnap = true;

widget/gtk/nsNativeThemeGTK.cpp

@@ -1091,7 +1091,10 @@ nsNativeThemeGTK::DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
   // to provide crisper and faster drawing.
   // Don't snap if it's a non-unit scale factor. We're going to have to take
   // slow paths then in any case.
-  bool snapped = ctx->UserToDevicePixelSnapped(rect);
+  // We prioritize the size when snapping in order to avoid distorting widgets
+  // that should be square, which can occur if edges are snapped independently.
+  bool snapped = ctx->UserToDevicePixelSnapped(
+      rect, gfxContext::SnapOption::PrioritizeSize);
   if (snapped) {
     // Leave rect in device coords but make dirtyRect consistent.
     dirtyRect = ctx->UserToDevice(dirtyRect);