Archives
/
koreader
mirror of https://github.com/koreader/koreader
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

flash_ui: Workaround potential EPDC races (#7332)

Browse Source 
* flash_ui: Yield to the kernel between the HL and the UNHL/CB to let the EPDC do its thing in peace.
* UIManager: Handle nils in task scheduling arguments.
* SkimTo: Use the same, thicker chapter nav icons as ReaderSearch (fix #7326).
* SkimTo: The bookmark toggle button doesn't require a vsync flag.
pull/7336/head
NiLuJe 3 years ago committed by GitHub
parent e582036c3e
commit 75356f2837
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 70 additions and 39 deletions
Show Stats Download Patch File Download Diff File

2
base

@ -1 +1 @@
Subproject commit 529d74140f5ad4d603e6a5939664a29acbfd770f
Subproject commit eaeb52226a16b69e21231e2d85d4e775f1605e86

21
frontend/ui/uimanager.lua
Unescape Escape View File

@ -538,6 +538,7 @@ function UIManager:schedule(time, action, ...)
table.insert(self._task_queue, p, {
time = time,
action = action,
argc = select('#', ...),
args = {...},
})
self._task_queue_dirty = true
@ -1177,7 +1178,7 @@ function UIManager:_checkTasks()
-- task is pending to be executed right now. do it.
-- NOTE: be careful that task.action() might modify
-- _task_queue here. So need to avoid race condition
task.action(unpack(task.args or {}))
task.action(unpack(task.args, 1, task.argc))
else
-- queue is sorted in ascendant order, safe to assume all items
-- are future tasks for now
@ -1463,6 +1464,24 @@ function UIManager:waitForVSync()
Screen:refreshWaitForLast()
end
--[[--
Yield to the EPDC.
This is a dumb workaround for potential races with the EPDC when we request a refresh on a specific region,
and then proceed to *write* to the framebuffer, in the same region, very, very, very soon after that.
This basically just puts ourselves to sleep for a very short amount of time, to let the kernel do its thing in peace.
@int sleep_us Amount of time to sleep for (in µs). (Optional, defaults to 1ms).
]]
function UIManager:yieldToEPDC(sleep_us)
if Device:hasEinkScreen() then
-- NOTE: Early empiric evidence suggests that going as low as 1ms is enough to do the trick.
-- Consider jumping to the jiffy resolution (100Hz/10ms) if it turns out it isn't ;).
ffiUtil.usleep(sleep_us or 1000)
end
end
--[[--
Used to repaint a specific sub-widget that isn't on the `_window_stack` itself.

9
frontend/ui/widget/button.lua
Unescape Escape View File

@ -301,6 +301,15 @@ function Button:onTapSelectButton()
if not self.vsync then
-- NOTE: Except when a Button is flagged vsync, in which case we *want* to bundle the highlight with the callback, to prevent further delays
UIManager:forceRePaint()
-- NOTE: Yield to the kernel for a tiny slice of time, otherwise, writing to the same fb region as the refresh we've just requested may be race-y,
-- causing mild variants of our friend the papercut refresh glitch ;).
-- Remember that the whole eInk refresh dance is completely asynchronous: we *request* a refresh from the kernel,
-- but it's up to the EPDC to schedule that however it sees fit...
-- The other approach would be to *ask* the EPDC to block until it's *completely* done,
-- but that's too much (because we only care about it being done *reading* the fb),
-- and that could take upwards of 300ms, which is also way too much ;).
UIManager:yieldToEPDC()
end
-- Unhighlight

1
frontend/ui/widget/checkbutton.lua
Unescape Escape View File

@ -115,6 +115,7 @@ function CheckButton:onTapCheckButton()
UIManager:setDirty(nil, "fast", highlight_dimen)
UIManager:forceRePaint()
UIManager:yieldToEPDC()
-- Unhighlight
--

1
frontend/ui/widget/iconbutton.lua
Unescape Escape View File

@ -110,6 +110,7 @@ function IconButton:onTapIconButton()
UIManager:setDirty(nil, "fast", self.dimen)
UIManager:forceRePaint()
UIManager:yieldToEPDC()
-- Unhighlight
--

1
frontend/ui/widget/keyvaluepage.lua
Unescape Escape View File

@ -293,6 +293,7 @@ function KeyValueItem:onTap()
UIManager:setDirty(nil, "fast", self[1].dimen)
UIManager:forceRePaint()
UIManager:yieldToEPDC()
-- Unhighlight
--

2
frontend/ui/widget/menu.lua
Unescape Escape View File

@ -480,6 +480,7 @@ function MenuItem:onTapSelect(arg, ges)
UIManager:setDirty(nil, "fast", self[1].dimen)
UIManager:forceRePaint()
UIManager:yieldToEPDC()
-- Unhighlight
--
@ -514,6 +515,7 @@ function MenuItem:onHoldSelect(arg, ges)
UIManager:setDirty(nil, "fast", self[1].dimen)
UIManager:forceRePaint()
UIManager:yieldToEPDC()
-- Unhighlight
--

1
frontend/ui/widget/radiobutton.lua
Unescape Escape View File

@ -124,6 +124,7 @@ function RadioButton:onTapCheckButton()
UIManager:setDirty(nil, "fast", self.dimen)
UIManager:forceRePaint()
UIManager:yieldToEPDC()
-- Unhighlight
--

5
frontend/ui/widget/skimtowidget.lua
Unescape Escape View File

@ -141,8 +141,8 @@ function SkimToWidget:init()
}
-- Top row buttons
local chapter_next_text = "▷│"
local chapter_prev_text = "│◁"
local chapter_next_text = "▷▏"
local chapter_prev_text = "▕◁"
local bookmark_next_text = "☆▷"
local bookmark_prev_text = "◁☆"
local bookmark_enabled_text = "★"
@ -223,7 +223,6 @@ function SkimToWidget:init()
enabled = true,
width = button_inner_width,
show_parent = self,
vsync = true,
callback = function()
self.ui:handleEvent(Event:new("ToggleBookmark"))
self:update()

8
frontend/ui/widget/touchmenu.lua
Unescape Escape View File

@ -173,6 +173,7 @@ function TouchMenuItem:onTapSelect(arg, ges)
UIManager:setDirty(nil, "fast", highlight_dimen)
UIManager:forceRePaint()
UIManager:yieldToEPDC()
-- Unhighlight
--
@ -215,12 +216,7 @@ function TouchMenuItem:onHoldSelect(arg, ges)
UIManager:setDirty(nil, "fast", highlight_dimen)
UIManager:forceRePaint()
-- NOTE: These very specific circumstances appear to reliably upset the EPDC,
-- causing a mild variant of our racey friend the papercut refresh glitch ;).
-- As it appears to stem from the race between *this* refresh for the highlight and the following writes to the fb,
-- let the kernel take a breather. It'll yield back to us when it's done.
-- Expect it to block for ~150 to 350ms. Given the context (a hold gesture), we can absorb the latency hit mostly unnnoticed.
UIManager:waitForVSync()
UIManager:yieldToEPDC()
-- Unhighlight
--

