From 285fc75aa7830eeb6d0a02e35dff763f758e0d87 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Tue, 2 Feb 2021 04:27:14 +0100 Subject: [PATCH] Button: Better handling of translucent MovableContainer (#7223) * DictQuickLookup: Preserve alpha when switching dict, and scrolling inside a dict. * Start moving the NumberPicker alpha hack to Button itself This makes handling flash_ui easier and saner, avoiding flickering. * Handle the transparency hack entirely from within Button * Murder the now unnecessary NumberPicker update_callback hack * Tweak comments * And the Button handling made that redundant, too * Squish debug print * More comment tweaks * Reset transparency on scrolling instead of rpeserving it * Reset alpha when switching dictionaries * Simplify the pre/post callbakc transparency state handling And explain why we need to care. * Give a named reference to ButtonDialog's MovableContainer, so the Button alpha hack behaves with it * Document the "self.movable" convention * Amend that comment a bit e.g., we don't care much about MultiConfirmBox'w MpvableContainer, as any button action will close it. * And make SkimTo's MovableContainer accessible so that Button can grok that it's translucent --- frontend/apps/reader/skimtowidget.lua | 9 +++--- frontend/ui/uimanager.lua | 4 +++ frontend/ui/widget/button.lua | 25 +++++++++++++++- frontend/ui/widget/buttondialog.lua | 16 +++++----- frontend/ui/widget/dictquicklookup.lua | 17 ++++++----- frontend/ui/widget/doublespinwidget.lua | 36 ++++------------------- frontend/ui/widget/numberpickerwidget.lua | 4 --- frontend/ui/widget/scrollhtmlwidget.lua | 30 +++++++++++++++---- frontend/ui/widget/scrolltextwidget.lua | 14 +++++++-- frontend/ui/widget/spinwidget.lua | 33 ++++----------------- 10 files changed, 96 insertions(+), 92 deletions(-) diff --git a/frontend/apps/reader/skimtowidget.lua b/frontend/apps/reader/skimtowidget.lua index 8cf5c96a6..67adcfc06 100644 --- a/frontend/apps/reader/skimtowidget.lua +++ b/frontend/apps/reader/skimtowidget.lua @@ -316,6 +316,10 @@ function SkimToWidget:init() vertical_group_down, } } + self.movable = MovableContainer:new{ + -- alpha = 0.8, + self.skimto_frame, + } self[1] = WidgetContainer:new{ align = "center", dimen =Geom:new{ @@ -323,10 +327,7 @@ function SkimToWidget:init() w = self.screen_width, h = self.screen_height, }, - MovableContainer:new{ - -- alpha = 0.8, - self.skimto_frame, - } + self.movable, } if Device:hasDPad() then diff --git a/frontend/ui/uimanager.lua b/frontend/ui/uimanager.lua index bdba07e92..0cac5ce54 100644 --- a/frontend/ui/uimanager.lua +++ b/frontend/ui/uimanager.lua @@ -674,6 +674,10 @@ to, among other things, flag the right widget as setDirty (c.f., those pesky deb This is why you often see stuff doing, when instantiating a new widget, FancyWidget:new{ show_parent = self.show_parent or self }; meaning, if I'm already a subwidget, cascade my parent, otherwise, it means I'm a window-level widget, so cascade myself as that widget's parent ;). +Another convention (that a few things rely on) is naming a (persistent) MovableContainer wrapping a full widget `movable`, accessible as an instance field. +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 ;). + @usage UIManager:setDirty(self.widget, "partial") diff --git a/frontend/ui/widget/button.lua b/frontend/ui/widget/button.lua index d3af202aa..e5f78383d 100644 --- a/frontend/ui/widget/button.lua +++ b/frontend/ui/widget/button.lua @@ -234,6 +234,13 @@ function Button:showHide(show) end function Button:onTapSelectButton() + -- NOTE: We have a few tricks up our sleeve in case our parent is inside a translucent MovableContainer... + local was_translucent = self.show_parent and self.show_parent.movable and self.show_parent.movable.alpha + -- We make a distinction between transparency pre- and post- callback, because if a widget *was* transparent, + -- but no longer is post-callback, we want to ensure that we refresh the *full* container, + -- instead of just the button's frame, in order to avoid leaving bits of the widget transparent ;). + local is_translucent = was_translucent + if self.enabled and self.callback then if G_reader_settings:isFalse("flash_ui") then self.callback() @@ -276,7 +283,13 @@ function Button:onTapSelectButton() UIManager:forceRePaint() -- Ensures we have a chance to see the highlight end self.callback() - UIManager:forceRePaint() -- Ensures whatever the callback wanted to paint will be shown *now*... + -- Check if the callback reset transparency... + is_translucent = was_translucent and self.show_parent.movable.alpha + -- We don't want to fence the callback when we're *still* translucent, because we want a *single* refresh post-callback *and* post-unhighlight, + -- in order to avoid flickering. + if not is_translucent then + UIManager:forceRePaint() -- Ensures whatever the callback wanted to paint will be shown *now*... + end if self.vsync then -- NOTE: This is mainly useful when the callback caused a REAGL update that we do not explicitly fence already, -- (i.e., Kobo Mk. 7). @@ -346,6 +359,16 @@ function Button:onTapSelectButton() elseif type(self.tap_input_func) == "function" then self:onInput(self.tap_input_func()) end + + -- If our parent belongs to a translucent MovableContainer, repaint all the things to honor alpha without layering glitches, + -- and refresh the full container, because the widget might have inhibited its own setDirty call to avoid flickering (c.f., *SpinWidget). + if was_translucent then + -- If the callback reset the transparency, we only need to repaint our parent + UIManager:setDirty(is_translucent and "all" or self.show_parent, function() + return "ui", self.show_parent.movable.dimen + end) + end + if self.readonly ~= true then return true end diff --git a/frontend/ui/widget/buttondialog.lua b/frontend/ui/widget/buttondialog.lua index 16a33099d..d0412fc02 100644 --- a/frontend/ui/widget/buttondialog.lua +++ b/frontend/ui/widget/buttondialog.lua @@ -37,9 +37,7 @@ function ButtonDialog:init() } } end - self[1] = CenterContainer:new{ - dimen = Screen:getSize(), - MovableContainer:new{ + self.movable = MovableContainer:new{ alpha = self.alpha, FrameContainer:new{ ButtonTable:new{ @@ -56,19 +54,23 @@ function ButtonDialog:init() padding_top = 0, padding_bottom = 0, } - } + } + + self[1] = CenterContainer:new{ + dimen = Screen:getSize(), + self.movable, } end function ButtonDialog:onShow() UIManager:setDirty(self, function() - return "ui", self[1][1].dimen + return "ui", self.movable.dimen end) end function ButtonDialog:onCloseWidget() UIManager:setDirty(nil, function() - return "flashui", self[1][1].dimen + return "flashui", self.movable.dimen end) end @@ -87,7 +89,7 @@ end function ButtonDialog:paintTo(...) InputContainer.paintTo(self, ...) - self.dimen = self[1][1].dimen -- FrameContainer + self.dimen = self.movable.dimen end return ButtonDialog diff --git a/frontend/ui/widget/dictquicklookup.lua b/frontend/ui/widget/dictquicklookup.lua index dd6d79896..309fe6bf8 100644 --- a/frontend/ui/widget/dictquicklookup.lua +++ b/frontend/ui/widget/dictquicklookup.lua @@ -828,14 +828,15 @@ function DictQuickLookup:update() end end - -- Reset alpha to avoid stacking transparency on top of the previous content. - -- NOTE: This doesn't take care of the Scroll*Widget, which will preserve alpha on scroll, - -- leading to increasingly opaque and muddy text as half-transparent stuff gets stacked on top of each other... - self.movable.alpha = nil - - UIManager:setDirty(self, function() - return "partial", self.dict_frame.dimen - end) + -- If we're translucent, reset alpha to make the new definition actually readable. + if self.movable.alpha then + self.movable.alpha = nil + -- And skip the setDirty, Button will handle it post-callback & post-unhighlight. + else + UIManager:setDirty(self, function() + return "partial", self.dict_frame.dimen + end) + end end function DictQuickLookup:getInitialVisibleArea() diff --git a/frontend/ui/widget/doublespinwidget.lua b/frontend/ui/widget/doublespinwidget.lua index 876f2bb67..4f2c0f1f0 100644 --- a/frontend/ui/widget/doublespinwidget.lua +++ b/frontend/ui/widget/doublespinwidget.lua @@ -83,10 +83,6 @@ function DoubleSpinWidget:init() end function DoubleSpinWidget:update() - -- This picker_update_callback will be redefined later. - -- It's a hack to restore transparency after a Button unhighlight in NumberPicker, - -- in case the MovableContainer was actually made transparent. - local picker_update_callback = function() end local left_widget = NumberPickerWidget:new{ show_parent = self, width = self.picker_width, @@ -96,7 +92,6 @@ function DoubleSpinWidget:update() value_step = self.left_step, value_hold_step = self.left_hold_step, wrap = false, - update_callback = function() picker_update_callback() end, } local right_widget = NumberPickerWidget:new{ show_parent = self, @@ -107,7 +102,6 @@ function DoubleSpinWidget:update() value_step = self.right_step, value_hold_step = self.right_hold_step, wrap = false, - update_callback = function() picker_update_callback() end, } local left_vertical_group = VerticalGroup:new{ align = "center", @@ -287,31 +281,11 @@ function DoubleSpinWidget:update() }, self.movable, } - UIManager:setDirty(self, function() - return "ui", self.widget_frame.dimen - end) - picker_update_callback = function() - -- If we're actually transparent, force an alpha-aware repaint. - if self.movable.alpha then - if G_reader_settings:nilOrTrue("flash_ui") then - -- It's delayed to the next tick to actually catch a Button unhighlight. - UIManager:nextTick(function() - UIManager:setDirty("all", function() - return "ui", self.movable.dimen - end) - end) - else - -- This should only really be necessary for the up/down buttons here, - -- because they repaint the center value button & text, unlike said button, - -- which just pops up the VK. - -- On the upside, we shouldn't need to delay anything without flash_ui ;). - UIManager:setDirty("all", function() - return "ui", self.movable.dimen - end) - end - end - -- If we'd like to have the values auto-applied, uncomment this: - -- self.callback(left_widget:getValue(), right_widget:getValue()) + -- If we're translucent, Button itself will handle that post-callback, in order to preserve alpha without flickering. + if not self.movable.alpha then + UIManager:setDirty(self, function() + return "ui", self.widget_frame.dimen + end) end end diff --git a/frontend/ui/widget/numberpickerwidget.lua b/frontend/ui/widget/numberpickerwidget.lua index b8a0d2167..5add128ed 100644 --- a/frontend/ui/widget/numberpickerwidget.lua +++ b/frontend/ui/widget/numberpickerwidget.lua @@ -12,7 +12,6 @@ Example: value_step = 1, value_hold_step = 4, wrap = true, - update_callback = function() end, } --]] @@ -46,7 +45,6 @@ local NumberPickerWidget = InputContainer:new{ value_table = nil, value_index = nil, wrap = true, - update_callback = function() end, -- in case we need calculate number of days in a given month and year date_month = nil, date_year = nil, @@ -169,7 +167,6 @@ function NumberPickerWidget:init() }, }, } - self.update_callback() UIManager:show(input_dialog) input_dialog:onShowKeyboard() end @@ -229,7 +226,6 @@ function NumberPickerWidget:update() UIManager:setDirty(self.show_parent, function() return "ui", self.dimen end) - self.update_callback() end --[[-- diff --git a/frontend/ui/widget/scrollhtmlwidget.lua b/frontend/ui/widget/scrollhtmlwidget.lua index adf2a46f9..a43144b77 100644 --- a/frontend/ui/widget/scrollhtmlwidget.lua +++ b/frontend/ui/widget/scrollhtmlwidget.lua @@ -119,9 +119,19 @@ function ScrollHtmlWidget:scrollToRatio(ratio) self.htmlbox_widget:freeBb() self.htmlbox_widget:_render() - UIManager:setDirty(self.dialog, function() - return "partial", self.dimen - end) + -- If our dialog is currently wrapped in a MovableContainer and that container has been made translucent, + -- reset the alpha and refresh the whole thing, because we assume that a scroll means the user actually wants to + -- *read* the content, which is kinda hard on a nearly transparent widget ;). + if self.dialog.movable and self.dialog.movable.alpha then + self.dialog.movable.alpha = nil + UIManager:setDirty(self.dialog, function() + return "partial", self.dialog.movable.dimen + end) + else + UIManager:setDirty(self.dialog, function() + return "partial", self.dimen + end) + end end function ScrollHtmlWidget:scrollText(direction) @@ -147,9 +157,17 @@ function ScrollHtmlWidget:scrollText(direction) self.htmlbox_widget:freeBb() self.htmlbox_widget:_render() - UIManager:setDirty(self.dialog, function() - return "partial", self.dimen - end) + -- Handle the container's alpha as above... + if self.dialog.movable and self.dialog.movable.alpha then + self.dialog.movable.alpha = nil + UIManager:setDirty(self.dialog, function() + return "partial", self.dialog.movable.dimen + end) + else + UIManager:setDirty(self.dialog, function() + return "partial", self.dimen + end) + end end function ScrollHtmlWidget:onScrollText(arg, ges) diff --git a/frontend/ui/widget/scrolltextwidget.lua b/frontend/ui/widget/scrolltextwidget.lua index 1316a637e..dfbe9faeb 100644 --- a/frontend/ui/widget/scrolltextwidget.lua +++ b/frontend/ui/widget/scrolltextwidget.lua @@ -150,9 +150,17 @@ function ScrollTextWidget:updateScrollBar(is_partial) if is_partial then refreshfunc = "partial" end - UIManager:setDirty(self.dialog, function() - return refreshfunc, self.dimen - end) + -- Reset transparency if the dialog's MovableContainer is currently translucent... + if is_partial and self.dialog.movable and self.dialog.movable.alpha then + self.dialog.movable.alpha = nil + UIManager:setDirty(self.dialog, function() + return refreshfunc, self.dialog.movable.dimen + end) + else + UIManager:setDirty(self.dialog, function() + return refreshfunc, self.dimen + end) + end if self.scroll_callback then self.scroll_callback(low, high) end diff --git a/frontend/ui/widget/spinwidget.lua b/frontend/ui/widget/spinwidget.lua index f93050109..70427a216 100644 --- a/frontend/ui/widget/spinwidget.lua +++ b/frontend/ui/widget/spinwidget.lua @@ -78,10 +78,6 @@ function SpinWidget:init() end function SpinWidget:update() - -- This picker_update_callback will be redefined later. - -- It's a hack to restore transparency after a Button unhighlight in NumberPicker, - -- in case the MovableContainer was actually made transparent. - local picker_update_callback = function() end local value_widget = NumberPickerWidget:new{ show_parent = self, width = math.floor(self.screen_width * 0.2), @@ -93,7 +89,6 @@ function SpinWidget:update() value_step = self.value_step, value_hold_step = self.value_hold_step, precision = self.precision, - update_callback = function() picker_update_callback() end, } local value_group = HorizontalGroup:new{ align = "center", @@ -239,29 +234,11 @@ function SpinWidget:update() }, self.movable, } - UIManager:setDirty(self, function() - return "ui", self.spin_frame.dimen - end) - picker_update_callback = function() - -- If we're actually transparent, force an alpha-aware repaint. - if self.movable.alpha then - if G_reader_settings:nilOrTrue("flash_ui") then - -- It's delayed to the next tick to actually catch a Button unhighlight. - UIManager:nextTick(function() - UIManager:setDirty("all", function() - return "ui", self.movable.dimen - end) - end) - else - -- This should only really be necessary for the up/down buttons here, - -- because they repaint the center value button & text, unlike said button, - -- which just pops up the VK. - -- On the upside, we shouldn't need to delay anything without flash_ui ;). - UIManager:setDirty("all", function() - return "ui", self.movable.dimen - end) - end - end + -- If we're translucent, Button itself will handle that post-callback, in order to preserve alpha without flickering. + if not self.movable.alpha then + UIManager:setDirty(self, function() + return "ui", self.spin_frame.dimen + end) end end