Geom: nil guard a few rect methods (#7664)

We've managed to trip a few of those on dimen fields post-init but
pre-paintTo in a few weird coner-cases, a point at which dimen is often
nil.

ConfigDialog: Deal with that very thing in update()

Fix #7656

frontend/apps/filemanager/filemanager.lua

@@ -539,6 +539,15 @@ function FileManager:init()
 
     self:initGesListener()
     self:handleEvent(Event:new("SetDimensions", self.dimen))
+
+    -- NOTE: ReaderUI has a _getRunningInstance method for this, because it used to store the instance reference in a private module variable.
+    if FileManager.instance == nil then
+        logger.dbg("Spinning up new FileManager instance", tostring(self))
+    else
+        -- Should never happen, given what we did above...
+        logger.err("FileManager instance mismatch! Opened", tostring(self), "while we still have an existing instance:", tostring(FileManager.instance), debug.traceback())
+    end
+    FileManager.instance = self
 end
 
 function FileChooser:onBack()
@@ -1168,15 +1177,6 @@ function FileManager:showFiles(path, focused_file)
         focused_file = focused_file,
     }
     UIManager:show(file_manager)
-
-    -- NOTE: ReaderUI has a _getRunningInstance method for this, because it used to store the instance reference in a private module variable.
-    if FileManager.instance == nil then
-        logger.dbg("Spinning up new FileManager instance", tostring(file_manager))
-    else
-        -- Should never happen, given what we did above...
-        logger.warn("FileManager instance mismatch! Opened", tostring(file_manager), "while we still have an existing instance:", tostring(FileManager.instance))
-    end
-    FileManager.instance = file_manager
 end
 
 --- A shortcut to execute mv.

frontend/apps/reader/readerui.lua

@@ -439,6 +439,14 @@ function ReaderUI:init()
     -- for _, tzone in ipairs(self._ordered_touch_zones) do
     --     print("  "..tzone.def.id)
     -- end
+
+    if ReaderUI.instance == nil then
+        logger.dbg("Spinning up new ReaderUI instance", tostring(self))
+    else
+        -- Should never happen, given what we did above...
+        logger.err("ReaderUI instance mismatch! Opened", tostring(self), "while we still have an existing instance:", tostring(ReaderUI.instance), debug.traceback())
+    end
+    ReaderUI.instance = self
 end
 
 function ReaderUI:setLastDirForFileBrowser(dir)
@@ -621,14 +629,6 @@ function ReaderUI:doShowReader(file, provider)
     end
 
     UIManager:show(reader, "full")
-
-    if ReaderUI.instance == nil then
-        logger.dbg("Spinning up new ReaderUI instance", tostring(reader))
-    else
-        -- Should never happen, given what we did above...
-        logger.warn("ReaderUI instance mismatch! Opened", tostring(reader), "while we still have an existing instance:", tostring(ReaderUI.instance))
-    end
-    ReaderUI.instance = reader
 end
 
 -- NOTE: The instance reference used to be stored in a private module variable, hence the getter method.

frontend/device/generic/device.lua

@@ -5,6 +5,7 @@ This module defines stubs for common methods.
 --]]
 
 local DataStorage = require("datastorage")
+local Geom = require("ui/geometry")
 local logger = require("logger")
 local util = require("util")
 local _ = require("gettext")
@@ -182,7 +183,7 @@ function Device:init()
     end
 
     logger.info("initializing for device", self.model)
-    logger.info("framebuffer resolution:", self.screen:getSize())
+    logger.info("framebuffer resolution:", self.screen:getRawSize())
 
     if not self.input then
         self.input = require("device/input"):new{device = self}
@@ -212,6 +213,13 @@ function Device:init()
             self:lockGSensor(true)
         end
     end
+
+    -- Screen:getSize is used throughout the code, and that code usually expects getting a real Geom object...
+    -- But as implementations come from base, they just return a Geom-like table...
+    self.screen.getSize = function()
+        local rect = self.screen.getRawSize(self.screen)
+        return Geom:new{ x = rect.x, y = rect.y, w = rect.w, h = rect.h }
+    end
 end
 
 function Device:setScreenDPI(dpi_override)

frontend/ui/geometry.lua

@@ -142,8 +142,10 @@ Works for rectangles, dimensions and points
 @treturn Geom
 ]]
 function Geom:combine(rect_b)
+    -- We'll want to return a *new* object, so, start with a copy of self
     local combined = self:copy()
     if not rect_b or rect_b:area() == 0 then return combined end
+
     if combined.x > rect_b.x then
         combined.x = rect_b.x
     end
@@ -171,8 +173,9 @@ Returns a new rectangle for the part that we and a given rectangle share
 ]]--
 --- @todo what happens if there is no rectangle shared? currently behaviour is undefined.
 function Geom:intersect(rect_b)
-    -- make a copy of self
     local intersected = self:copy()
+    if not rect_b or rect_b:area() == 0 then return intersected end
+
     if self.x < rect_b.x then
         intersected.x = rect_b.x
     end
@@ -198,6 +201,8 @@ Returns true if self does not share any area with rect_b
 @tparam Geom rect_b
 ]]
 function Geom:notIntersectWith(rect_b)
+    if not rect_b or rect_b:area() == 0 then return true end
+
     if (self.x >= (rect_b.x + rect_b.w))
     or (self.y >= (rect_b.y + rect_b.h))
     or (rect_b.x >= (self.x + self.w))
@@ -228,17 +233,19 @@ function Geom:setSizeTo(rect_b)
 end
 
 --[[--
-Checks whether rect_b is within current rectangle
+Checks whether geom is within current rectangle
 
 Works for dimensions, too. For points, it is basically an equality check.
 
-@tparam Geom rect_b
+@tparam Geom geom
 ]]
-function Geom:contains(rect_b)
+function Geom:contains(geom)
-    if self.x <= rect_b.x
+    if not geom then return false end
-    and self.y <= rect_b.y
+
-    and self.x + self.w >= rect_b.x + rect_b.w
+    if self.x <= geom.x
-    and self.y + self.h >= rect_b.y + rect_b.h
+    and self.y <= geom.y
+    and self.x + self.w >= geom.x + geom.w
+    and self.y + self.h >= geom.y + geom.h
     then
         return true
     end

frontend/ui/widget/configdialog.lua

@@ -880,6 +880,7 @@ function ConfigDialog:update()
         config_dialog = self,
     }
 
+    local old_dimen = self.dialog_frame and self.dialog_frame.dimen and self.dialog_frame.dimen:copy() or Geom:new{}
     self.dialog_frame = FrameContainer:new{
         background = Blitbuffer.COLOR_WHITE,
         padding_bottom = 0, -- ensured by MenuBar
@@ -888,6 +889,10 @@ function ConfigDialog:update()
             self.config_menubar,
         },
     }
+    -- Ensure we have a sane-ish Geom object *before* paintTo gets called,
+    -- to avoid weirdness in race-y SwipeCloseMenu calls...
+    self.dialog_frame.dimen = old_dimen
+
     -- Reset the focusmanager cursor
     self.selected.y=#self.layout
     self.selected.x=self.panel_index

spec/unit/device_spec.lua

@@ -9,7 +9,7 @@ describe("device module", function()
         mock_fb = {
             new = function()
                 return {
-                    getSize = function() return {w = 600, h = 800} end,
+                    getRawSize = function() return {w = 600, h = 800} end,
                     getWidth = function() return 600 end,
                     getDPI = function() return 72 end,
                     setViewport = function() end,