Bug 1757106 - Cleanup size/pos handling in NativeMoveResize. r=stransky
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 01 Mar 2022 01:06:37 +0000
changeset 609229 975f49d4b6859c61daca155e501fac1983662de8
parent 609228 420728eba6a02ac8f369382b669881f2c808b206
child 609230 1adc73ab00de0fdb1f51b80522613513bf307bb2
push id158659
push userealvarez@mozilla.com
push dateTue, 01 Mar 2022 01:09:00 +0000
treeherderautoland@975f49d4b685 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersstransky
bugs1757106
milestone99.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 1757106 - Cleanup size/pos handling in NativeMoveResize. r=stransky DevicePixelsToGdkRectRoundOut should have the same behavior here and is simpler / less weird than having a positionless rect variable to represent a size. Depends on D139623 Differential Revision: https://phabricator.services.mozilla.com/D139672
widget/gtk/nsWindow.cpp
widget/gtk/nsWindow.h
--- a/widget/gtk/nsWindow.cpp
+++ b/widget/gtk/nsWindow.cpp
@@ -2024,17 +2024,17 @@ void nsWindow::WaylandPopupSetDirectPosi
     if (nsMenuPopupFrame* popupFrame = GetMenuPopupFrame(GetFrame())) {
       auto p = CSSIntPoint::Round(
           mBounds.TopLeft() / popupFrame->PresContext()->CSSToDevPixelScale());
       popupFrame->MoveTo(p, true);
     }
   }
 }
 
