bug 814434 - use global display pixels for window positioning/sizing for consistency across mixed-resolution screens. r=smichaud

2012-12-06 17:31:34 +00:00 · 2012-12-06 17:31:34 +00:00 · 5f9d375273
--- a/view/src/nsView.cpp
+++ b/view/src/nsView.cpp
@ -272,17 +272,30 @@ void nsView::DoResetWidgetBounds(bool aMoveOnly,
  bool changedSize = curBounds.Size() != newBounds.Size();

  // Child views are never attached to top level widgets, this is safe.
+
+  // Coordinates are converted to display pixels for window Move/Resize APIs,
+  // because of the potential for device-pixel coordinate spaces for mixed
+  // hidpi/lodpi screens to overlap each other and result in bad placement
+  // (bug 814434).
+  nsRefPtr<nsDeviceContext> dx;
+  mViewManager->GetDeviceContext(*getter_AddRefs(dx));
+  double invScale = dx->UnscaledAppUnitsPerDevPixel() / 60.0;
+
  if (changedPos) {
    if (changedSize && !aMoveOnly) {
-      mWindow->ResizeClient(newBounds.x, newBounds.y,
-                            newBounds.width, newBounds.height,
+      mWindow->ResizeClient(NSToIntRound(newBounds.x * invScale),
+                            NSToIntRound(newBounds.y * invScale),
+                            NSToIntRound(newBounds.width * invScale),
+                            NSToIntRound(newBounds.height * invScale),
                            aInvalidateChangedSize);
    } else {
-      mWindow->MoveClient(newBounds.x, newBounds.y);
+      mWindow->MoveClient(NSToIntRound(newBounds.x * invScale),
+                          NSToIntRound(newBounds.y * invScale));
    }
  } else {
    if (changedSize && !aMoveOnly) {
-      mWindow->ResizeClient(newBounds.width, newBounds.height,
+      mWindow->ResizeClient(NSToIntRound(newBounds.width * invScale),
+                            NSToIntRound(newBounds.height * invScale),
                            aInvalidateChangedSize);
    } // else do nothing!
  }
--- a/widget/cocoa/nsCocoaWindow.mm
+++ b/widget/cocoa/nsCocoaWindow.mm
@ -1124,19 +1124,25 @@ void nsCocoaWindow::SetSizeConstraints(const SizeConstraints& aConstraints)
  NS_OBJC_END_TRY_ABORT_BLOCK;
 }

+// Coordinates are global display pixels
 NS_IMETHODIMP nsCocoaWindow::Move(int32_t aX, int32_t aY)
 {
-  if (!mWindow || (mBounds.x == aX && mBounds.y == aY))
+  if (!mWindow) {
    return NS_OK;
+  }

  // The point we have is in Gecko coordinates (origin top-left). Convert
  // it to Cocoa ones (origin bottom-left).
-  CGFloat scaleFactor = BackingScaleFactor();
  NSPoint coord = {
-    nsCocoaUtils::DevPixelsToCocoaPoints(aX, scaleFactor),
-    nsCocoaUtils::FlippedScreenY(nsCocoaUtils::DevPixelsToCocoaPoints(aY, scaleFactor))
+    static_cast<float>(aX),
+    static_cast<float>(nsCocoaUtils::FlippedScreenY(aY))
  };
-  [mWindow setFrameTopLeftPoint:coord];
+
+  NSRect frame = [mWindow frame];
+  if (frame.origin.x != coord.x ||
+      frame.origin.y + frame.size.height != coord.y) {
+    [mWindow setFrameTopLeftPoint:coord];
+  }

  return NS_OK;
 }
@ -1301,38 +1307,45 @@ NS_METHOD nsCocoaWindow::MakeFullScreen(bool aFullScreen)
  NS_OBJC_END_TRY_ABORT_BLOCK_NSRESULT;
 }

+// Coordinates are global display pixels
 nsresult nsCocoaWindow::DoResize(int32_t aX, int32_t aY,
                                 int32_t aWidth, int32_t aHeight,
-                                 bool aRepaint, bool aConstrainToCurrentScreen)
+                                 bool aRepaint,
+                                 bool aConstrainToCurrentScreen)
 {
  NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;

+  if (!mWindow) {
+    return NS_OK;
+  }
+
+  // ConstrainSize operates in device pixels, so we need to convert using
+  // the backing scale factor here
+  CGFloat scale = BackingScaleFactor();
+  aWidth *= scale;
+  aHeight *= scale;
  ConstrainSize(&aWidth, &aHeight);
+  aWidth = NSToIntRound(aWidth / scale);
+  aHeight = NSToIntRound(aHeight / scale);

  nsIntRect newBounds(aX, aY, aWidth, aHeight);

-  // convert requested size into Cocoa points
-  CGFloat scaleFactor = BackingScaleFactor();
-  NSRect cocoaBounds = nsCocoaUtils::DevPixelsToCocoaPoints(newBounds, scaleFactor);
+  // constrain to the screen that contains the largest area of the new rect
+  FitRectToVisibleAreaForScreen(newBounds, aConstrainToCurrentScreen ?
+                                           [mWindow screen] : nullptr);

-  // constrain to the visible area of the window's current screen if requested,
-  // or to the screen that contains the largest area of the new rect
-  nsCocoaUtils::NSRectToGeckoRect(cocoaBounds, newBounds);
-  FitRectToVisibleAreaForScreen(newBounds,
-                                aConstrainToCurrentScreen ?
-                                    [mWindow screen] : nullptr);
+  // convert requested bounds into Cocoa coordinate system
+  NSRect newFrame = nsCocoaUtils::GeckoRectToCocoaRect(newBounds);

-  // then convert back to device pixels
-  nsCocoaUtils::GeckoRectToNSRect(newBounds, cocoaBounds);
-  newBounds = nsCocoaUtils::CocoaPointsToDevPixels(cocoaBounds, scaleFactor);
+  NSRect frame = [mWindow frame];
+  BOOL isMoving = newFrame.origin.x != frame.origin.x ||
+                  newFrame.origin.y != frame.origin.y;
+  BOOL isResizing = newFrame.size.width != frame.size.width ||
+                    newFrame.size.height != frame.size.height;

-  BOOL isMoving = (mBounds.x != newBounds.x || mBounds.y != newBounds.y);
-  BOOL isResizing = (mBounds.width != newBounds.width || mBounds.height != newBounds.height);
-
-  if (!mWindow || (!isMoving && !isResizing))
+  if (!isMoving && !isResizing) {
    return NS_OK;
-
-  NSRect newFrame = nsCocoaUtils::GeckoRectToCocoaRectDevPix(newBounds, scaleFactor);
+  }

  // We ignore aRepaint -- we have to call display:YES, otherwise the
  // title bar doesn't immediately get repainted and is displayed in
@ -1344,6 +1357,7 @@ nsresult nsCocoaWindow::DoResize(int32_t aX, int32_t aY,
  NS_OBJC_END_TRY_ABORT_BLOCK_NSRESULT;
 }

+// Coordinates are global display pixels
 NS_IMETHODIMP nsCocoaWindow::Resize(int32_t aX, int32_t aY,
                                    int32_t aWidth, int32_t aHeight,
                                    bool aRepaint)
@ -1351,9 +1365,13 @@ NS_IMETHODIMP nsCocoaWindow::Resize(int32_t aX, int32_t aY,
  return DoResize(aX, aY, aWidth, aHeight, aRepaint, false);
 }

+// Coordinates are global display pixels
 NS_IMETHODIMP nsCocoaWindow::Resize(int32_t aWidth, int32_t aHeight, bool aRepaint)
 {
-  return DoResize(mBounds.x, mBounds.y, aWidth, aHeight, aRepaint, true);
+  double invScale = 1.0 / GetDefaultScale();
+  return DoResize(NSToIntRound(mBounds.x * invScale),
+                  NSToIntRound(mBounds.y * invScale),
+                  aWidth, aHeight, aRepaint, true);
 }

 NS_IMETHODIMP nsCocoaWindow::GetClientBounds(nsIntRect &aRect)
--- a/widget/nsIWidget.h
+++ b/widget/nsIWidget.h
@ -688,6 +688,19 @@ class nsIWidget : public nsISupports {
                                 int32_t *aX,
                                 int32_t *aY) = 0;

+    /**
+     * NOTE:
+     *
+     * For a top-level window widget, the "parent's coordinate system" is the
+     * "global" display pixel coordinate space, *not* device pixels (which
+     * may be inconsistent between multiple screens, at least in the Mac OS
+     * case with mixed hi-dpi and lo-dpi displays). This applies to all the
+     * following Move and Resize widget APIs.
+     *
+     * Currently, only Mac OS X implements a display-/device-pixel distinction;
+     * this may change in future, however.
+     **/
+
    /**
     * Move this widget.
     *
--- a/xpfe/appshell/src/nsXULWindow.cpp
+++ b/xpfe/appshell/src/nsXULWindow.cpp
@ -534,7 +534,10 @@ NS_IMETHODIMP nsXULWindow::SetPosition(int32_t aX, int32_t aY)
 {
  // Don't reset the window's size mode here - platforms that don't want to move
  // maximized windows should reset it in their respective Move implementation.
-  NS_ENSURE_SUCCESS(mWindow->Move(aX, aY), NS_ERROR_FAILURE);
+  double invScale = 1.0 / mWindow->GetDefaultScale();
+  nsresult rv = mWindow->Move(NSToIntRound(aX * invScale),
+                              NSToIntRound(aY * invScale));
+  NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
  if (!mChromeLoaded) {
    // If we're called before the chrome is loaded someone obviously wants this
    // window at this position. We don't persist this one-time position.
@ -560,7 +563,11 @@ NS_IMETHODIMP nsXULWindow::SetSize(int32_t aCX, int32_t aCY, bool aRepaint)

  mIntrinsicallySized = false;

-  NS_ENSURE_SUCCESS(mWindow->Resize(aCX, aCY, aRepaint), NS_ERROR_FAILURE);
+  double invScale = 1.0 / mWindow->GetDefaultScale();
+  nsresult rv = mWindow->Resize(NSToIntRound(aCX * invScale),
+                                NSToIntRound(aCY * invScale),
+                                aRepaint);
+  NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
  if (!mChromeLoaded) {
    // If we're called before the chrome is loaded someone obviously wants this
    // window at this size & in the normal size mode (since it is the only mode
@ -590,7 +597,13 @@ NS_IMETHODIMP nsXULWindow::SetPositionAndSize(int32_t aX, int32_t aY,

  mIntrinsicallySized = false;

-  NS_ENSURE_SUCCESS(mWindow->Resize(aX, aY, aCX, aCY, aRepaint), NS_ERROR_FAILURE);
+  double invScale = 1.0 / mWindow->GetDefaultScale();
+  nsresult rv = mWindow->Resize(NSToIntRound(aX * invScale),
+                                NSToIntRound(aY * invScale),
+                                NSToIntRound(aCX * invScale),
+                                NSToIntRound(aCY * invScale),
+                                aRepaint);
+  NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
  if (!mChromeLoaded) {
    // If we're called before the chrome is loaded someone obviously wants this
    // window at this size and position. We don't persist this one-time setting.