fix(layout): wrap extent cycling when the next preset is out of reach

Fix #425

Author: ItsLemmy

src/layout/dwindle.cpp

@@ -689,16 +689,14 @@ namespace umbriel {
   }
 
   bool DwindleLayout::cycleWidth(int columnIndex, int direction) {
-    Node* node = nodeAtFlatIndex(columnIndex);
-    const std::vector<Split> axisSplits = splits(node, Node::HSplit);
-    if (axisSplits.empty()) {
-      return false;
-    }
-    double current = 1.0;
-    for (const Split& split : axisSplits) {
-      current *= splitShare(split);
-    }
-    return applyFraction(axisSplits, nextFractionPreset(m_config->extentPresets, current, direction));
+    // Each step reads the splits afresh: applying a fraction can move outer splits, which the cached products miss.
+    return cycleFractionPreset(
+        m_config->extentPresets,
+        widthFraction(columnIndex),
+        direction,
+        [&](double fraction) { return setWidthFraction(columnIndex, fraction); },
+        [&] { return widthFraction(columnIndex); }
+    );
   }
 
   bool DwindleLayout::toggleFullWidth(int columnIndex) {

src/layout/layout.h

@@ -205,6 +205,27 @@ namespace umbriel {
     return presets.front();
   }
 
+  // Steps a layout extent to the next preset. `apply` sets a fraction, clamped to what the layout can realize, and
+  // returns false when the extent is not adjustable; `realized` reads the fraction back. A preset past that range
+  // leaves the extent where it was and would be picked again on every step, so a step that does not move the extent
+  // wraps instead.
+  template <typename Apply, typename Realized>
+  bool cycleFractionPreset(
+      const std::vector<double>& presets, double current, int direction, Apply&& apply, Realized&& realized
+  ) {
+    const double next = nextFractionPreset(presets, current, direction);
+    if (!apply(next)) {
+      return false;
+    }
+    const bool stepped = direction < 0 ? next < current : next > current;
+    const double now = realized();
+    const bool stalled = direction < 0 ? now >= current - kFractionEpsilon : now <= current + kFractionEpsilon;
+    if (stepped && stalled) {
+      apply(direction < 0 ? presets.back() : presets.front());
+    }
+    return true;
+  }
+
   // Layout-owned interactive resize session. Cursor feeds a pointer delta; the session mutates only its layout's
   // geometry state (split ratios, width fractions, row weights). Protocol calls and arrange() stay in Cursor.
   struct ResizeGrab {

src/layout/master.cpp

@@ -915,8 +915,13 @@ namespace umbriel {
     if (!widthAdjustable() || visualArea(columnIndex) == nullptr) {
       return false;
     }
-    const double next = nextFractionPreset(m_config->extentPresets, widthFraction(columnIndex), direction);
-    return setWidthFraction(columnIndex, next);
+    return cycleFractionPreset(
+        m_config->extentPresets,
+        widthFraction(columnIndex),
+        direction,
+        [&](double fraction) { return setWidthFraction(columnIndex, fraction); },
+        [&] { return widthFraction(columnIndex); }
+    );
   }
 
   bool MasterStackLayout::toggleFullWidth(int columnIndex) {

src/workspace/workspace.cpp

@@ -1639,9 +1639,12 @@ namespace umbriel {
     if (m_focusedView != nullptr && m_focusedView->maximizedToEdges()) {
       m_focusedView->setMaximizedToEdges(false);
     }
-    const double current = m_layout->heightFraction(m_focusedView);
-    if (!m_layout->setHeightFraction(
-            m_focusedView, nextFractionPreset(m_layoutConfig.extentPresets, current, direction)
+    if (!cycleFractionPreset(
+            m_layoutConfig.extentPresets,
+            m_layout->heightFraction(m_focusedView),
+            direction,
+            [&](double fraction) { return m_layout->setHeightFraction(m_focusedView, fraction); },
+            [&] { return m_layout->heightFraction(m_focusedView); }
         )) {
       return false;
     }

tests/unit/dwindle_layout.cpp

@@ -817,6 +817,19 @@ UMBRIEL_TEST(cycleWidthBackWalksThePresetsInReverse) {
   CHECK(std::abs(fixture.layout.targetBox(stub(0)).width - static_cast<int>(span * (2.0 / 3.0))) <= 2);
 }
 
+UMBRIEL_TEST(cycleWidthWrapsWhenTheNextPresetIsOutOfReach) {
+  // A split share tops out at 0.9, short of the 1.0 preset.
+  Fixture fixture;
+  fixture.config.extentPresets = {0.5, 1.0};
+  fixture.addLeaves(2);
+  fixture.layout.arrange(kUsable);
+  CHECK(fixture.layout.setWidthFraction(0, 0.5));
+  CHECK(fixture.layout.cycleWidth(0, 1));
+  CHECK(std::abs(fixture.layout.widthFraction(0) - 0.9) < 1e-9);
+  CHECK(fixture.layout.cycleWidth(0, 1));
+  CHECK(std::abs(fixture.layout.widthFraction(0) - 0.5) < 1e-9);
+}
+
 UMBRIEL_TEST(resizeEdgesComeFromTileThirds) {
   Fixture fixture;
   fixture.addLeaves(2);

tests/unit/master_layout.cpp

@@ -255,6 +255,20 @@ UMBRIEL_TEST(cycleWidthWalksThePresets) {
   CHECK(std::abs(fixture.layout.widthFraction(0) - 1.0 / 3.0) < 1e-9);
 }
 
+UMBRIEL_TEST(cycleWidthWrapsWhenTheNextPresetIsOutOfReach) {
+  // A centered side column tops out at 0.45, short of the 0.5 preset.
+  Fixture fixture;
+  fixture.config.master.position = umbriel::MasterPosition::Center;
+  fixture.addViews(3);
+  CHECK(std::abs(fixture.layout.widthFraction(0) - 0.225) < 1e-9);
+  CHECK(fixture.layout.cycleWidth(0, 1));
+  CHECK(std::abs(fixture.layout.widthFraction(0) - 1.0 / 3.0) < 1e-9);
+  CHECK(fixture.layout.cycleWidth(0, 1));
+  CHECK(std::abs(fixture.layout.widthFraction(0) - 0.45) < 1e-9);
+  CHECK(fixture.layout.cycleWidth(0, 1));
+  CHECK(std::abs(fixture.layout.widthFraction(0) - 1.0 / 3.0) < 1e-9);
+}
+
 UMBRIEL_TEST(toggleFullWidthExpandsAndRestores) {
   Fixture fixture;
   fixture.addViews(2);