Assorted fixes after #7118 (#7161)

* I'd failed to notice that ButtonTable *also* instantiates seven billion Buttons on each update. Unfortunately, that one is way trickier to fix properly, so, work around its behavior in Button. (This fixes multiple issues with stuff using ButtonTable, which is basically anything with a persistent set of buttons. A good and easy test-case is the dictionary popup, e.g., the Highlight button changes text, and the next/prev dic buttons change state. All that, and more, was broken ;p).

* Handle corner-cases related to VirtualKeyboard (e.g., Terminal & Text Editor), which screwed with both TouchMenu & Button heuristics because it's weird.

* Flag a the dictionary switch buttons as vsync

(They trigger a partial repaint of the dictionary content).

* Flag the ReaderSearch buttons as vsync

They very obviously trigger a partial repaint, much like SkimTo ;p.

frontend/apps/reader/modules/readersearch.lua

@@ -47,6 +47,7 @@ function ReaderSearch:onShowFulltextSearchInput()
                 },
                 {
                     text = backward_text,
+                    vsync = true,
                     callback = function()
                         self:onShowSearchDialog(self.input_dialog:getInputText(), 1)
                         self:closeInputDialog()
@@ -54,6 +55,7 @@ function ReaderSearch:onShowFulltextSearchInput()
                 },
                 {
                     text = forward_text,
+                    vsync = true,
                     is_enter_default = true,
                     callback = function()
                         self:onShowSearchDialog(self.input_dialog:getInputText(), 0)
@@ -159,18 +161,22 @@ function ReaderSearch:onShowSearchDialog(text, direction)
             {
                 {
                     text = from_start_text,
+                    vsync = true,
                     callback = do_search(self.searchFromStart, text),
                 },
                 {
                     text = backward_text,
+                    vsync = true,
                     callback = do_search(self.searchNext, text, 1),
                 },
                 {
                     text = forward_text,
+                    vsync = true,
                     callback = do_search(self.searchNext, text, 0),
                 },
                 {
                     text = from_end_text,
+                    vsync = true,
                     callback = do_search(self.searchFromEnd, text),
                 },
             }

frontend/ui/widget/button.lua

@@ -269,15 +269,12 @@ function Button:onTapSelectButton()
             end
 
             if not self[1] or not self[1].invert or not self[1].dimen then
-                -- If the widget no longer exists (destroyed, re-init'ed by setText(), or not inverted: nothing to invert back
-                return true
-            end
-
-            -- If the callback closed our parent (which ought to have been the top level widget), abort early
-            if UIManager:getTopWidget() ~= self.show_parent then
+                -- If the frame widget no longer exists (destroyed, re-init'ed by setText(), or is no longer inverted: we have nothing to invert back
+                -- NOTE: This cannot catch orphaned Button instances, c.f., the isSubwidgetShown(self) check below for that.
                 return true
             end
 
+            -- Reset colors early, regardless of what we do later, to avoid code duplication
             self[1].invert = false
             if self.text then
                 if self[1].radius == Size.radius.button then
@@ -285,16 +282,47 @@ function Button:onTapSelectButton()
                     self[1].background = self[1].background:invert()
                     self.label_widget.fgcolor = self.label_widget.fgcolor:invert()
                 end
-
-                UIManager:widgetRepaint(self[1], self[1].dimen.x, self[1].dimen.y)
-            else
-                UIManager:widgetInvert(self[1], self[1].dimen.x, self[1].dimen.y)
             end
-            -- If the button was disabled, switch to UI to make sure the gray comes through unharmed ;).
-            UIManager:setDirty(nil, function()
-                return self.enabled and "fast" or "ui", self[1].dimen
-            end)
-            --UIManager:forceRePaint() -- Ensures the unhighlight happens now, instead of potentially waiting and having it batched with something else.
+
+            -- If the callback closed our parent (which may not always be the top-level widget, or even *a* window-level widget), we're done
+            local top_widget = UIManager:getTopWidget()
+            if top_widget == self.show_parent or UIManager:isSubwidgetShown(self.show_parent) then
+                -- If the button can no longer be found inside a shown widget, abort early
+                -- (this allows us to catch widgets that instanciate *new* Buttons on every update... (e.g., ButtonTable) :()
+                if not UIManager:isSubwidgetShown(self) then
+                    return true
+                end
+
+                -- If our parent is no longer the toplevel widget, toplevel is now a true modal, and our highlight would clash with that modal's region,
+                -- we have no other choice than repainting the full stack...
+                if top_widget ~= self.show_parent and top_widget ~= "VirtualKeyboard" and top_widget.modal and self[1].dimen:intersectWith(UIManager:getPreviousRefreshRegion()) then
+                    -- Much like in TouchMenu, the fact that the two intersect means we have no choice but to repaint the full stack to avoid half-painted widgets...
+                    UIManager:waitForVSync()
+                    UIManager:setDirty(self.show_parent, function()
+                        return "ui", self[1].dimen
+                    end)
+
+                    -- It's a sane exit, handle the return the same way.
+                    if self.readonly ~= true then
+                        return true
+                    end
+                end
+
+                if self.text then
+                    UIManager:widgetRepaint(self[1], self[1].dimen.x, self[1].dimen.y)
+                else
+                    UIManager:widgetInvert(self[1], self[1].dimen.x, self[1].dimen.y)
+                end
+                -- If the button was disabled, switch to UI to make sure the gray comes through unharmed ;).
+                UIManager:setDirty(nil, function()
+                    return self.enabled and "fast" or "ui", self[1].dimen
+                end)
+                --UIManager:forceRePaint() -- Ensures the unhighlight happens now, instead of potentially waiting and having it batched with something else.
+            else
+                -- This branch will mainly be taken by stuff that pops up the virtual keyboard (e.g., TextEditor), where said keyboard will always be top-level,
+                -- (hence the exception in the check above).
+                return true
+            end
         end
     elseif self.tap_input then
         self:onInput(self.tap_input)

frontend/ui/widget/buttontable.lua

@@ -55,6 +55,7 @@ function ButtonTable:init()
                 enabled = btn_entry.enabled,
                 callback = btn_entry.callback,
                 hold_callback = btn_entry.hold_callback,
+                vsync = btn_entry.vsync,
                 width = (self.width - sizer_space)/column_cnt,
                 max_width = (self.width - sizer_space)/column_cnt - 2*self.sep_width - 2*self.padding,
                 bordersize = 0,

frontend/ui/widget/dictquicklookup.lua

@@ -460,6 +460,7 @@ function DictQuickLookup:update()
             {
                 {
                     text = prev_dict_text,
+                    vsync = true,
                     enabled = self:isPrevDictAvaiable(),
                     callback = function()
                         self:changeToPrevDict()
@@ -482,6 +483,7 @@ function DictQuickLookup:update()
                 },
                 {
                     text = next_dict_text,
+                    vsync = true,
                     enabled = self:isNextDictAvaiable(),
                     callback = function()
                         self:changeToNextDict()

frontend/ui/widget/inputdialog.lua

@@ -725,6 +725,7 @@ function InputDialog:_addScrollButtons(nav_bar)
         table.insert(row, {
             text = "⇱",
             id = "top",
+            vsync = true,
             callback = function()
                 self._input_widget:scrollToTop()
             end,
@@ -732,6 +733,7 @@ function InputDialog:_addScrollButtons(nav_bar)
         table.insert(row, {
             text = "⇲",
             id = "bottom",
+            vsync = true,
             callback = function()
                 self._input_widget:scrollToBottom()
             end,

frontend/ui/widget/touchmenu.lua

@@ -186,6 +186,12 @@ function TouchMenuItem:onTapSelect(arg, ges)
                 return true
             end
 
+            -- If the callback opened the Virtual Keyboard, we're done
+            -- (this is for TextEditor, Terminal & co)
+            if top_widget == "VirtualKeyboard" then
+                return true
+            end
+
             -- If we're still on top, or if a modal was opened outside of our highlight region, we can unhighlight safely
             if top_widget == self.menu or highlight_dimen:notIntersectWith(UIManager:getPreviousRefreshRegion()) then
                 UIManager:widgetInvert(self.item_frame, highlight_dimen.x, highlight_dimen.y, highlight_dimen.w)
@@ -234,10 +240,27 @@ function TouchMenuItem:onHoldSelect(arg, ges)
         --UIManager:waitForVSync()
 
         self.item_frame.invert = false
-        UIManager:widgetInvert(self.item_frame, highlight_dimen.x, highlight_dimen.y, highlight_dimen.w)
-        UIManager:setDirty(nil, function()
-            return "ui", highlight_dimen
-        end)
+        -- If the callback closed the menu, we're done. (This field defaults to nil, meaning keep the menu open)
+        if self.item.hold_keep_menu_open == false then
+            return true
+        end
+
+        local top_widget = UIManager:getTopWidget()
+        -- If we're still on top, or if a modal was opened outside of our highlight region, we can unhighlight safely
+        if top_widget == self.menu or highlight_dimen:notIntersectWith(UIManager:getPreviousRefreshRegion()) then
+            UIManager:widgetInvert(self.item_frame, highlight_dimen.x, highlight_dimen.y, highlight_dimen.w)
+            UIManager:setDirty(nil, function()
+                return "ui", highlight_dimen
+            end)
+        else
+            -- That leaves modals that might have been displayed on top of the highlighted menu entry, in which case,
+            -- we can't take any shortcuts, as it would invert/paint *over* the popop.
+            -- Instead, fence the callback to avoid races, and repaint the *full* widget stack properly.
+            UIManager:waitForVSync()
+            UIManager:setDirty(self.show_parent, function()
+                return "ui", highlight_dimen
+            end)
+        end
         --UIManager:forceRePaint()
     end
     return true

frontend/ui/widget/virtualkeyboard.lua

@@ -639,6 +639,7 @@ function VirtualKeyPopup:init()
 end
 
 local VirtualKeyboard = FocusManager:new{
+    name = "VirtualKeyboard",
     modal = true,
     disable_double_tap = true,
     inputbox = nil,