From c22dbbe3ae76adf75ae5ed9f3ce074eb9ee9cf7d Mon Sep 17 00:00:00 2001 From: Hans-Werner Hilse Date: Fri, 14 Nov 2014 09:09:50 +0100 Subject: [PATCH 1/2] factor out repaint to its own method it's gotten complex enough to isolate it --- frontend/ui/uimanager.lua | 194 +++++++++++++++++++------------------- 1 file changed, 98 insertions(+), 96 deletions(-) diff --git a/frontend/ui/uimanager.lua b/frontend/ui/uimanager.lua index 0f1b69382..75137beb5 100644 --- a/frontend/ui/uimanager.lua +++ b/frontend/ui/uimanager.lua @@ -373,6 +373,104 @@ function UIManager:checkTasks() return wait_until end +-- repaint dirty widgets +function UIManager:repaint() + local dirty = false + local request_full_refresh = false + local force_full_refresh = false + local force_partial_refresh = false + local force_fast_refresh = false + for _, widget in ipairs(self._window_stack) do + -- paint if repaint_all is request + -- paint also if current widget or any widget underneath is dirty + if self.repaint_all or dirty or self._dirty[widget.widget] then + widget.widget:paintTo(Screen.bb, widget.x, widget.y) + + if self._dirty[widget.widget] == "auto" then + request_full_refresh = true + end + if self._dirty[widget.widget] == "full" then + force_full_refresh = true + end + if self._dirty[widget.widget] == "partial" then + force_partial_refresh = true + end + if self._dirty[widget.widget] == "fast" then + force_fast_refresh = true + end + -- and remove from list after painting + self._dirty[widget.widget] = nil + -- trigger repaint + dirty = true + end + end + + if self.full_refresh then + dirty = true + force_full_refresh = true + end + + if self.partial_refresh then + dirty = true + force_partial_refresh = true + end + + self.repaint_all = false + self.full_refresh = false + self.partial_refresh = false + + local refresh_type = self.default_refresh_type + local waveform_mode = self.default_waveform_mode + local wait_for_marker = self.wait_for_every_marker + if dirty then + if force_partial_refresh or force_fast_refresh then + refresh_type = UPDATE_MODE_PARTIAL + elseif force_full_refresh or self.refresh_count == self.FULL_REFRESH_COUNT - 1 then + refresh_type = UPDATE_MODE_FULL + end + -- Handle the waveform mode selection... + if refresh_type == UPDATE_MODE_FULL then + waveform_mode = self.full_refresh_waveform_mode + else + waveform_mode = self.partial_refresh_waveform_mode + end + if force_fast_refresh then + waveform_mode = self.fast_waveform_mode + -- FIXME: Should we also avoid doing an MXCFB_WAIT_FOR_UPDATE_SUBMISSION for this update? + --wait_for_marker = false + end + -- If the device is REAGL-aware, we're specifically asking for a REAGL update, and we're doing a PARTIAL *reader* refresh, apply some trickery to match the stock reader's behavior + -- (On most device, REAGL updates are always FULL, but there's no black flash. On devices where this isn't the case [H2O], we're letting the driver do the job by using AUTO). + if not force_partial_refresh and not force_fast_refresh and refresh_type == UPDATE_MODE_PARTIAL and (waveform_mode == WAVEFORM_MODE_REAGL or waveform_mode == NTX_WFM_MODE_GLD16) then + refresh_type = UPDATE_MODE_FULL + end + -- On the other hand, if we asked for a PARTIAL *UI* refresh, fall back to the default waveform mode, which is tailored per-device to hopefully be more appropriate in this instance than the one we use in the reader. + if force_partial_refresh then + -- NOTE: Using default_waveform_mode might seem counter-intuitive when we have partial_refresh_waveform_mode, but partial_refresh_waveform_mode is mostly there as a means to flag REAGL-aware devices ;). + -- Here, we're actually interested in handling PARTIAL, regional (be it properly flagged or not) updates, and not the PARTIAL updates from the reader that actually refresh the whole screen (i.e., those between black flashes). + waveform_mode = self.default_waveform_mode + end + if self.update_regions_func then + local update_regions = self.update_regions_func() + for _, update_region in ipairs(update_regions) do + -- in some rare cases update region has 1 pixel offset + Screen:refresh(refresh_type, waveform_mode, wait_for_marker, + update_region.x-1, update_region.y-1, + update_region.w+2, update_region.h+2) + end + else + Screen:refresh(refresh_type, waveform_mode, wait_for_marker) + end + -- REAGL refreshes are always FULL (but without a black flash), but we want to keep our black flash timeout working, so don't reset the counter on FULL REAGL refreshes... + if refresh_type == UPDATE_MODE_FULL and waveform_mode ~= WAVEFORM_MODE_REAGL and waveform_mode ~= NTX_WFM_MODE_GLD16 then + self.refresh_count = 0 + elseif not force_partial_refresh and not force_full_refresh then + self.refresh_count = (self.refresh_count + 1)%self.FULL_REFRESH_COUNT + end + self.update_regions_func = nil + end +end + -- this is the main loop of the UI controller -- it is intended to manage input events and delegate -- them to dialogs @@ -395,102 +493,6 @@ function UIManager:run() return nil end - -- repaint dirty widgets - local dirty = false - local request_full_refresh = false - local force_full_refresh = false - local force_partial_refresh = false - local force_fast_refresh = false - for _, widget in ipairs(self._window_stack) do - -- paint if repaint_all is request - -- paint also if current widget or any widget underneath is dirty - if self.repaint_all or dirty or self._dirty[widget.widget] then - widget.widget:paintTo(Screen.bb, widget.x, widget.y) - - if self._dirty[widget.widget] == "auto" then - request_full_refresh = true - end - if self._dirty[widget.widget] == "full" then - force_full_refresh = true - end - if self._dirty[widget.widget] == "partial" then - force_partial_refresh = true - end - if self._dirty[widget.widget] == "fast" then - force_fast_refresh = true - end - -- and remove from list after painting - self._dirty[widget.widget] = nil - -- trigger repaint - dirty = true - end - end - - if self.full_refresh then - dirty = true - force_full_refresh = true - end - - if self.partial_refresh then - dirty = true - force_partial_refresh = true - end - - self.repaint_all = false - self.full_refresh = false - self.partial_refresh = false - - local refresh_type = self.default_refresh_type - local waveform_mode = self.default_waveform_mode - local wait_for_marker = self.wait_for_every_marker - if dirty then - if force_partial_refresh or force_fast_refresh then - refresh_type = UPDATE_MODE_PARTIAL - elseif force_full_refresh or self.refresh_count == self.FULL_REFRESH_COUNT - 1 then - refresh_type = UPDATE_MODE_FULL - end - -- Handle the waveform mode selection... - if refresh_type == UPDATE_MODE_FULL then - waveform_mode = self.full_refresh_waveform_mode - else - waveform_mode = self.partial_refresh_waveform_mode - end - if force_fast_refresh then - waveform_mode = self.fast_waveform_mode - -- FIXME: Should we also avoid doing an MXCFB_WAIT_FOR_UPDATE_SUBMISSION for this update? - --wait_for_marker = false - end - -- If the device is REAGL-aware, we're specifically asking for a REAGL update, and we're doing a PARTIAL *reader* refresh, apply some trickery to match the stock reader's behavior - -- (On most device, REAGL updates are always FULL, but there's no black flash. On devices where this isn't the case [H2O], we're letting the driver do the job by using AUTO). - if not force_partial_refresh and not force_fast_refresh and refresh_type == UPDATE_MODE_PARTIAL and (waveform_mode == WAVEFORM_MODE_REAGL or waveform_mode == NTX_WFM_MODE_GLD16) then - refresh_type = UPDATE_MODE_FULL - end - -- On the other hand, if we asked for a PARTIAL *UI* refresh, fall back to the default waveform mode, which is tailored per-device to hopefully be more appropriate in this instance than the one we use in the reader. - if force_partial_refresh then - -- NOTE: Using default_waveform_mode might seem counter-intuitive when we have partial_refresh_waveform_mode, but partial_refresh_waveform_mode is mostly there as a means to flag REAGL-aware devices ;). - -- Here, we're actually interested in handling PARTIAL, regional (be it properly flagged or not) updates, and not the PARTIAL updates from the reader that actually refresh the whole screen (i.e., those between black flashes). - waveform_mode = self.default_waveform_mode - end - if self.update_regions_func then - local update_regions = self.update_regions_func() - for _, update_region in ipairs(update_regions) do - -- in some rare cases update region has 1 pixel offset - Screen:refresh(refresh_type, waveform_mode, wait_for_marker, - update_region.x-1, update_region.y-1, - update_region.w+2, update_region.h+2) - end - else - Screen:refresh(refresh_type, waveform_mode, wait_for_marker) - end - -- REAGL refreshes are always FULL (but without a black flash), but we want to keep our black flash timeout working, so don't reset the counter on FULL REAGL refreshes... - if refresh_type == UPDATE_MODE_FULL and waveform_mode ~= WAVEFORM_MODE_REAGL and waveform_mode ~= NTX_WFM_MODE_GLD16 then - self.refresh_count = 0 - elseif not force_partial_refresh and not force_full_refresh then - self.refresh_count = (self.refresh_count + 1)%self.FULL_REFRESH_COUNT - end - self.update_regions_func = nil - end - self:checkTasks() -- wait for next event From 45cf927ee8704582c942c477b6113c371b1f6963 Mon Sep 17 00:00:00 2001 From: Hans-Werner Hilse Date: Fri, 14 Nov 2014 09:12:46 +0100 Subject: [PATCH 2/2] Fix double-check of task list Since commit 12a76fee3375741bc257ddbfd6eaa9fc531695ca, we had a potential bug on the event mechanism: It introduced (besides the checkTasks method itself) a second run of the checkTasks() method. In the second run, however, scheduled events were not taken into consideration in how long to wait for input events afterwards. So when the after the first run of checkTasks() there were new scheduled tasks added to the task queue, they were not properly scheduled and and depended on an already existing scheduled event or an input event to trigger. This might have led to unexpected order of execution (though the order is not guaranteed by the task scheduling anyway!) or to events triggering not at all until the next input event. --- frontend/ui/uimanager.lua | 42 +++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/frontend/ui/uimanager.lua b/frontend/ui/uimanager.lua index 75137beb5..98eed17bd 100644 --- a/frontend/ui/uimanager.lua +++ b/frontend/ui/uimanager.lua @@ -69,6 +69,7 @@ local UIManager = { _running = true, _window_stack = {}, _execution_stack = {}, + _execution_stack_dirty = false, _dirty = {}, _zeromqs = {}, } @@ -232,6 +233,7 @@ end -- schedule an execution task function UIManager:schedule(time, action) table.insert(self._execution_stack, { time = time, action = action }) + self._execution_stack_dirty = true end -- schedule task in a certain amount of seconds (fractions allowed) from now @@ -370,7 +372,8 @@ function UIManager:checkTasks() end end until all_tasks_checked - return wait_until + self._execution_stack_dirty = false + return wait_until, now end -- repaint dirty widgets @@ -477,26 +480,31 @@ end function UIManager:run() self._running = true while self._running do - local now = { util.gettime() } - local wait_until = self:checkTasks() + local wait_until, now + -- run this in a loop, so that paints can trigger events + -- that will be honored when calculating the time to wait + -- for input events: + repeat + wait_until, now = self:checkTasks() - --DEBUG("---------------------------------------------------") - --DEBUG("exec stack", self._execution_stack) - --DEBUG("window stack", self._window_stack) - --DEBUG("dirty stack", self._dirty) - --DEBUG("---------------------------------------------------") + --DEBUG("---------------------------------------------------") + --DEBUG("exec stack", self._execution_stack) + --DEBUG("window stack", self._window_stack) + --DEBUG("dirty stack", self._dirty) + --DEBUG("---------------------------------------------------") - -- stop when we have no window to show - if #self._window_stack == 0 then - DEBUG("no dialog left to show") - self:quit() - return nil - end + -- stop when we have no window to show + if #self._window_stack == 0 then + DEBUG("no dialog left to show") + self:quit() + return nil + end - self:checkTasks() + self:repaint() + until not self._execution_stack_dirty -- wait for next event - -- note that we will skip that if in the meantime we have tasks that are ready to run + -- note that we will skip that if we have tasks that are ready to run local input_event = nil if not wait_until then if #self._zeromqs > 0 then @@ -509,7 +517,7 @@ function UIManager:run() end end else - -- no pending task, wait endlessly + -- no pending task, wait without timeout input_event = Input:waitEvent() end elseif wait_until[1] > now[1]