From 3fbd16d9a3458b39dc762a45a89f01d2c04aa60e Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Thu, 13 Aug 2015 15:42:39 +0200 Subject: [PATCH 1/2] wibox.layout.align: Correctly size second widget In expand nodes "none" and "outside", the variable size_remains describes how much space is available for the first/third widget. Everything else is used by the second widget. Thus, fitting the second widget to anything involving size_remains is wrong. Instead, this commit uses the correct value. This also fixes a messed up argument order for horizontal align layouts. Signed-off-by: Uli Schlachter --- lib/wibox/layout/align.lua | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/wibox/layout/align.lua b/lib/wibox/layout/align.lua index 1a048b85..0d33ed64 100644 --- a/lib/wibox/layout/align.lua +++ b/lib/wibox/layout/align.lua @@ -31,6 +31,9 @@ function align:draw(context, cr, width, height) local size_first = 0 -- start with all the space given by the parent, subtract as we go along local size_remains = self.dir == "y" and height or width + -- This is only set & used if expand ~= "inside" and we have second width. + -- It contains the size allocated to the second widget. + local size_second -- we will prioritize the middle widget unless the expand mode is "inside" -- if it is, we prioritize the first widget by not doing this block also, @@ -38,7 +41,7 @@ function align:draw(context, cr, width, height) -- instead if self._expand ~= "inside" and self.second then local w, h = base.fit_widget(context, self.second, width, height) - local size_second = self.dir == "y" and h or w + size_second = self.dir == "y" and h or w -- if all the space is taken, skip the rest, and draw just the middle -- widget if size_second >= size_remains then @@ -123,10 +126,10 @@ function align:draw(context, cr, width, height) end else if self.dir == "y" then - _, h = base.fit_widget(context, self.second, width, size_remains) + _, h = base.fit_widget(context, self.second, width, size_second) y = floor( (height - h)/2 ) else - w, _ = base.fit_widget(context, self.second, width, size_remains) + w, _ = base.fit_widget(context, self.second, size_second, height) x = floor( (width -w)/2 ) end end From bcc1751fca11736fcf9b7b183ecaba8f9596ad71 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Thu, 13 Aug 2015 15:48:56 +0200 Subject: [PATCH 2/2] fit_widget(): Sanitize the result of :fit() After this change, fit_widget() enforces that a widget cannot ask for more space than was offered to it. This also fixes a rounding issue in the flex layout where its fit function would return too small numbers. Thanks to this, lots of "XXX" comments in spec/ disappear. Signed-off-by: Uli Schlachter --- lib/wibox/layout/base.lua | 7 ++++++- lib/wibox/layout/flex.lua | 4 ++-- spec/wibox/layout/align_spec.lua | 23 ++++++++--------------- spec/wibox/layout/fixed_spec.lua | 6 +++--- spec/wibox/layout/flex_spec.lua | 6 ++---- spec/wibox/test_utils.lua | 2 +- 6 files changed, 22 insertions(+), 26 deletions(-) diff --git a/lib/wibox/layout/base.lua b/lib/wibox/layout/base.lua index 3e9a2832..a9205276 100644 --- a/lib/wibox/layout/base.lua +++ b/lib/wibox/layout/base.lua @@ -26,7 +26,12 @@ function base.fit_widget(context, widget, width, height) local width = math.max(0, width) local height = math.max(0, height) - return widget._fit_geometry_cache:get(context, width, height) + local w, h = widget._fit_geometry_cache:get(context, width, height) + + -- Also sanitize the output. + w = math.max(0, math.min(w, width)) + h = math.max(0, math.min(h, height)) + return w, h end --- Draw a widget via a cairo context diff --git a/lib/wibox/layout/flex.lua b/lib/wibox/layout/flex.lua index af41af65..021867cd 100644 --- a/lib/wibox/layout/flex.lua +++ b/lib/wibox/layout/flex.lua @@ -80,8 +80,8 @@ function flex:fit(context, orig_width, orig_height) local used_in_other = 0 -- Figure out the maximum size we can give out to sub-widgets - local sub_height = self.dir == "x" and orig_height or floor(orig_height / #self.widgets) - local sub_width = self.dir == "y" and orig_width or floor(orig_width / #self.widgets) + local sub_height = self.dir == "x" and orig_height or orig_height / #self.widgets + local sub_width = self.dir == "y" and orig_width or orig_width / #self.widgets for k, v in pairs(self.widgets) do local w, h = base.fit_widget(context, v, sub_width, sub_height) diff --git a/spec/wibox/layout/align_spec.lua b/spec/wibox/layout/align_spec.lua index df4ba35b..4ea7772d 100644 --- a/spec/wibox/layout/align_spec.lua +++ b/spec/wibox/layout/align_spec.lua @@ -56,8 +56,7 @@ describe("wibox.layout.flex", function() describe("without enough height", function() it("fit", function() - -- XXX: Is this really what should happen? - assert.widget_fit(layout, { 5, 100 }, { 15, 35 }) + assert.widget_fit(layout, { 5, 100 }, { 5, 35 }) end) it("draw", function() @@ -72,16 +71,14 @@ describe("wibox.layout.flex", function() describe("without enough width", function() it("fit", function() - -- XXX: Is this really what should happen? - assert.widget_fit(layout, { 100, 20 }, { 15, 35 }) + assert.widget_fit(layout, { 100, 20 }, { 15, 20 }) end) it("draw", function() layout:draw("wibox", "cr", 100, 20) - --- XXX: Shouldn't this also draw part of the second widget? utils.check_widgets_drawn({ - { first, 0, 0, 100, 10 }, - { third, 0, 10, 100, 10 }, + { first, 0, 0, 100, 2 }, + { third, 0, 18, 100, 2 }, { second, 0, 2, 100, 15 }, }) end) @@ -135,8 +132,7 @@ describe("wibox.layout.flex", function() describe("without enough height", function() it("fit", function() - -- XXX: Is this really what should happen? - assert.widget_fit(layout, { 5, 100 }, { 15, 35 }) + assert.widget_fit(layout, { 5, 100 }, { 5, 35 }) end) it("draw", function() @@ -151,8 +147,7 @@ describe("wibox.layout.flex", function() describe("without enough width", function() it("fit", function() - -- XXX: Is this really what should happen? - assert.widget_fit(layout, { 100, 20 }, { 15, 35 }) + assert.widget_fit(layout, { 100, 20 }, { 15, 20 }) end) it("draw", function() @@ -213,8 +208,7 @@ describe("wibox.layout.flex", function() describe("without enough height", function() it("fit", function() - -- XXX: Is this really what should happen? - assert.widget_fit(layout, { 5, 100 }, { 15, 35 }) + assert.widget_fit(layout, { 5, 100 }, { 5, 35 }) end) it("draw", function() @@ -229,8 +223,7 @@ describe("wibox.layout.flex", function() describe("without enough width", function() it("fit", function() - -- XXX: Is this really what should happen? - assert.widget_fit(layout, { 100, 20 }, { 15, 35 }) + assert.widget_fit(layout, { 100, 20 }, { 15, 20 }) end) it("draw", function() diff --git a/spec/wibox/layout/fixed_spec.lua b/spec/wibox/layout/fixed_spec.lua index 5475bd7d..744ffb8c 100644 --- a/spec/wibox/layout/fixed_spec.lua +++ b/spec/wibox/layout/fixed_spec.lua @@ -54,8 +54,7 @@ describe("wibox.layout.fixed", function() describe("without enough height", function() it("fit", function() - -- XXX: Is this really what should happen? - assert.widget_fit(layout, { 5, 100 }, { 15, 35 }) + assert.widget_fit(layout, { 5, 100 }, { 5, 35 }) end) it("draw", function() @@ -76,9 +75,10 @@ describe("wibox.layout.fixed", function() it("draw", function() layout:draw("wibox", "cr", 100, 20) - --- XXX: Shouldn't this also draw part of the second widget? utils.check_widgets_drawn({ { first, 0, 0, 100, 10 }, + { second, 0, 10, 100, 10 }, + { third, 0, 20, 100, 0 }, }) end) end) diff --git a/spec/wibox/layout/flex_spec.lua b/spec/wibox/layout/flex_spec.lua index 8a6a7e5a..a324861c 100644 --- a/spec/wibox/layout/flex_spec.lua +++ b/spec/wibox/layout/flex_spec.lua @@ -54,8 +54,7 @@ describe("wibox.layout.flex", function() describe("without enough height", function() it("fit", function() - -- XXX: Is this really what should happen? - assert.widget_fit(layout, { 5, 100 }, { 15, 35 }) + assert.widget_fit(layout, { 5, 100 }, { 5, 35 }) end) it("draw", function() @@ -70,8 +69,7 @@ describe("wibox.layout.flex", function() describe("without enough width", function() it("fit", function() - -- XXX: Is this really what should happen? - assert.widget_fit(layout, { 100, 20 }, { 15, 35 }) + assert.widget_fit(layout, { 100, 20 }, { 15, 20 }) end) it("draw", function() diff --git a/spec/wibox/test_utils.lua b/spec/wibox/test_utils.lua index d993baf7..f6f859a9 100644 --- a/spec/wibox/test_utils.lua +++ b/spec/wibox/test_utils.lua @@ -35,7 +35,7 @@ local function widget_fit(state, arguments) local widget = arguments[1] local given = arguments[2] local expected = arguments[3] - local w, h = widget:fit({ "fake context" }, given[1], given[2]) + local w, h = lbase.fit_widget({ "fake context" }, widget, given[1], given[2]) local fits = expected[1] == w and expected[2] == h if state.mod == fits then