Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KOReader freezes after predetermined time (Appimage) #9262

Closed
offset-torque opened this issue Jun 27, 2022 · 18 comments · Fixed by #9263
Closed

KOReader freezes after predetermined time (Appimage) #9262

offset-torque opened this issue Jun 27, 2022 · 18 comments · Fixed by #9263
Labels
AppImage Desktop Linux bug

Comments

@offset-torque
Copy link

  • KOReader version: koreader-appimage-x86_64-linux-gnu-v2022.06
  • Device: Linux PC - KDE - X11

Issue

KOReader freezes after:

  • 5 minutes if menu is open
06/27/22-16:56:39 INFO  Loading plugins from directory
06/27/22-17:11:40 INFO  Inhibiting user input
  • 15 minutes if menu is not open
06/27/22-18:10:11 INFO  Loading plugins from directory
06/27/22-18:25:12 INFO  Inhibiting user input

Exact times and the inhibition message makes me think that this might be intentional.
But this puts KOReader into an unresponsive state.
No mouse or keyboard input is registered.
Even the close button doesn't work.
It has to be killed from the task manager or terminal.

Steps to reproduce

  • Open KOReader
  • Don't click on anything for the durations mentioned above
@Frenzie Frenzie added bug AppImage Desktop Linux labels Jun 27, 2022
@Frenzie
Copy link
Member

Frenzie commented Jun 27, 2022

You don't see a little suspended message? (Not that you should in a regular desktop app, but it's probably something from the emulator.)

@NiLuJe
Copy link
Member

NiLuJe commented Jun 27, 2022

Verbose debug, please ;).

Because, yeah, that looks like AutoSuspend attempting a suspend, which should either not happen at all or do something visible in the form of the suspend screensaver as @Frenzie mentioned ;).

@NiLuJe
Copy link
Member

NiLuJe commented Jun 27, 2022

Even if you did see the screensaver, we're probably missing an SDL-specific keycode whitelisted to get out of the simulated suspend...

Thoughts?

NiLuJe added a commit to NiLuJe/koreader that referenced this issue Jun 27, 2022
@NiLuJe
Copy link
Member

NiLuJe commented Jun 27, 2022

Hmm, the SDL event handler is buried in the SDL implementation, it might just be easier to simply not void the SDL handler during suspend...

@Frenzie
Copy link
Member

Frenzie commented Jun 27, 2022

It used to be that performing an action canceled out suspend on the emulator. You or @zwim changed that recently for reasons that are unclear to me. ;-)

@Frenzie
Copy link
Member

Frenzie commented Jun 27, 2022

But I think F2 should still get out of it, probably?

@NiLuJe
Copy link
Member

NiLuJe commented Jun 27, 2022

Okay, yeah, the SDL event handler appears to be for, well, SDL-specific bits.

I'm hoping the rest goes through standard EV_KEY stuff, so, yeah, F2 might do the trick?

Regardless of that, nerfing that handler was probably not a great idea ;p.

(Waiting on verbose debug logs to confirm).

@offset-torque
Copy link
Author

You don't see a little suspended message?

Nope, it freezes as whatever on the screen at the moment of inhibition message, even the menu is visible

But I think F2 should still get out of it, probably?

Nope, nothing works including F2


Apparently it is not frozen internally.

After getting this message:
INFO Inhibiting user input

KOReader stays unresponsive while these keycodes are scrolling in the terminal:

Mouse input (trying to open menu)

DEBUG input event => type: 3 (EV_ABS), code: 57 (ABS_MT_TRACKING_ID), value: 0
DEBUG input event => type: 3 (EV_ABS), code: 53 (ABS_MT_POSITION_X), value: 169
DEBUG input event => type: 3 (EV_ABS), code: 54 (ABS_MT_POSITION_Y), value: 395
DEBUG input event => type: 0 (EV_SYN), code: 0 (SYN_REPORT), value: 0
DEBUG input event => type: 3 (EV_ABS), code: 57 (ABS_MT_TRACKING_ID), value: -1
DEBUG input event => type: 0 (EV_SYN), code: 0 (SYN_REPORT), value: 0
DEBUG input event => type: 83 (EV_SDL), code: 1027, value: cdata<struct SDL_MouseWheelEvent &>: 0x7eff386789d8

F2 (which is exactly same whether it is frozen or not)

