Merge pull request #458 from psychon/explicit_widget_deps

Explicitly track dependencies between widgets

Closes https://github.com/awesomeWM/awesome/pull/458.

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

@@ -18,41 +18,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, ...)
@@ -61,24 +32,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
 -- }}}
 
@@ -91,12 +74,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
@@ -107,12 +93,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)
@@ -130,12 +114,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
@@ -145,10 +132,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
 
@@ -382,7 +366,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