layout/fixed: Fix wrong space calculation in fit

The fit function is called twice in row.

- The first time it gets the maximum available width, and returns how
  much of it it needs (with 0 spacing it would be 477)

- The second time the available width it gets is the same as it returned
  last phase (and probably is expected to return the same result again)

The width fit requests is the total width of all widgets together + the
spacing (e.g. if each tag widget is 53 px and spacing is -10 then the
requested width 53 * 9 - 80).

The function tries to first fit all its widgets (the tag numbers) in the
amount of width it received, and only then adds the spacing to it. This
is problematic because in the second phase the widgets need to fit
themselves in the same width they requested earlier minus the spacing
(in case of negative spacing). This is of course impossible and so some
widgets are just not being drawn correctly.

This patch makes fit function take into account the spacing while
placing the widgets and not afterwards.

Also add unit-testing that test the bug described.

Signed-off-by: Shay Agroskin <agrosshay@gmail.com>
This commit is contained in:
Shay Agroskin 2021-01-01 19:07:33 +02:00 committed by Shay Agroskin
parent 749422100e
commit a18e3508f6
2 changed files with 145 additions and 42 deletions

View File

@ -263,16 +263,17 @@ end
function fixed:fit(context, orig_width, orig_height)
local width_left, height_left = orig_width, orig_height
local spacing = self._private.spacing or 0
local widgets_nr = #self._private.widgets
local is_y = self._private.dir == "y"
local used_max = 0
-- when no widgets exist the function can be called with orig_width or
-- orig_height equal to nil. Exit early in this case.
if #self._private.widgets == 0 then
if widgets_nr == 0 then
return 0, 0
end
for _, v in pairs(self._private.widgets) do
for k, v in pairs(self._private.widgets) do
local w, h = base.fit_widget(self, context, v, width_left, height_left)
local max
@ -288,23 +289,33 @@ function fixed:fit(context, orig_width, orig_height)
used_max = max
end
if width_left <= 0 or height_left <= 0 then
if k < widgets_nr then
if is_y then
height_left = 0
height_left = height_left - spacing
else
width_left = 0
width_left = width_left - spacing
end
end
if width_left <= 0 or height_left <= 0 then
-- this complicated two lines determine whether we're out-of-space
-- because of spacing, or if the last widget doesn't fit in
if is_y then
height_left = k < widgets_nr and height_left + spacing or height_left
height_left = height_left < 0 and 0 or height_left
else
width_left = k < widgets_nr and width_left + spacing or width_left
width_left = width_left < 0 and 0 or width_left
end
break
end
end
local total_spacing = spacing * (#self._private.widgets-1)
if is_y then
return used_max, orig_height - height_left + total_spacing
return used_max, orig_height - height_left
end
return orig_width - width_left + total_spacing, used_max
return orig_width - width_left, used_max
end
function fixed:reset()

View File

@ -39,48 +39,140 @@ describe("wibox.layout.fixed", function()
layout:add(first, second, third)
end)
describe("with enough space", function()
it("fit", function()
assert.widget_fit(layout, { 100, 100 }, { 15, 35 })
describe("without spacing", function()
describe("with enough space", function()
it("fit", function()
assert.widget_fit(layout, { 100, 100 }, { 15, 35 })
end)
it("layout", function()
assert.widget_layout(layout, { 100, 100 }, {
p(first, 0, 0, 100, 10),
p(second, 0, 10, 100, 15),
p(third, 0, 25, 100, 10),
})
end)
end)
it("layout", function()
assert.widget_layout(layout, { 100, 100 }, {
p(first, 0, 0, 100, 10),
p(second, 0, 10, 100, 15),
p(third, 0, 25, 100, 10),
})
describe("without enough height", function()
it("fit", function()
assert.widget_fit(layout, { 5, 100 }, { 5, 35 })
end)
it("layout", function()
assert.widget_layout(layout, { 5, 100 }, {
p(first, 0, 0, 5, 10),
p(second, 0, 10, 5, 15),
p(third, 0, 25, 5, 10),
})
end)
end)
describe("without enough width", function()
it("fit", function()
assert.widget_fit(layout, { 100, 20 }, { 15, 20 })
end)
it("layout", 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)
end)
describe("without enough height", function()
it("fit", function()
assert.widget_fit(layout, { 5, 100 }, { 5, 35 })
describe("with spacing", function()
local spacing_widget = utils.widget_stub(10, 10)
before_each(function()
layout:set_spacing_widget(spacing_widget)
end)
it("layout", function()
assert.widget_layout(layout, { 5, 100 }, {
p(first, 0, 0, 5, 10),
p(second, 0, 10, 5, 15),
p(third, 0, 25, 5, 10),
})
end)
end)
describe(", positive spacing", function()
before_each(function()
layout:set_spacing(10)
end)
describe("without enough width", function()
it("fit", function()
assert.widget_fit(layout, { 100, 20 }, { 15, 20 })
end)
describe("and with enough space", function()
it("fit", function()
assert.widget_fit(layout, { 100, 100 }, { 15, 55 })
end)
it("layout", 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)
end)
it("layout", function()
assert.widget_layout(layout, { 100, 100 }, {
p(first, 0, 0, 100, 10),
p(spacing_widget, 0, 10, 100, 10),
p(second, 0, 20, 100, 15),
p(spacing_widget, 0, 35, 100, 10),
p(third, 0, 45, 100, 10),
})
end)
end)
describe("and without enough space", function()
it("fit", function()
assert.widget_fit(layout, { 100, 45 }, { 15, 35 })
end)
it("layout", function()
assert.widget_layout(layout, { 100, 35 }, {
p(first, 0, 0, 100, 10),
p(spacing_widget, 0, 10, 100, 10),
p(second, 0, 20, 100, 15),
})
end)
end)
end) -- , positive spacing
describe(", negative spacing", function()
before_each(function()
layout:set_spacing(-5)
end)
describe("and with more than needed space", function()
it("fit", function()
assert.widget_fit(layout, { 100, 100 }, { 15, 25 })
end)
it("layout", function()
assert.widget_layout(layout, { 100, 100 }, {
p(first, 0, 0, 100, 10),
p(spacing_widget, 0, 5, 100, 5),
p(second, 0, 5, 100, 15),
p(spacing_widget, 0, 15, 100, 5),
p(third, 0, 15, 100, 10),
})
end)
end)
describe("and with exactly the needed space", function()
it("fit", function()
assert.widget_fit(layout, { 15, 25 }, { 15, 25 })
end)
it("layout", function()
assert.widget_layout(layout, { 15, 25 }, {
p(first, 0, 0, 15, 10),
p(spacing_widget, 0, 5, 15, 5),
p(second, 0, 5, 15, 15),
p(spacing_widget, 0, 15, 15, 5),
p(third, 0, 15, 15, 10),
})
end)
end)
describe("and without enough space", function()
it("fit", function()
assert.widget_fit(layout, { 15, 20 }, { 15, 20 })
end)
end)
end) -- , negative spacing
end) -- with spacing
end) -- with widgets
describe("emitting signals", function()
local layout_changed