DEBUG key event => code: 1073741883 (Power), value: 1
DEBUG AutoSuspend: onInputEvent
DEBUG AutoTurn: onInputEvent
DEBUG key event => code: 1073741883 (Power), value: 0
DEBUG AutoSuspend: onInputEvent
DEBUG AutoTurn: onInputEvent

Close button

DEBUG key event => code: 1073742050 (Alt), value: 1
DEBUG key event => code: 1073741885 (F4), value: 1

NiLuJe added a commit to NiLuJe/koreader that referenced this issue Jun 27, 2022
@NiLuJe
Copy link
Member

NiLuJe commented Jun 27, 2022

You don't see a little suspended message?

Nope, it freezes as whatever on the screen at the moment of inhibition message, even the menu is visible

That's... weird. Another issue, possibly, mildly related, but weird.

@yparitcher
Copy link
Member

I bumped into this today while trying to test suspend in the emulator.

I may have abused koreader last night to write a custom dynamic kindle screensaver ;) (Koreader is a better gui toolkit than FBInk)

related:
Does anyone mind if i implement WakeupManager for kindle? On supportsScreensaver and non legacy devices?

This would be by listening for powerd's wakeup & readyToSuspend events, and setting the rtc when in the readyToSuspend state via lipc.

@NiLuJe
Copy link
Member

NiLuJe commented Jun 28, 2022

Go for it ;).

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

@yparitcher
Not sure why we'd mind. :-)

@NiLuJe

Okay, yeah, the SDL event handler appears to be for, well, SDL-specific bits.

I'm not really sure what you mean by the SDL event handler. Are you talking about the part where I decided that handling some SDL events more intelligently in front made a lot more architectural sense than trying to do a bunch of pretend down in base?

@NiLuJe
Copy link
Member

NiLuJe commented Jun 28, 2022

Okay, yeah, the SDL event handler appears to be for, well, SDL-specific bits.

I'm not really sure what you mean by the SDL event handler. Are you talking about the part where I decided that handling some SDL events more intelligently in front made a lot more architectural sense than trying to do a bunch of pretend down in base?

Nah, I just misremembered how the whole SDL thing worked, as far as input event goes. There's much less SDL-specific bits for "pure" input than I originally thought (thanks to said base shenanigans), so the stuff in front is mostly not-really-input anymore ;).

Still, the point stands, might not be great to inhibit those, I'll check it out later ;).

EDIT: I'm talking about

handleSdlEv = function(device_input, ev)
local Geom = require("ui/geometry")
local UIManager = require("ui/uimanager")
-- SDL events can remain cdata but are almost completely transparent
local SDL_TEXTINPUT = 771
local SDL_MOUSEWHEEL = 1027
local SDL_MULTIGESTURE = 2050
local SDL_DROPFILE = 4096
local SDL_WINDOWEVENT_MOVED = 4
local SDL_WINDOWEVENT_RESIZED = 5
if ev.code == SDL_MOUSEWHEEL then
local scrolled_x = ev.value.x
local scrolled_y = ev.value.y
local up = 1
local down = -1
local pos = Geom:new{
x = 0,
y = 0,
w = 0, h = 0,
}
local fake_ges = {
ges = "pan",
distance = 200,
relative = {
x = 50*scrolled_x,
y = 100*scrolled_y,
},
pos = pos,
time = ev.time,
mousewheel_direction = scrolled_y,
}
local fake_ges_release = {
ges = "pan_release",
distance = fake_ges.distance,
relative = fake_ges.relative,
pos = pos,
time = ev.time,
from_mousewheel = true,
}
local fake_pan_ev = Event:new("Pan", nil, fake_ges)
local fake_release_ev = Event:new("Gesture", fake_ges_release)
if scrolled_y == down then
fake_ges.direction = "north"
UIManager:broadcastEvent(fake_pan_ev)
UIManager:broadcastEvent(fake_release_ev)
elseif scrolled_y == up then
fake_ges.direction = "south"
UIManager:broadcastEvent(fake_pan_ev)
UIManager:broadcastEvent(fake_release_ev)
end
elseif ev.code == SDL_MULTIGESTURE then
-- no-op for now
do end -- luacheck: ignore 541
elseif ev.code == SDL_DROPFILE then
local dropped_file_path = ev.value
if dropped_file_path and dropped_file_path ~= "" then
local ReaderUI = require("apps/reader/readerui")
ReaderUI:doShowReader(dropped_file_path)
end
elseif ev.code == SDL_WINDOWEVENT_RESIZED then
device_input.device.screen.screen_size.w = ev.value.data1
device_input.device.screen.screen_size.h = ev.value.data2
device_input.device.screen.resize(device_input.device.screen, ev.value.data1, ev.value.data2)
self.window.width = ev.value.data1
self.window.height = ev.value.data2
local new_size = device_input.device.screen:getSize()
logger.dbg("Resizing screen to", new_size)
-- try to catch as many flies as we can
-- this means we can't just return one ScreenResize or SetDimensons event
UIManager:broadcastEvent(Event:new("SetDimensions", new_size))
UIManager:broadcastEvent(Event:new("ScreenResize", new_size))
--- @todo Toggle this elsewhere based on ScreenResize?
-- this triggers paged media like PDF and DjVu to redraw
-- CreDocument doesn't need it
UIManager:broadcastEvent(Event:new("RedrawCurrentPage"))
local FileManager = require("apps/filemanager/filemanager")
if FileManager.instance then
FileManager.instance:reinit(FileManager.instance.path,
FileManager.instance.focused_file)
end
elseif ev.code == SDL_WINDOWEVENT_MOVED then
self.window.left = ev.value.data1
self.window.top = ev.value.data2
elseif ev.code == SDL_TEXTINPUT then
UIManager:sendEvent(Event:new("TextInput", tostring(ev.value)))
end
end,
in the context of Input:inhibitInput ;).

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