-bool nsWindow::WaylandPopupFitsParentWindow(GdkRectangle* aSize) {
+bool nsWindow::WaylandPopupFitsParentWindow(const GdkRectangle& aSize) {
   GtkWindow* parentGtkWindow = gtk_window_get_transient_for(GTK_WINDOW(mShell));
   nsWindow* parentWindow =
       get_window_for_gtk_widget(GTK_WIDGET(parentGtkWindow));
 
   // Don't try to fiddle with popup position when our parent is also popup.
   // Use layout setup in such case.
   if (parentWindow->IsPopup()) {
     return false;
@@ -2047,31 +2047,30 @@ bool nsWindow::WaylandPopupFitsParentWin
   // x,y are offsets of mozcontainer, i.e. size of CSD decorations size.
   int x, y;
   gdk_window_get_position(parentGdkWindow, &x, &y);
 
   // We use parent mozcontainer size plus left/top CSD decorations sizes as
   // this coordinates are used by mBounds.
   int parentWidth = gdk_window_get_width(parentGdkWindow) + x;
   int parentHeight = gdk_window_get_width(parentGdkWindow) + y;
-  int popupWidth = aSize->width;
-  int popupHeight = aSize->height;
+  int popupWidth = aSize.width;
+  int popupHeight = aSize.height;
 
   auto popupBounds = DevicePixelsToGdkRectRoundOut(mBounds);
 
   return popupBounds.x + popupWidth <= parentWidth &&
          popupBounds.y + popupHeight <= parentHeight;
 }
 
 void nsWindow::NativeMoveResizeWaylandPopup(bool aMove, bool aResize) {
-  GdkPoint position = DevicePixelsToGdkPointRoundDown(mBounds.TopLeft());
-  GdkRectangle size = DevicePixelsToGdkSizeRoundUp(mBounds.Size());
-
-  LOG("nsWindow::NativeMoveResizeWaylandPopup %d,%d -> %d x %d\n", position.x,
-      position.y, size.width, size.height);
+  GdkRectangle rect = DevicePixelsToGdkRectRoundOut(mBounds);
+
+  LOG("nsWindow::NativeMoveResizeWaylandPopup %d,%d -> %d x %d\n", rect.x,
+      rect.y, rect.width, rect.height);
 
   // Compositor may be confused by windows with width/height = 0
   // and positioning such windows leads to Bug 1555866.
   if (!AreBoundsSane()) {
     LOG("  Bounds are not sane (width: %d height: %d)\n", mBounds.width,
         mBounds.height);
     return;
   }
@@ -2092,35 +2091,35 @@ void nsWindow::NativeMoveResizeWaylandPo
   mNewBoundsAfterMoveToRect = LayoutDeviceIntRect(0, 0, 0, 0);
 
   if (!WaylandPopupNeedsTrackInHierarchy()) {
     WaylandPopupSetDirectPosition();
     return;
   }
 
   if (aResize) {
-    LOG("  set size [%d, %d]\n", size.width, size.height);
-    gtk_window_resize(GTK_WINDOW(mShell), size.width, size.height);
-  }
-
-  if (!aMove && WaylandPopupFitsParentWindow(&size)) {
+    LOG("  set size [%d, %d]\n", rect.width, rect.height);
+    gtk_window_resize(GTK_WINDOW(mShell), rect.width, rect.height);
+  }
+
+  if (!aMove && WaylandPopupFitsParentWindow(rect)) {
     // Popup position has not been changed and its position/size fits
     // parent window so no need to reposition the window.
     LOG("  fits parent window size, just resize\n");
     return;
   }
 
   // Mark popup as changed as we're updating position/size.
   mPopupChanged = true;
 
   // Save popup position for former re-calculations when popup hierarchy
   // is changed.
   LOG("  popup position changed from [%d, %d] to [%d, %d]\n", mPopupPosition.x,
-      mPopupPosition.y, position.x, position.y);
-  mPopupPosition = position;
+      mPopupPosition.y, rect.x, rect.y);
+  mPopupPosition = {rect.x, rect.y};
 
   UpdateWaylandPopupHierarchy();
 }
 
 struct PopupSides {
   Maybe<Side> mVertical;
   Maybe<Side> mHorizontal;
 };
@@ -5935,65 +5934,64 @@ void nsWindow::SetWindowClass(const nsAS
 
 nsAutoCString nsWindow::GetDebugTag() const {
   nsAutoCString tag;
   tag.AppendPrintf("[%p]", this);
   return tag;
 }
 
 void nsWindow::NativeMoveResize(bool aMoved, bool aResized) {
-  GdkRectangle size = DevicePixelsToGdkSizeRoundUp(mBounds.Size());
-  GdkPoint topLeft = DevicePixelsToGdkPointRoundDown(mBounds.TopLeft());
+  GdkRectangle rect = DevicePixelsToGdkRectRoundOut(mBounds);
 
   LOG("nsWindow::NativeMoveResize move %d resize %d to %d,%d -> %d x %d\n",
-      aMoved, aResized, topLeft.x, topLeft.y, size.width, size.height);
+      aMoved, aResized, rect.x, rect.y, rect.width, rect.height);
 
   if (aResized && !AreBoundsSane()) {
     LOG("  bounds are insane, hidding the window");
     // We have been resized but to incorrect size.
     // If someone has set this so that the needs show flag is false
     // and it needs to be hidden, update the flag and hide the
     // window.  This flag will be cleared the next time someone
     // hides the window or shows it.  It also prevents us from
     // calling NativeShow(false) excessively on the window which
     // causes unneeded X traffic.
     if (!mNeedsShow && mIsShown) {
       mNeedsShow = true;
       NativeShow(false);
     }
     if (aMoved) {
-      LOG("  moving to %d x %d", topLeft.x, topLeft.y);
-      gtk_window_move(GTK_WINDOW(mShell), topLeft.x, topLeft.y);
+      LOG("  moving to %d x %d", rect.x, rect.y);
+      gtk_window_move(GTK_WINDOW(mShell), rect.x, rect.y);
     }
     return;
   }
 
   // Set position to hidden window on X11 may fail, so save the position
   // and move it when it's shown.
   if (aMoved && GdkIsX11Display() && IsPopup() &&
       !gtk_widget_get_visible(GTK_WIDGET(mShell))) {
     LOG("  store position of hidden popup window");
     mHiddenPopupPositioned = true;
-    mPopupPosition = topLeft;
+    mPopupPosition = {rect.x, rect.y};
   }
 
   if (IsWaylandPopup()) {
     NativeMoveResizeWaylandPopup(aMoved, aResized);
   } else {
     // x and y give the position of the window manager frame top-left.
     if (aMoved) {
-      gtk_window_move(GTK_WINDOW(mShell), topLeft.x, topLeft.y);
+      gtk_window_move(GTK_WINDOW(mShell), rect.x, rect.y);
     }
     if (aResized) {
-      gtk_window_resize(GTK_WINDOW(mShell), size.width, size.height);
+      gtk_window_resize(GTK_WINDOW(mShell), rect.width, rect.height);
       if (mIsDragPopup) {
         // DND window is placed inside container so we need to make hard size
         // request to ensure parent container is resized too.
-        gtk_widget_set_size_request(GTK_WIDGET(mShell), size.width,
-                                    size.height);
+        gtk_widget_set_size_request(GTK_WIDGET(mShell), rect.width,
+                                    rect.height);
       }
     }
   }
 
   // Notify the GtkCompositorWidget of a ClientSizeChange
   // This is different than OnSizeAllocate to catch initial sizing
   if (mCompositorWidgetDelegate && aResized) {
     mCompositorWidgetDelegate->NotifyClientSizeChanged(GetClientSize());
--- a/widget/gtk/nsWindow.h
+++ b/widget/gtk/nsWindow.h
@@ -764,17 +764,17 @@ class nsWindow final : public nsBaseWidg
   void WaylandPopupMove();
   bool WaylandPopupRemoveNegativePosition(int* aX = nullptr, int* aY = nullptr);
   nsWindow* WaylandPopupGetTopmostWindow();
   bool IsPopupInLayoutPopupChain(nsTArray<nsIWidget*>* aLayoutWidgetHierarchy,
                                  bool aMustMatchParent);
   void WaylandPopupMarkAsClosed();
   void WaylandPopupRemoveClosedPopups();
   void WaylandPopupSetDirectPosition();
-  bool WaylandPopupFitsParentWindow(GdkRectangle* aSize);
+  bool WaylandPopupFitsParentWindow(const GdkRectangle& aSize);
   nsWindow* WaylandPopupFindLast(nsWindow* aPopup);
   GtkWindow* GetCurrentTopmostWindow();
   nsAutoCString GetFrameTag() const;
   nsCString GetPopupTypeName();
   bool IsPopupDirectionRTL();
 
 #ifdef MOZ_LOGGING
   void LogPopupHierarchy();