aspect ratios should take base size into account, fix size hint initialization

2 files changed, 109 insertions, 105 deletions

diff --git a/src/FbWinFrame.cc b/src/FbWinFrame.cc
index d044dd7..eb8510e 100644
--- a/src/FbWinFrame.cc
+++ b/src/FbWinFrame.cc
@@ -1689,6 +1689,14 @@ void closestPointToAspect(unsigned int &ret_x, unsigned int &ret_y,
     ret_y = static_cast<unsigned int>(u * aspect_y);
 }
 
+unsigned int increaseToMultiple(unsigned int val, unsigned int inc) {
+    return val % inc ? val + inc - (val % inc) : val;
+}
+
+unsigned int decreaseToMultiple(unsigned int val, unsigned int inc) {
+    return val % inc ? val - (val % inc) : val;
+}
+    
 /**
  * Changes width and height to the nearest (lower) value
  * that conforms to it's size hints.
@@ -1700,12 +1708,7 @@ void closestPointToAspect(unsigned int &ret_x, unsigned int &ret_y,
  * See ICCCM section 4.1.2.3
  */
 void FbWinFrame::SizeHints::apply(unsigned int &width, unsigned int &height,
-                                  bool maximizing) const {
-
-    // we apply aspect ratios before incrementals
-    // Too difficult to exactly satisfy both incremental+aspect
-    // in most situations
-    // (they really shouldn't happen at the same time anyway).
+                                  bool make_fit) const {
 
     /* aspect ratios are applied exclusive to the base size
      *
@@ -1713,8 +1716,6 @@ void FbWinFrame::SizeHints::apply(unsigned int &width, unsigned int &height,
      * ------------  <  -------  <  ------------
      * min_aspect_y      height     max_aspect_y
      *
-     * beware of integer maximum (so I'll use doubles instead and divide)
-     *
      * The trick is how to get back to the aspect ratio with minimal
      * change - do we modify x, y or both?
      * A: we minimise the distance between the current point, and
@@ -1722,43 +1723,62 @@ void FbWinFrame::SizeHints::apply(unsigned int &width, unsigned int &height,
      *  Consider that the aspect ratio is a line, and the current
      *  w/h is a point, so we're just using the formula for
      *  shortest distance from a point to a line!
-     *
-     * When maximizing, we must not increase any of the sizes, because we
-     * would end up with the window partly off a screen, so a simpler formula
-     * is used in that case.
      */
 
-    if (min_aspect_y > 0 && width*min_aspect_y < min_aspect_x*height) {
-        if (maximizing)
-            height = width * min_aspect_y / min_aspect_x;
-        else
-            closestPointToAspect(width, height, width, height,
-                                 min_aspect_x, min_aspect_y);
-    } else if (max_aspect_x > 0 && width*max_aspect_y > max_aspect_x*height) {
-        if (maximizing)
-            width = height * max_aspect_x / max_aspect_y;
-        else
-            closestPointToAspect(width, height, width, height,
-                                 max_aspect_x, max_aspect_y);
+    // make respective to base_size
+    unsigned int w = width - base_width, h = height - base_height;
+
+    if (min_aspect_y > 0 && w * min_aspect_y < min_aspect_x * h) {
+        closestPointToAspect(w, h, w, h, min_aspect_x, min_aspect_y);
+        // new w must be > old w, new h must be < old h
+        w = increaseToMultiple(w, width_inc);
+        h = decreaseToMultiple(h, height_inc);
+    } else if (max_aspect_x > 0 && w * max_aspect_y > max_aspect_x * h) {
+        closestPointToAspect(w, h, w, h, max_aspect_x, max_aspect_y);
+        // new w must be < old w, new h must be > old h
+        w = decreaseToMultiple(w, width_inc);
+        h = increaseToMultiple(h, height_inc);
     }
 
     // Check minimum size
-    if (width < min_width)
-        width = min_width;
+    if (w + base_width < min_width) {
+        w = increaseToMultiple(min_width - base_width, width_inc);
+        // need to check maximum aspect again
+        if (max_aspect_x > 0 && w * max_aspect_y > max_aspect_x * h)
+            h = increaseToMultiple(w * max_aspect_y / max_aspect_x, height_inc);
+    }
+
+    if (h + base_height < min_height) {
+        h = increaseToMultiple(min_height - base_height, height_inc);
+        // need to check minimum aspect again
+        if (min_aspect_y > 0 && w * min_aspect_y < min_aspect_x * h)
+            w = increaseToMultiple(h * min_aspect_x / min_aspect_y, width_inc);
+    }
 
-    if (height < min_height)
-        height = min_height;
+    unsigned int max_w = make_fit && (width < max_width || max_width == 0) ?
+                         width : max_width;
+    unsigned int max_h = make_fit && (height < max_height || max_height == 0) ?
+                         height : max_height;
 
     // Check maximum size
-    if (max_width > 0 && width > max_width)
-        width = max_width;
+    if (max_w > 0 && w + base_width > max_w)
+        w = max_w - base_width;
+
+    if (max_h > 0 && h + base_height > max_h)
+        h = max_h - base_height;
+
+    w = decreaseToMultiple(w, width_inc);
+    h = decreaseToMultiple(h, height_inc);
+
+    // need to check aspects one more time
+    if (min_aspect_y > 0 && w * min_aspect_y < min_aspect_x * h)
+        h = decreaseToMultiple(w * min_aspect_y / min_aspect_x, height_inc);
 
-    if (max_height > 0 && height > max_height)
-        height = max_height;
+    if (max_aspect_x > 0 && w * max_aspect_y > max_aspect_x * h)
+        w = decreaseToMultiple(h * max_aspect_x / max_aspect_y, width_inc);
 
-    // enforce incremental size limits, wrt base size
-    width -= (width - base_width) % width_inc;
-    height -= (height - base_height) % height_inc;
+    width = w + base_width;
+    height = h + base_height;
 }
 
 // check if the given width and height satisfy the size hints
@@ -1775,10 +1795,10 @@ bool FbWinFrame::SizeHints::valid(unsigned int w, unsigned int h) const {
     if ((h - base_height) % height_inc != 0)
         return false;
 
-    if (min_aspect_x * h > w * min_aspect_y)
+    if (min_aspect_x * (h - base_height) > (w - base_width) * min_aspect_y)
         return false;
 
-    if (max_aspect_x * h < w * max_aspect_y)
+    if (max_aspect_x * (h - base_height) < (w - base_width) * max_aspect_y)
         return false;
 
     return true;
diff --git a/src/WinClient.cc b/src/WinClient.cc
index f421013..2c9cdc6 100644
--- a/src/WinClient.cc
+++ b/src/WinClient.cc
@@ -82,14 +82,6 @@ WinClient::WinClient(Window win, BScreen &screen, FluxboxWindow *fbwin):
                      m_mwm_hint(0),
                      m_strut(0) {
 
-    m_size_hints.min_width = m_size_hints.min_height =
-        m_size_hints.width_inc = m_size_hints.height_inc =
-        m_size_hints.base_width = m_size_hints.base_height = 1;
-
-    m_size_hints.max_width = m_size_hints.max_height =
-        m_size_hints.min_aspect_x = m_size_hints.min_aspect_y =
-        m_size_hints.max_aspect_x = m_size_hints.max_aspect_y = 0;
-
     updateWMProtocols();
     updateMWMHints();
     updateWMHints();
@@ -471,73 +463,65 @@ void WinClient::updateWMHints() {
 void WinClient::updateWMNormalHints() {
     long icccm_mask;
     XSizeHints sizehint;
-    if (! XGetWMNormalHints(display(), window(), &sizehint, &icccm_mask)) {
-        m_size_hints.min_width = m_size_hints.min_height =
-            m_size_hints.base_width = m_size_hints.base_height =
-            m_size_hints.width_inc = m_size_hints.height_inc = 1;
+    if (!XGetWMNormalHints(display(), window(), &sizehint, &icccm_mask))
+        sizehint.flags = 0;
+
+    normal_hint_flags = sizehint.flags;
+
+    if (sizehint.flags & PMinSize) {
+        m_size_hints.min_width = sizehint.min_width;
+        m_size_hints.min_height = sizehint.min_height;
+    } else
+        m_size_hints.min_width = m_size_hints.min_height = 1;
+
+    if (sizehint.flags & PBaseSize) {
+        m_size_hints.base_width = sizehint.base_width;
+        m_size_hints.base_height = sizehint.base_height;
+        if (!(sizehint.flags & PMinSize)) {
+            m_size_hints.min_width = m_size_hints.base_width;
+            m_size_hints.min_height = m_size_hints.base_height;
+        }
+    } else
+        m_size_hints.base_width = m_size_hints.base_height = 0;
+
+    if (sizehint.flags & PMaxSize) {
+        m_size_hints.max_width = sizehint.max_width;
+        m_size_hints.max_height = sizehint.max_height;
+    } else {
         m_size_hints.max_width = 0; // unbounded
         m_size_hints.max_height = 0;
+    }
+
+    if (sizehint.flags & PResizeInc) {
+        m_size_hints.width_inc = sizehint.width_inc;
+        m_size_hints.height_inc = sizehint.height_inc;
+    } else
+        m_size_hints.width_inc = m_size_hints.height_inc = 1;
+
+    if (sizehint.flags & PAspect) {
+        m_size_hints.min_aspect_x = sizehint.min_aspect.x;
+        m_size_hints.min_aspect_y = sizehint.min_aspect.y;
+        m_size_hints.max_aspect_x = sizehint.max_aspect.x;
+        m_size_hints.max_aspect_y = sizehint.max_aspect.y;
+    } else
         m_size_hints.min_aspect_x = m_size_hints.min_aspect_y =
             m_size_hints.max_aspect_x = m_size_hints.max_aspect_y = 0;
-        m_win_gravity = NorthWestGravity;
-    } else {
-        normal_hint_flags = sizehint.flags;
-
-        if (sizehint.flags & PMinSize) {
-            m_size_hints.min_width = sizehint.min_width;
-            m_size_hints.min_height = sizehint.min_height;
-            if (!(sizehint.flags & PBaseSize)) {
-                m_size_hints.base_width = m_size_hints.min_width;
-                m_size_hints.base_height = m_size_hints.min_height;
-            }
-        } else {
-            m_size_hints.min_width = m_size_hints.min_height = 1;
-            m_size_hints.base_width = m_size_hints.base_height = 0;
-        }
-
-        if (sizehint.flags & PBaseSize) {
-            m_size_hints.base_width = sizehint.base_width;
-            m_size_hints.base_height = sizehint.base_height;
-            if (!(sizehint.flags & PMinSize)) {
-                m_size_hints.min_width = m_size_hints.base_width;
-                m_size_hints.min_height = m_size_hints.base_height;
-            }
-        } // default set in PMinSize
-
-        if (sizehint.flags & PMaxSize) {
-            m_size_hints.max_width = sizehint.max_width;
-            m_size_hints.max_height = sizehint.max_height;
-        } else {
-            m_size_hints.max_width = 0; // unbounded
-            m_size_hints.max_height = 0;
-        }
 
-        if (sizehint.flags & PResizeInc) {
-            m_size_hints.width_inc = sizehint.width_inc;
-            m_size_hints.height_inc = sizehint.height_inc;
-        } else
-            m_size_hints.width_inc = m_size_hints.height_inc = 1;
-
-        if (m_size_hints.width_inc == 0)
-            m_size_hints.width_inc = 1;
-        if (m_size_hints.height_inc == 0)
-            m_size_hints.height_inc = 1;
-
-        if (sizehint.flags & PAspect) {
-            m_size_hints.min_aspect_x = sizehint.min_aspect.x;
-            m_size_hints.min_aspect_y = sizehint.min_aspect.y;
-            m_size_hints.max_aspect_x = sizehint.max_aspect.x;
-            m_size_hints.max_aspect_y = sizehint.max_aspect.y;
-        } else
-            m_size_hints.min_aspect_x = m_size_hints.min_aspect_y =
-                m_size_hints.max_aspect_x = m_size_hints.max_aspect_y = 0;
+    if (sizehint.flags & PWinGravity)
+        m_win_gravity = sizehint.win_gravity;
+    else
+        m_win_gravity = NorthWestGravity;
 
-        if (sizehint.flags & PWinGravity)
-            m_win_gravity = sizehint.win_gravity;
-        else
-            m_win_gravity = NorthWestGravity;
+    // some sanity checks
+    if (m_size_hints.width_inc == 0)
+        m_size_hints.width_inc = 1;
+    if (m_size_hints.height_inc == 0)
+        m_size_hints.height_inc = 1;
 
-    }
+    if (m_size_hints.base_width > m_size_hints.min_width)
+        m_size_hints.min_width = m_size_hints.base_width;
+    if (m_size_hints.base_height > m_size_hints.min_height)
+        m_size_hints.min_height = m_size_hints.base_height;
 }
 
 Window WinClient::getGroupLeftWindow() const {