58
spec/unit/uimanager_spec.lua
Unescape Escape View File

@ -15,11 +15,11 @@ describe("UIManager spec", function()
local future2 = {future[1] + 5, future[2]}
UIManager:quit()
UIManager._task_queue = {
{ time = {now[1] - 10, now[2] }, action = noop },
{ time = {now[1], now[2] - 5 }, action = noop },
{ time = now, action = noop },
{ time = future, action = noop },
{ time = future2, action = noop },
{ time = {now[1] - 10, now[2] }, action = noop, args = {}, argc = 0 },
{ time = {now[1], now[2] - 5 }, action = noop, args = {}, argc = 0 },
{ time = now, action = noop, args = {}, argc = 0 },
{ time = future, action = noop, args = {}, argc = 0 },
{ time = future2, action = noop, args = {}, argc = 0 },
}
UIManager:_checkTasks()
assert.are.same(2, #UIManager._task_queue, 2)
@ -32,11 +32,11 @@ describe("UIManager spec", function()
local future = { now[1] + 60000, now[2] }
UIManager:quit()
UIManager._task_queue = {
{ time = {now[1] - 10, now[2] }, action = noop },
{ time = {now[1], now[2] - 5 }, action = noop },
{ time = now, action = noop },
{ time = future, action = noop },
{ time = {future[1] + 5, future[2]}, action = noop },
{ time = {now[1] - 10, now[2] }, action = noop, args = {}, argc = 0 },
{ time = {now[1], now[2] - 5 }, action = noop, args = {}, argc = 0 },
{ time = now, action = noop, args = {}, argc = 0 },
{ time = future, action = noop, args = {}, argc = 0 },
{ time = {future[1] + 5, future[2]}, action = noop, args = {}, argc = 0 },
}
wait_until, now = UIManager:_checkTasks()
assert.are.same(future, wait_until)
@ -46,9 +46,9 @@ describe("UIManager spec", function()
now = { util.gettime() }
UIManager:quit()
UIManager._task_queue = {
{ time = {now[1] - 10, now[2] }, action = noop },
{ time = {now[1], now[2] - 5 }, action = noop },
{ time = now, action = noop },
{ time = {now[1] - 10, now[2] }, action = noop, args = {}, argc = 0 },
{ time = {now[1], now[2] - 5 }, action = noop, args = {}, argc = 0 },
{ time = now, action = noop, args = {}, argc = 0 },
}
wait_until, now = UIManager:_checkTasks()
assert.are.same(nil, wait_until)
@ -69,7 +69,7 @@ describe("UIManager spec", function()
local future = { now[1]+10000, now[2] }
UIManager:quit()
UIManager._task_queue = {
{ time = future, action = '1' },
{ time = future, action = '1', args = {}, argc = 0 },
}
assert.are.same(1, #UIManager._task_queue)
UIManager:scheduleIn(150, 'quz')
@ -78,7 +78,7 @@ describe("UIManager spec", function()
UIManager:quit()
UIManager._task_queue = {
{ time = now, action = '1' },
{ time = now, action = '1', args = {}, argc = 0 },
}
assert.are.same(1, #UIManager._task_queue)
UIManager:scheduleIn(150, 'foo')
@ -93,9 +93,9 @@ describe("UIManager spec", function()
now = { util.gettime() }
UIManager:quit()
UIManager._task_queue = {
{ time = {now[1] - 10, now[2] }, action = '1' },
{ time = {now[1], now[2] - 5 }, action = '2' },
{ time = now, action = '3' },
{ time = {now[1] - 10, now[2] }, action = '1', args = {}, argc = 0 },
{ time = {now[1], now[2] - 5 }, action = '2', args = {}, argc = 0 },
{ time = now, action = '3', args = {}, argc = 0 },
}
-- insert into the tail slot
UIManager:scheduleIn(10, 'foo')
@ -118,17 +118,17 @@ describe("UIManager spec", function()
now = { util.gettime() }
UIManager:quit()
UIManager._task_queue = {
{ time = {now[1] - 15, now[2] }, action = '3' },
{ time = {now[1] - 10, now[2] }, action = '1' },
{ time = {now[1], now[2] - 6 }, action = '3' },
{ time = {now[1], now[2] - 5 }, action = '2' },
{ time = now, action = '3' },
{ time = {now[1] - 15, now[2] }, action = '3', args = {}, argc = 0 },
{ time = {now[1] - 10, now[2] }, action = '1', args = {}, argc = 0 },
{ time = {now[1], now[2] - 6 }, action = '3', args = {}, argc = 0 },
{ time = {now[1], now[2] - 5 }, action = '2', args = {}, argc = 0 },
{ time = now, action = '3', args = {}, argc = 0 },
}
-- insert into the tail slot
UIManager:unschedule('3')
assert.are.same({
{ time = {now[1] - 10, now[2] }, action = '1' },
{ time = {now[1], now[2] - 5 }, action = '2' },
{ time = {now[1] - 10, now[2] }, action = '1', args = {}, argc = 0 },
{ time = {now[1], now[2] - 5 }, action = '2', args = {}, argc = 0 },
}, UIManager._task_queue)
end)
@ -140,15 +140,17 @@ describe("UIManager spec", function()
end
UIManager:quit()
UIManager._task_queue = {
{ time = { now[1], now[2]-5 }, action = task_to_remove },
{ time = { now[1], now[2]-5 }, action = task_to_remove, args = {}, argc = 0 },
{
time = { now[1]-10, now[2] },
action = function()
run_count = run_count + 1
UIManager:unschedule(task_to_remove)
end
end,
args = {},
argc = 0
},
{ time = now, action = task_to_remove },
{ time = now, action = task_to_remove, args = {}, argc = 0 },
}
UIManager:_checkTasks()
assert.are.same(2, run_count)

Write Preview
Loading…
Cancel
Save