From 62850476d2d3b21abee791b40cfc2217cc3c4ced Mon Sep 17 00:00:00 2001 From: Shay Agroskin Date: Fri, 1 Jan 2021 21:13:19 +0200 Subject: [PATCH] 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 --- lib/wibox/layout/fixed.lua | 56 ++++++++++++++++++++------------ spec/wibox/layout/fixed_spec.lua | 9 ++++- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/lib/wibox/layout/fixed.lua b/lib/wibox/layout/fixed.lua index 6525f2ae5..0649b549f 100644 --- a/lib/wibox/layout/fixed.lua +++ b/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_widget = self._private.spacing_widget + local spacing = self._private.spacing or 0 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 - w, h = width, height - pos - if k ~= #self._private.widgets or not self._private.fill_space then - _, h = base.fit_widget(self, context, v, w, h); + if k ~= widgets_nr or not self._private.fill_space then + h = select(2, base.fit_widget(self, context, v, w, h)) + end + + 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 - w, h = width - pos, height - if k ~= #self._private.widgets or not self._private.fill_space then - w, _ = base.fit_widget(self, context, v, w, h); + if k ~= widgets_nr or not self._private.fill_space then + w = select(1, base.fit_widget(self, context, v, w, h)) + end + + 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 - (is_x and pos-spacing > width) then - break - end + -- Place widget + table.insert(result, base.place_widget_at(v, x, y, w, h)) - -- Add the spacing widget - if k > 1 and abspace > 0 and spacing_widget then + x = is_x and x + w + spacing or x + 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 diff --git a/spec/wibox/layout/fixed_spec.lua b/spec/wibox/layout/fixed_spec.lua index 537465679..3d50163ec 100644 --- a/spec/wibox/layout/fixed_spec.lua +++ b/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