From 0c76c73e4f0409f7843d927ebf2079f92156a697 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Mon, 18 Jan 2021 16:51:25 +0100 Subject: [PATCH] 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 | 6 +++ frontend/ui/widget/button.lua | 54 ++++++++++++++----- frontend/ui/widget/buttontable.lua | 1 + frontend/ui/widget/dictquicklookup.lua | 2 + frontend/ui/widget/inputdialog.lua | 2 + frontend/ui/widget/touchmenu.lua | 31 +++++++++-- frontend/ui/widget/virtualkeyboard.lua | 1 + 7 files changed, 80 insertions(+), 17 deletions(-) diff --git a/frontend/apps/reader/modules/readersearch.lua b/frontend/apps/reader/modules/readersearch.lua index dcb640b2a..43f69147a 100644 --- a/frontend/apps/reader/modules/readersearch.lua +++ b/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), }, } diff --git a/frontend/ui/widget/button.lua b/frontend/ui/widget/button.lua index f9336caf3..a4f2affea 100644 --- a/frontend/ui/widget/button.lua +++ b/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 + end + + -- 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 - UIManager:widgetRepaint(self[1], self[1].dimen.x, self[1].dimen.y) + -- 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 - UIManager:widgetInvert(self[1], self[1].dimen.x, self[1].dimen.y) + -- 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 - -- 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. end elseif self.tap_input then self:onInput(self.tap_input) diff --git a/frontend/ui/widget/buttontable.lua b/frontend/ui/widget/buttontable.lua index 1ca9f6aad..596d9822b 100644 --- a/frontend/ui/widget/buttontable.lua +++ b/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, diff --git a/frontend/ui/widget/dictquicklookup.lua b/frontend/ui/widget/dictquicklookup.lua index e9b31b3a4..ef0864f62 100644 --- a/frontend/ui/widget/dictquicklookup.lua +++ b/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() diff --git a/frontend/ui/widget/inputdialog.lua b/frontend/ui/widget/inputdialog.lua index ebe41a427..bdb6ff403 100644 --- a/frontend/ui/widget/inputdialog.lua +++ b/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, diff --git a/frontend/ui/widget/touchmenu.lua b/frontend/ui/widget/touchmenu.lua index 79f1a8283..6503f8ddb 100644 --- a/frontend/ui/widget/touchmenu.lua +++ b/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 diff --git a/frontend/ui/widget/virtualkeyboard.lua b/frontend/ui/widget/virtualkeyboard.lua index c9fe59397..df95c4f99 100644 --- a/frontend/ui/widget/virtualkeyboard.lua +++ b/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,