layout/fixed: Prevent overloading widgets with negative spacing

For each widget, the layout function checks whether placing it would
make the function exceed the allowed geometry.
If not, the function places both the widget and a spacing widget.
This check ignores the size of the spacing widget itself, this can cause
overloading of widgets on top of each other.

For example, the following scenario with these widgets:
    widgets: widget1 { width = 10, height = 10  }
	     widget2 { width = 10, height = 10  }
	     widget3 { width = 10, height = 10  }
and a call to horizontal layout with the
{ width = 10, height = 10, spacing = -5 } parameters.

The function would layout the widgets the following way:
{
    widget1: { x = 0, y = 0, width = 10, height = 10 }
    spacing: { x = 5, y = 0, width = 5, height = 10  }
    widget2: { x = 5, y = 0, width = 5, height = 10  }
    spacing: { x = 5, y = 0, width = 5, height = 10 }
    widget3: { x = 5, y = 0, width = 5, height = 10  }
}

This behaviour would be the same for any number of widgets for negative
layout.

This patch changes the layout function to check whether the current
widget uses up the whole space.
It also removes 'pos' variable. Its purpose isn't intuitive in the
presence of x and y. This helps to understand where each widget is
placed now that x, y don't hold the end location of the widget in the
previous loop iteration.

The result of the previous example becomes:
{
    widget1: { x = 0, y = 0, width = 10, height = 10 }
}

While this might not be the wanted behaviour exactly, distinguishing
between the scenario where 2 widgets are drawn and a scenario where 3
are drawn might complicate the layout function too much.

This patch also adds unit testing that catches the described behaviour.

Signed-off-by: Shay Agroskin <agrosshay@gmail.com>

lib/wibox/layout/fixed.lua

@@ -20,45 +20,61 @@ local fixed = {}
 -- @param height The available height.
 function fixed:layout(context, width, height)
     local result = {}
-    local pos,spacing = 0, self._private.spacing
+    local spacing = self._private.spacing or 0
-    local spacing_widget = self._private.spacing_widget
     local is_y = self._private.dir == "y"
     local is_x = not is_y
     local abspace = math.abs(spacing)
     local spoffset = spacing < 0 and 0 or spacing
+    local widgets_nr = #self._private.widgets
+    local spacing_widget
+    local x, y = 0, 0
+
+    spacing_widget = spacing ~= 0 and self._private.spacing_widget or nil
 
     for k, v in pairs(self._private.widgets) do
-        local x, y, w, h, _
+        local w, h = width - x, height - y
+
         if is_y then
-            x, y = 0, pos
+            if k ~= widgets_nr or not self._private.fill_space then
-            w, h = width, height - pos
+                h = select(2, base.fit_widget(self, context, v, w, h))
-            if k ~= #self._private.widgets or not self._private.fill_space then
+            end
-                _, h = base.fit_widget(self, context, v, w, h);
+
+            if y - spacing >= height then
+                -- pop the spacing widget added in previous iteration if used
+                if spacing_widget then
+                    table.remove(result)
+                end
+                break
             end
-            pos = pos + h + spacing
         else
-            x, y = pos, 0
+            if k ~= widgets_nr or not self._private.fill_space then
-            w, h = width - pos, height
+                w = select(1, base.fit_widget(self, context, v, w, h))
-            if k ~= #self._private.widgets or not self._private.fill_space then
+            end
-                w, _ = base.fit_widget(self, context, v, w, h);
+
+            if x - spacing >= width then
+                -- pop the spacing widget added in previous iteration if used
+                if spacing_widget then
+                    table.remove(result)
+                end
+                break
             end
-            pos = pos + w + spacing
         end
 
-        if (is_y and pos-spacing > height) or
+        -- Place widget
-            (is_x and pos-spacing > width) then
+        table.insert(result, base.place_widget_at(v, x, y, w, h))
-            break
-        end
 
-        -- Add the spacing widget
+        x = is_x and x + w + spacing or x
-        if k > 1 and abspace > 0 and spacing_widget then
+        y = is_y and y + h + spacing or y
+
+        -- Add the spacing widget (if needed)
+        if k < widgets_nr and spacing_widget then
             table.insert(result, base.place_widget_at(
                 spacing_widget, is_x and (x - spoffset) or x, is_y and (y - spoffset) or y,
                 is_x and abspace or w, is_y and abspace or h
             ))
         end
-        table.insert(result, base.place_widget_at(v, x, y, w, h))
     end
+
     return result
 end
 

spec/wibox/layout/fixed_spec.lua

@@ -78,7 +78,6 @@ describe("wibox.layout.fixed", function()
                     assert.widget_layout(layout, { 100, 20 }, {
                         p(first,  0,  0, 100, 10),
                         p(second, 0, 10, 100, 10),
-                        p(third,  0, 20, 100,  0),
                     })
                 end)
             end)
@@ -169,6 +168,14 @@ describe("wibox.layout.fixed", function()
                     it("fit", function()
                         assert.widget_fit(layout, { 15, 20 }, { 15, 20 })
                     end)
+
+                    it("layout", function()
+                        assert.widget_layout(layout, { 15, 20 }, {
+                            p(first,  0, 0, 15, 10),
+                            p(spacing_widget,  0, 5, 15, 5),
+                            p(second, 0, 5, 15, 15),
+                        })
+                    end)
                 end)
             end) -- , negative spacing
         end) -- with spacing