I wrote that, yes. But the way "suspend" is supposed to work on the emulator in my view (not on regular desktop but presumably no one ever changed it) is that it just shows a little popup or whatever UI saying hello, we're in suspend status, and basically any input cancels it. That suffices for the UI and you can't develop the suspend without real hardware anyway.

There's much less SDL-specific bits for "pure" input than I originally thought (thanks to said base shenanigans), so the stuff in front is mostly not-really-input anymore ;).

It depends a bit on the specifics what makes sense where but I may well intend to lift more handling stuff to front 'cause in base you're very limited to a certain kind of emulation instead of directly tapping into things. Which was perfectly acceptable for the original emulator concept of course. ;-)

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

Anyway, so this line should say isEmulator instead of isSDL; do we have any other sneaky leftovers like that?

not Device:isSDL() and

@NiLuJe
Copy link
Member

NiLuJe commented Jun 28, 2022

Great minds think alike, I've just cleaned that up in #9263 ;). (EDIT: hadn't pushed yet, done).

(And I'm looking into how the Emu behaves re: SS/suspend right now)

@NiLuJe
Copy link
Member

NiLuJe commented Jun 28, 2022

Okay, it now works as it ought to with #9263 applied over here.

I'm definitely seeing the screensaver, whether I trip it via Exit > Sleep, or via AutoSuspend. (And I don't think I fixed anything on that front...).

emu_sleep

@Frenzie
Copy link
Member

Frenzie commented Jun 28, 2022

Yeah, same for me on current master & stable, except you can't get out of it.

NiLuJe added a commit that referenced this issue Jul 2, 2022
* AutoSuspend: Use the canSuspend devcap check instead of reinventing the wheel.
* Device & UIManager: Cleanup canSuspend devcap check related stuff to avoid boilerplate code.
  (It also now defaults to no, and is explicitly set by device implementations where supported).
* AutoSuspend: Re-engage suspend/shutdown timers when fully charged.
  This restores the existing behavior pre #9036
  (c.f., #9258 (comment))
* SDL: Unbreak the fake suspend behavior so that it actually works.
  Tweak the default screensaver message to remind users that Power is bound to F2.
  (Fix #9262)
* AutoSuspend: Re-engage suspend/shutdown timers on unplug.
  This matters on Kobo, because the unexpected wakeup guard might have stopped the suspend timer.
rjd22 pushed a commit that referenced this issue Nov 7, 2022
* AutoSuspend: Use the canSuspend devcap check instead of reinventing the wheel.
* Device & UIManager: Cleanup canSuspend devcap check related stuff to avoid boilerplate code.
  (It also now defaults to no, and is explicitly set by device implementations where supported).
* AutoSuspend: Re-engage suspend/shutdown timers when fully charged.
  This restores the existing behavior pre #9036
  (c.f., #9258 (comment))
* SDL: Unbreak the fake suspend behavior so that it actually works.
  Tweak the default screensaver message to remind users that Power is bound to F2.
  (Fix #9262)
* AutoSuspend: Re-engage suspend/shutdown timers on unplug.
  This matters on Kobo, because the unexpected wakeup guard might have stopped the suspend timer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AppImage Desktop Linux bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants