Explicitly track dependencies between widgets

Before this, dependencies between widgets where implicitly discovered by
recursive calls to base.fit_widget() and base.layout_widget(). However, it is
too easy to get this wrong (just call one of these functions from outside of a
widget's :fit() / :layout() function) and the resulting mess would be hard to
debug.

Thus, this commit changes the API so that callers have to identify themselves
and we can explicitly record the dependency between the widgets involved.

This also fixes a bug where no dependencies were tracked for widgets after
:set_visible(false). Whoops...

Sorry for breaking the API for adding this.

Signed-off-by: Uli Schlachter <psychon@znc.in>

lib/wibox/hierarchy.lua

@@ -12,12 +12,13 @@
 local matrix = require("gears.matrix")
 local cairo = require("lgi").cairo
 local base = require("wibox.widget.base")
+local no_parent = base.no_parent_I_know_what_I_am_doing
 
 local hierarchy = {}
 
 local function hierarchy_new(context, widget, width, height, redraw_callback, layout_callback, callback_arg,
             matrix_to_parent, matrix_to_device)
-    local children = base.layout_widget(context, widget, width, height)
+    local children = base.layout_widget(no_parent, context, widget, width, height)
     local draws_x1, draws_y1, draws_x2, draws_y2 = 0, 0, width, height
     local result = {
         _matrix = matrix_to_parent,

lib/wibox/layout/align.lua

@@ -40,7 +40,7 @@ function align:layout(context, width, height)
     --  if the second widget doesn't exist, we will prioritise the first one
     --  instead
     if self._expand ~= "inside" and self.second then
-        local w, h = base.fit_widget(context, self.second, width, height)
+        local w, h = base.fit_widget(self, context, self.second, width, height)
         size_second = self.dir == "y" and h or w
         -- if all the space is taken, skip the rest, and draw just the middle
         -- widget
@@ -59,7 +59,7 @@ function align:layout(context, width, height)
         --  into the remaining space
         if self._expand ~= "outside" then
             if self.dir == "y" then
-                _, h = base.fit_widget(context, self.first, width, size_remains)
+                _, h = base.fit_widget(self, context, self.first, width, size_remains)
                 size_first = h
                 -- for "inside", the third widget will get a chance to use the
                 --  remaining space, then the middle widget. For "none" we give
@@ -70,7 +70,7 @@ function align:layout(context, width, height)
                     size_remains = size_remains - h
                 end
             else
-                w, _ = base.fit_widget(context, self.first, size_remains, height)
+                w, _ = base.fit_widget(self, context, self.first, size_remains, height)
                 size_first = w
                 if self._expand == "inside" or not self.second then
                     size_remains = size_remains - w
@@ -90,13 +90,13 @@ function align:layout(context, width, height)
         local w, h, _ = width, height, nil
         if self._expand ~= "outside" then
             if self.dir == "y" then
-                _, h = base.fit_widget(context, self.third, width, size_remains)
+                _, h = base.fit_widget(self, context, self.third, width, size_remains)
                 -- give the middle widget the rest of the space for "inside" mode
                 if self._expand == "inside" then
                     size_remains = size_remains - h
                 end
             else
-                w, _ = base.fit_widget(context, self.third, size_remains, height)
+                w, _ = base.fit_widget(self, context, self.third, size_remains, height)
                 if self._expand == "inside" then
                     size_remains = size_remains - w
                 end
@@ -125,10 +125,10 @@ function align:layout(context, width, height)
             end
         else
             if self.dir == "y" then
-                _, h = base.fit_widget(context, self.second, width, size_second)
+                _, h = base.fit_widget(self, context, self.second, width, size_second)
                 y = floor( (height - h)/2 )
             else
-                w, _ = base.fit_widget(context, self.second, size_second, height)
+                w, _ = base.fit_widget(self, context, self.second, size_second, height)
                 x = floor( (width -w)/2 )
             end
         end
@@ -175,7 +175,7 @@ function align:fit(context, orig_width, orig_height)
     local used_in_other = 0
 
     for k, v in pairs{self.first, self.second, self.third} do
-        local w, h = base.fit_widget(context, v, orig_width, orig_height)
+        local w, h = base.fit_widget(self, context, v, orig_width, orig_height)
 
         local max = self.dir == "y" and w or h
         if max > used_in_other then

lib/wibox/layout/constraint.lua

@@ -27,7 +27,7 @@ function constraint:fit(context, width, height)
         w = self._strategy(width, self._width)
         h = self._strategy(height, self._height)
 
-        w, h = base.fit_widget(context, self.widget, w, h)
+        w, h = base.fit_widget(self, context, self.widget, w, h)
     else
         w, h = 0, 0
     end

lib/wibox/layout/fixed.lua

@@ -26,7 +26,7 @@ function fixed:layout(context, width, height)
             x, y = 0, pos
             w, h = width, height - pos
             if k ~= #self.widgets or not self._fill_space then
-                _, h = base.fit_widget(context, v, w, h);
+                _, h = base.fit_widget(self, context, v, w, h);
             end
             pos = pos + h + spacing
             in_dir = h
@@ -34,7 +34,7 @@ function fixed:layout(context, width, height)
             x, y = pos, 0
             w, h = width - pos, height
             if k ~= #self.widgets or not self._fill_space then
-                w, _ = base.fit_widget(context, v, w, h);
+                w, _ = base.fit_widget(self, context, v, w, h);
             end
             pos = pos + w + spacing
             in_dir = w
@@ -65,7 +65,7 @@ function fixed:fit(context, orig_width, orig_height)
     local used_in_dir, used_max = 0, 0
 
     for k, v in pairs(self.widgets) do
-        local w, h = base.fit_widget(context, v, width, height)
+        local w, h = base.fit_widget(self, context, v, width, height)
         local in_dir, max
         if self.dir == "y" then
             max, in_dir = w, h

lib/wibox/layout/flex.lua

@@ -73,7 +73,7 @@ function flex:fit(context, orig_width, orig_height)
     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)
+        local w, h = base.fit_widget(self, context, v, sub_width, sub_height)
 
         local max = self.dir == "y" and w or h
         if max > used_in_other then

lib/wibox/layout/margin.lua

@@ -53,7 +53,7 @@ function margin:fit(context, width, height)
     local extra_h = self.top + self.bottom
     local w, h = 0, 0
     if self.widget then
-        w, h = base.fit_widget(context, self.widget, width - extra_w, height - extra_h)
+        w, h = base.fit_widget(self, context, self.widget, width - extra_w, height - extra_h)
     end
     return w + extra_w, h + extra_h
 end

lib/wibox/layout/mirror.lua

@@ -45,7 +45,7 @@ function mirror:fit(context, ...)
     if not self.widget then
         return 0, 0
     end
-    return base.fit_widget(context, self.widget, ...)
+    return base.fit_widget(self, context, self.widget, ...)
 end
 
 --- Set the widget that this layout mirrors.

lib/wibox/layout/rotate.lua

@@ -54,7 +54,7 @@ function rotate:fit(context, width, height)
     if not self.widget then
         return 0, 0
     end
-    return transform(self, base.fit_widget(context, self.widget, transform(self, width, height)))
+    return transform(self, base.fit_widget(self, context, self.widget, transform(self, width, height)))
 end
 
 --- Set the widget that this layout rotates.

lib/wibox/widget/background.lua

@@ -52,7 +52,7 @@ function background:fit(context, width, height)
         return 0, 0
     end
 
-    return base.fit_widget(context, self.widget, width, height)
+    return base.fit_widget(self, context, self.widget, width, height)
 end
 
 --- Set the widget that is drawn on top of the background

lib/wibox/widget/base.lua

@@ -19,41 +19,12 @@ local base = {}
 
 -- {{{ Caches
 
-local call_stack = {}
 -- Indexes are widgets, allow them to be garbage-collected
 local widget_dependencies = setmetatable({}, { __mode = "k" })
 
--- Don't do this in unit tests
+-- Get the cache of the given kind for this widget. This returns a gears.cache
-if awesome and awesome.connect_signal then
-    -- Reset the call stack at each refresh. This fixes things up in case there was
-    -- an error in some callback and thus put_cache() wasn't called (if this
-    -- happens, we possibly recorded too many deps, but so what?)
-    awesome.connect_signal("refresh", function()
-        call_stack = {}
-    end)
-end
-
--- When you call get_cache_and_record_deps(), the widget is recorded in a stack
--- until the following put_cache(). All other calls to
--- get_cache_and_record_deps() that happen during this will cause a dependency
--- between the widgets that are involved to be recorded. This information is
--- used by clear_caches() to also clear all caches of dependent widgets.
-
--- Get the caches for a widget and record its dependencies. All following
--- cache-uses will record this widgets as a dependency. This returns a function
 -- that calls the callback of kind `kind` on the widget.
-local function get_cache_and_record_deps(widget, kind)
+local function get_cache(widget, kind)
-    -- Record dependencies (each entry in the call stack depends on `widget`)
-    local deps = widget_dependencies[widget] or {}
-    for _, w in pairs(call_stack) do
-        deps[w] = true
-    end
-    widget_dependencies[widget] = deps
-
-    -- Add widget to call stack
-    table.insert(call_stack, widget)
-
-    -- Create cache if needed
     if not widget._widget_caches[kind] then
         widget._widget_caches[kind] = cache.new(function(...)
             return widget[kind](widget, ...)
@@ -62,24 +33,36 @@ local function get_cache_and_record_deps(widget, kind)
     return widget._widget_caches[kind]
 end
 
--- Each call to the above function should be followed by a call to this
+-- Special value to skip the dependency recording that is normally done by
--- function. Everything in-between is recorded as a dependency (it's
+-- base.fit_widget() and base.layout_widget(). The caller must ensure that no
--- complicated...).
+-- caches depend on the result of the call and/or must handle the childs
-local function put_cache(widget)
+-- widget::layout_changed signal correctly when using this.
-    assert(#call_stack ~= 0)
+base.no_parent_I_know_what_I_am_doing = {}
-    if table.remove(call_stack) ~= widget then
+
-        put_cache(widget)
+-- Record a dependency from parent to child: The layout of parent depends on the
+-- layout of child.
+local function record_dependency(parent, child)
+    if parent == base.no_parent_I_know_what_I_am_doing then
+        return
     end
+
+    base.check_widget(parent)
+    base.check_widget(child)
+
+    local deps = widget_dependencies[child] or {}
+    deps[parent] = true
+    widget_dependencies[child] = deps
 end
 
 -- Clear the caches for `widget` and all widgets that depend on it.
-local function clear_caches(widget)
+local clear_caches
-    for w in pairs(widget_dependencies[widget] or {}) do
+function clear_caches(widget)
-        widget_dependencies[w] = {}
+    local deps = widget_dependencies[widget] or {}
-        w._widget_caches = {}
-    end
     widget_dependencies[widget] = {}
     widget._widget_caches = {}
+    for w in pairs(deps) do
+        clear_caches(w)
+    end
 end
 -- }}}
 
@@ -92,12 +75,15 @@ end
 --- Fit a widget for the given available width and height. This calls the
 -- widget's `:fit` callback and caches the result for later use. Never call
 -- `:fit` directly, but always through this function!
+-- @param parent The parent widget which requests this information.
 -- @param context The context in which we are fit.
 -- @param widget The widget to fit (this uses widget:fit(width, height)).
 -- @param width The available width for the widget
 -- @param height The available height for the widget
 -- @return The width and height that the widget wants to use
-function base.fit_widget(context, widget, width, height)
+function base.fit_widget(parent, context, widget, width, height)
+    record_dependency(parent, widget)
+
     if not widget.visible then
         return 0, 0
     end
@@ -108,12 +94,10 @@ function base.fit_widget(context, widget, width, height)
 
     local w, h = 0, 0
     if widget.fit then
-        local cache = get_cache_and_record_deps(widget, "fit")
+        w, h = get_cache(widget, "fit"):get(context, width, height)
-        w, h = cache:get(context, width, height)
-        put_cache(widget)
     else
         -- If it has no fit method, calculate based on the size of children
-        local children = base.layout_widget(context, widget, width, height)
+        local children = base.layout_widget(parent, context, widget, width, height)
         for _, info in ipairs(children or {}) do
             local x, y, w2, h2 = matrix.transform_rectangle(info._matrix,
                 0, 0, info._width, info._height)
@@ -131,12 +115,15 @@ end
 -- widget's `:layout` callback and caches the result for later use. Never call
 -- `:layout` directly, but always through this function! However, normally there
 -- shouldn't be any reason why you need to use this function.
+-- @param parent The parent widget which requests this information.
 -- @param context The context in which we are laid out.
 -- @param widget The widget to layout (this uses widget:layout(context, width, height)).
 -- @param width The available width for the widget
 -- @param height The available height for the widget
 -- @return The result from the widget's `:layout` callback.
-function base.layout_widget(context, widget, width, height)
+function base.layout_widget(parent, context, widget, width, height)
+    record_dependency(parent, widget)
+
     if not widget.visible then
         return
     end
@@ -146,10 +133,7 @@ function base.layout_widget(context, widget, width, height)
     local height = math.max(0, height)
 
     if widget.layout then
-        local cache = get_cache_and_record_deps(widget, "layout")
+        return get_cache(widget, "layout"):get(context, width, height)
-        local result = cache:get(context, width, height)
-        put_cache(widget)
-        return result
     end
 end
 
@@ -383,7 +367,7 @@ function base.make_widget(proxy, widget_name)
 
     if proxy then
         ret.fit = function(_, context, width, height)
-            return base.fit_widget(context, proxy, width, height)
+            return base.fit_widget(ret, context, proxy, width, height)
         end
         ret.layout = function(_, context, width, height)
             return { base.place_widget_at(proxy, 0, 0, width, height) }

spec/wibox/test_utils.lua

@@ -9,6 +9,7 @@ local matrix_equals = require("gears.matrix").equals
 local base = require("wibox.widget.base")
 local say = require("say")
 local assert = require("luassert")
+local no_parent = base.no_parent_I_know_what_I_am_doing
 
 -- {{{ Own widget-based assertions
 local function widget_fit(state, arguments)
@@ -19,7 +20,7 @@ local function widget_fit(state, arguments)
     local widget = arguments[1]
     local given = arguments[2]
     local expected = arguments[3]
-    local w, h = base.fit_widget({ "fake context" }, widget, given[1], given[2])
+    local w, h = base.fit_widget(no_parent, { "fake context" }, widget, given[1], given[2])
 
     local fits = expected[1] == w and expected[2] == h
     if state.mod == fits then