From 82f0e681186d0107a17ef9bda636b959916e4851 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Sun, 7 Feb 2021 18:29:32 +0100 Subject: [PATCH] Button: Fix some more weird highlighting corner-cases (#7256) Namely, the Terminal plug-in and its chain of InputDialog on top of each other (via the "Save" button, which opens a whole new fs InputDialog to set the filename, while keeping the file's fs InputDialog open). --- frontend/ui/uimanager.lua | 54 +++++++++++++++++++++++++++---- frontend/ui/widget/button.lua | 36 +++++++++++++-------- frontend/ui/widget/iconbutton.lua | 6 +++- frontend/ui/widget/menu.lua | 2 +- frontend/ui/widget/touchmenu.lua | 2 +- 5 files changed, 78 insertions(+), 22 deletions(-) diff --git a/frontend/ui/uimanager.lua b/frontend/ui/uimanager.lua index 0cac5ce54..d6e07dbe9 100644 --- a/frontend/ui/uimanager.lua +++ b/frontend/ui/uimanager.lua @@ -390,7 +390,7 @@ function UIManager:show(widget, refreshtype, refreshregion, x, y, refreshdither) self._running = true local window = {x = x or 0, y = y or 0, widget = widget} - -- put this window on top of the toppest non-modal window + -- put this window on top of the topmost non-modal window for i = #self._window_stack, 0, -1 do local top_window = self._window_stack[i] -- toasts are stacked on top of other toasts, @@ -595,9 +595,10 @@ dbg:guard(UIManager, 'unschedule', Registers a widget to be repainted and enqueues a refresh. the second parameter (refreshtype) can either specify a refreshtype -(optionally in combination with a refreshregion - which is suggested) -or a function that returns refreshtype AND refreshregion and is called -*after* painting the widget. +(optionally in combination with a refreshregion - which is suggested, +and an even more optional refreshdither flag if the content requires dithering) +or a function that returns a refreshtype, refreshregion tuple (or a refreshtype, refreshregion, refreshdither triple) +and is called *after* painting the widget. This is an interesting distinction, because a widget's geometry, usually stored in a field named `dimen`, in only computed at painting time (e.g., during `paintTo`). The TL;DR being: if you already know the region, you can pass everything by value directly, @@ -678,6 +679,16 @@ Another convention (that a few things rely on) is naming a (persistent) MovableC This is useful when it's used for transparency purposes, which, e.g., Button relies on to handle highlighting inside a transparent widget properly, by checking if self.show_parent.movable exists and is currently translucent ;). +When I mentioned passing the *right* widget to `setDirty` earlier, what I meant is that setDirty will only actually flag a widget for repaint +*if* that widget is a window-level widget (that is, a widget that was passed to `show` earlier and hasn't been `close`'d yet), +hence the self.show_parent convention detailed above to get at the proper widget from within a subwidget ;). +Otherwise, you'll notice in debug mode that a debug guard will shout at you if that contract is broken, +and what happens in practice is the same thing as if an explicit `nil` were passed: no widgets will actually be flagged for repaint, +and only the *refresh* matching the requested region *will* be enqueued. +This is why you'll find a number of valid use-cases for passing a nil here, when you *just* want a screen refresh without a repaint :). +The string "all" is also accepted in place of a widget, and will do the obvious thing: flag the *full* window stack, bottom to top, for repaint, +while still honoring the refresh region (e.g., this doesn't enforce a full-screen refresh). + @usage UIManager:setDirty(self.widget, "partial") @@ -685,9 +696,9 @@ UIManager:setDirty(self.widget, "partial", Geom:new{x=10,y=10,w=100,h=50}) UIManager:setDirty(self.widget, function() return "ui", self.someelement.dimen end) --]] ----- @param widget a widget object +---- @param widget a window-level widget object, "all", or nil ---- @param refreshtype "full", "flashpartial", "flashui", "partial", "ui", "fast" ----- @param refreshregion a Geom object +---- @param refreshregion an optional Geom object ---- @param refreshdither an optional bool function UIManager:setDirty(widget, refreshtype, refreshregion, refreshdither) if widget then @@ -839,6 +850,37 @@ function UIManager:getTopWidget() return top.widget end +--- Get the *second* topmost widget, if there is one (name if possible, ref otherwise). +--- Useful when VirtualKeyboard is involved, as it *always* steals the top spot ;). +--- NOTE: Will skip over VirtualKeyboard instances, plural, in case there are multiple (because, apparently, we can do that.. ugh). +function UIManager:getSecondTopmostWidget() + if #self._window_stack <= 1 then + -- Not enough widgets in the stack, bye! + return nil + end + + -- Because everything is terrible, you can actually instantiate multiple VirtualKeyboards, + -- and they'll stack at the top, so, loop until we get something that *isn't* VK... + for i = #self._window_stack - 1, 1, -1 do + local sec = self._window_stack[i] + if not sec or not sec.widget then + return nil + end + + if sec.widget.name then + if sec.widget.name ~= "VirtualKeyboard" then + return sec.widget.name + end + -- Meaning if name is set, and is set to VK => continue, as we want the *next* widget. + -- I *really* miss the continue keyword, Lua :/. + else + return sec.widget + end + end + + return nil +end + --- Check if a widget is still in the window stack, or is a subwidget of a widget still in the window stack function UIManager:isSubwidgetShown(widget, max_depth) for i = #self._window_stack, 1, -1 do diff --git a/frontend/ui/widget/button.lua b/frontend/ui/widget/button.lua index e5f78383d..26b82a02c 100644 --- a/frontend/ui/widget/button.lua +++ b/frontend/ui/widget/button.lua @@ -315,6 +315,10 @@ function Button:onTapSelectButton() -- 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() + -- When VirtualKeyboard is involved, it steals the top widget slot... So, instead, look below it to find the *effective* top-level widget, because we generally don't give a damn about VK here... + if top_widget == "VirtualKeyboard" then + top_widget = UIManager:getSecondTopmostWidget() + end 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., some ButtonTable users) :() @@ -322,21 +326,27 @@ function Button:onTapSelectButton() 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... - -- 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, because we want to catch modals *over* all that ;). - 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 + -- If our parent is no longer the toplevel widget... + if top_widget ~= self.show_parent then + -- ... and the new toplevel covers the full screen, we're done. + if top_widget.covers_fullscreen then return true end + + -- ... and 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.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 end if self.text then diff --git a/frontend/ui/widget/iconbutton.lua b/frontend/ui/widget/iconbutton.lua index d24f4996c..b766c9521 100644 --- a/frontend/ui/widget/iconbutton.lua +++ b/frontend/ui/widget/iconbutton.lua @@ -112,7 +112,11 @@ function IconButton:onTapIconButton() -- 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 callback popped up the VK, it prevents us from finessing this any further, so repaint the whole stack + -- If the callback popped up the VK, it prevents us from finessing this any further, + -- because getPreviousRefreshRegion will return the VK's region, + -- and it's impossible to get the actual geometry of *only* the InputText of an InputDialog, + -- making the same kind of getSecondTopmostWidget trickery as in Button useless, + -- so repaint the whole stack instead. if top_widget == "VirtualKeyboard" then UIManager:waitForVSync() UIManager:setDirty(self.show_parent, function() diff --git a/frontend/ui/widget/menu.lua b/frontend/ui/widget/menu.lua index 66325bd68..ce1e84904 100644 --- a/frontend/ui/widget/menu.lua +++ b/frontend/ui/widget/menu.lua @@ -516,7 +516,7 @@ function MenuItem:onTapSelect(arg, ges) -- If the callback opened the Virtual Keyboard, it gets trickier if top_widget == "VirtualKeyboard" then -- Unfortunately, we can't really tell full-screen widgets apart from - -- stuff that might just pop the keyboard for a TextInput box... + -- stuff that might just pop the keyboard for an InputText box... -- So, a full fenced redraw it is... UIManager:waitForVSync() UIManager:setDirty(self.show_parent, function() diff --git a/frontend/ui/widget/touchmenu.lua b/frontend/ui/widget/touchmenu.lua index 7505370e6..b76961493 100644 --- a/frontend/ui/widget/touchmenu.lua +++ b/frontend/ui/widget/touchmenu.lua @@ -192,7 +192,7 @@ function TouchMenuItem:onTapSelect(arg, ges) -- (this is for TextEditor, Terminal & co) if top_widget == "VirtualKeyboard" then -- Unfortunately, we can't really tell full-screen widgets (e.g., TextEditor, Terminal) apart from - -- stuff that might just pop the keyboard for a TextInput box... + -- stuff that might just pop the keyboard for an InputText box... -- So, a full fenced redraw it is... UIManager:waitForVSync() UIManager:setDirty(self.show_parent, function()