From f2f2aa6e4dbdcd27ca0b1d38b6c907f158d04bd3 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Tue, 15 Oct 2019 21:57:51 +0200 Subject: [PATCH] Minor state handling tweaks when the footer is disabled/invisible (#5494) * Don't break `doc_pages` updates when it's completely disabled. (c.f., the commit's comment, doing it on every page turn seems a bit overkill, but I get that it's probably done that way because it was easier ;)). * Detect the empty footerTextGenerator properly in `_updateFooterText` (it's `""`, not `nil`). * Abort `_updateFooterText` early when the footer is invisible (and has been for a while/ever, i.e., when not requesting a repaint, like a mode switch would). * Never send `SetBottomPageMargin` events twice. * Minor tweaks to touchmenu & configdialog to avoid useless repaints (don't redraw the reader/fm unless we absolutely need to). * Make sure we show the progress bar alone instead of nothing when disabling the last enabled mode in the menu. --- frontend/apps/reader/modules/readerfooter.lua | 57 +++++++++++-------- frontend/ui/widget/configdialog.lua | 7 ++- frontend/ui/widget/touchmenu.lua | 4 +- spec/unit/readerfooter_spec.lua | 3 + 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/frontend/apps/reader/modules/readerfooter.lua b/frontend/apps/reader/modules/readerfooter.lua index 2e9b684e4..1de8012fa 100644 --- a/frontend/apps/reader/modules/readerfooter.lua +++ b/frontend/apps/reader/modules/readerfooter.lua @@ -244,7 +244,7 @@ function ReaderFooter:init() self.mode_list[self.mode_index[i]] = i end if self.settings.disabled then - -- footer featuren disabled completely, stop initialization now + -- footer feature is completely disabled, stop initialization now self:disableFooter() return end @@ -472,8 +472,8 @@ function ReaderFooter:disableFooter() self.onReaderReady = function() end self.resetLayout = function() end self.onCloseDocument = nil - self.onPageUpdate = function() end - self.onPosUpdate = function() end + self.updateFooterPage = function() end + self.updateFooterPos = function() end self.onUpdatePos = function() end self.onSetStatusLine = function() end self.mode = self.mode_list.off @@ -576,6 +576,8 @@ function ReaderFooter:addToMainMenu(menu_items) callback = function() self.settings[option] = not self.settings[option] G_reader_settings:saveSetting("footer", self.settings) + -- We only need to send a SetPageBottomMargin event when we truly affect the margin + local should_signal = false -- only case that we don't need a UI update is enable/disable -- non-current mode when all_at_once is disabled. local should_update = false @@ -595,7 +597,7 @@ function ReaderFooter:addToMainMenu(menu_items) if self.has_no_mode then self.footer_container.dimen.h = 0 self.footer_text.height = 0 - self.ui:handleEvent(Event:new("SetPageBottomMargin", self.view.document.configurable.b_page_margin)) + should_signal = true self.genFooterText = footerTextGeneratorMap.empty self.mode = self.mode_list.off elseif prev_has_no_mode then @@ -605,10 +607,10 @@ function ReaderFooter:addToMainMenu(menu_items) self.mode = self.mode_list.page_progress self:applyFooterMode() end - self.ui:handleEvent(Event:new("SetPageBottomMargin", self.view.document.configurable.b_page_margin)) + should_signal = true G_reader_settings:saveSetting("reader_footer_mode", first_enabled_mode_num) elseif self.reclaim_height ~= prev_reclaim_height then - self.ui:handleEvent(Event:new("SetPageBottomMargin", self.view.document.configurable.b_page_margin)) + should_signal = true should_update = true end if callback then @@ -622,12 +624,18 @@ function ReaderFooter:addToMainMenu(menu_items) -- progress bar if not self.has_no_mode then self.mode = first_enabled_mode_num + else + -- If we've just disabled our last mode, first_enabled_mode_num is nil + -- If the progress bar is enabled, + -- fake an innocuous mode so that we switch to showing the progress bar alone, instead of nothing, + -- This is exactly what the "Show progress bar" toggle does. + self.mode = self.settings.disable_progress_bar and self.mode_list.off or self.mode_list.page_progress end should_update = true self:applyFooterMode() end - if should_update then - self:refreshFooter(true, true) + if should_update or should_signal then + self:refreshFooter(should_update, should_signal) end end, } @@ -1246,39 +1254,43 @@ function ReaderFooter:getDataFromStatistics(title, pages) return title .. sec end -function ReaderFooter:updateFooter(force_repaint) +function ReaderFooter:updateFooter(force_repaint, force_recompute) if self.pageno then - self:updateFooterPage(force_repaint) + self:updateFooterPage(force_repaint, force_recompute) else - self:updateFooterPos(force_repaint) + self:updateFooterPos(force_repaint, force_recompute) end end -function ReaderFooter:updateFooterPage(force_repaint) +function ReaderFooter:updateFooterPage(force_repaint, force_recompute) if type(self.pageno) ~= "number" then return end self.progress_bar.percentage = self.pageno / self.pages - self:updateFooterText(force_repaint) + self:updateFooterText(force_repaint, force_recompute) end -function ReaderFooter:updateFooterPos(force_repaint) +function ReaderFooter:updateFooterPos(force_repaint, force_recompute) if type(self.position) ~= "number" then return end self.progress_bar.percentage = self.position / self.doc_height - self:updateFooterText(force_repaint) + self:updateFooterText(force_repaint, force_recompute) end -- updateFooterText will start as a noop. After onReaderReady event is -- received, it will initialized as _updateFooterText below -function ReaderFooter:updateFooterText(force_repaint) +function ReaderFooter:updateFooterText(force_repaint, force_recompute) end -- only call this function after document is fully loaded -function ReaderFooter:_updateFooterText(force_repaint) +function ReaderFooter:_updateFooterText(force_repaint, force_recompute) + -- footer is invisible, we need neither a repaint nor a recompute, go away. + if not self.view.footer_visible and not force_repaint and not force_recompute then + return + end local text = self:genFooterText() if text then self.footer_text:setText(text) end if self.settings.disable_progress_bar then - if self.has_no_mode or not text then + if self.has_no_mode or not text or text == "" then self.text_width = 0 self.footer_container.dimen.h = 0 self.footer_text.height = 0 @@ -1294,7 +1306,7 @@ function ReaderFooter:_updateFooterText(force_repaint) self.progress_bar.width = math.floor(self._saved_screen_width - 2 * self.settings.progress_margin_width) self.text_width = self.footer_text:getSize().w else - if self.has_no_mode or not text then + if self.has_no_mode or not text or text == "" then self.text_width = 0 else self.text_width = self.footer_text:getSize().w + self.text_left_margin @@ -1482,13 +1494,12 @@ end function ReaderFooter:refreshFooter(refresh, signal) self:updateFooterContainer() self:resetLayout(true) - self:updateFooter() + -- If we signal, the event we send will trigger a full repaint anyway, so we should be able to skip this one. + -- We *do* need to ensure we at least re-compute the footer layout, though, especially when going from visible to invisible... + self:updateFooter(refresh and not signal, refresh and signal) if signal then self.ui:handleEvent(Event:new("SetPageBottomMargin", self.view.document.configurable.b_page_margin)) end - if refresh then - UIManager:setDirty(nil, "ui") - end end function ReaderFooter:onResume() diff --git a/frontend/ui/widget/configdialog.lua b/frontend/ui/widget/configdialog.lua index e4ba0c611..3de2698b1 100644 --- a/frontend/ui/widget/configdialog.lua +++ b/frontend/ui/widget/configdialog.lua @@ -865,10 +865,15 @@ end function ConfigDialog:onShowConfigPanel(index) self.panel_index = index local old_dimen = self.dialog_frame.dimen and self.dialog_frame.dimen:copy() + local old_layout_h = self.layout and #self.layout self:update() -- NOTE: Keep that one as UI to avoid delay when both this and the topmenu are shown. -- Plus, this is also called for each tab anyway, so that wouldn't have been great. - UIManager:setDirty(self.is_fresh and self or "all", function() + -- NOTE: And we also only need to repaint what's behind us when switching to a smaller dialog... + -- This is trickier than in touchmenu, because dimen appear to fluctuate before/after painting... + -- So we've settled instead for the amount of lines in the panel, as line-height is constant. + local keep_bg = old_layout_h and #self.layout >= old_layout_h + UIManager:setDirty((self.is_fresh or keep_bg) and self or "all", function() local refresh_dimen = old_dimen and old_dimen:combine(self.dialog_frame.dimen) or self.dialog_frame.dimen diff --git a/frontend/ui/widget/touchmenu.lua b/frontend/ui/widget/touchmenu.lua index 84586f755..5d7b83b99 100644 --- a/frontend/ui/widget/touchmenu.lua +++ b/frontend/ui/widget/touchmenu.lua @@ -614,7 +614,9 @@ function TouchMenu:updateItems() -- NOTE: We use a slightly ugly hack to detect a brand new menu vs. a tab switch, -- in order to optionally flash on initial menu popup... -- NOTE: Also avoid repainting what's underneath us on initial popup. - UIManager:setDirty(self.is_fresh and self.show_parent or "all", function() + -- NOTE: And we also only need to repaint what's behind us when switching to a smaller menu... + local keep_bg = old_dimen and self.dimen.h >= old_dimen.h + UIManager:setDirty((self.is_fresh or keep_bg) and self.show_parent or "all", function() local refresh_dimen = old_dimen and old_dimen:combine(self.dimen) or self.dimen diff --git a/spec/unit/readerfooter_spec.lua b/spec/unit/readerfooter_spec.lua index 60721511a..1c3b51740 100644 --- a/spec/unit/readerfooter_spec.lua +++ b/spec/unit/readerfooter_spec.lua @@ -270,6 +270,9 @@ describe("Readerfooter module", function() assert.is.same(1, footer.mode) footer:onTapFooter() assert.is.same(0, footer.mode) + -- Make it visible again to make the following tests behave... + footer:onTapFooter() + assert.is.same(1, footer.mode) end) it("should pick up screen resize in resetLayout", function()