From bcc1751fca11736fcf9b7b183ecaba8f9596ad71 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Thu, 13 Aug 2015 15:48:56 +0200 Subject: [PATCH] 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 3e9a2832a..a92052769 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 af41af653..021867cd1 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 df4ba35b4..4ea7772d6 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 5475bd7dd..744ffb8c1 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 8a6a7e5a6..a324861c5 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 d993baf75..f6f859a98 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