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

UIManager: avoid painting widgets covered by a full screen widget #3770

Merged
merged 2 commits into from Mar 17, 2018

Conversation

Projects
None yet
2 participants
@poire-z
Contributor

poire-z commented Mar 17, 2018

Navigating the TOC, viewing a full screen image, browsing reading stats... would call paintTo() on ReaderUI (so, asking the engine to render the page) or FileManager (so, rendering cover images) even though their content is hidden.
Widgets registering to UIManager have to explicitely state they cover the full screen with covers_fullscreen = true (UIManager can't know, parts of their dimen may be transparent, e.g. if it is a CenterContainer, like an InfoMessage has dimension of the full screen).

This makes these things snappier.
(We can't do that for the top menu, as the book text is displayed below, and when changing menu items, the next one may be smaller, so book text has to be displayed - so, this still lags a bit by the same time taken by page turn).

I've been testing this only for a few minutes, and haven't seen side effects.
There is still the business of calling the refreshes (refresh_func, refresh_stack) that I don't know if this may mess with them. I guess not as it's the late widgets (so, on top and displayed) that can only be registering some refreshes/setDirty , but...

I've set covers_fullscreen = true to ReaderUI and FileManager, because some time ago, i've seen a FileManager cover image being transiently redrawn when changing fonts on a Reader! I can't reproduce it now, and logs don't show this could happen.. So it may have been fixed.
I also set it to Screensaver hoping it could solve #3130 (comment) - but I doubt it: this issue is with refreshes - avoiding some painting in a single refresh would change nothing - so I may remove them as they make screensaver.lua a bit ugly.

UIManager: avoid painting widgets covered by a full screen widget
Navigating the TOC, viewing a full screen image, browsing
reading stats... would call paintTo() on ReaderUI (so, asking
the engine to render the page) or FileManager (so, rendering cover
images) even though their content is hidden.
Widgets registering to UIManager have to explicitely states
they cover the full screen (UIManager can't know, parts of their
dimen may be transparent, e.g. if it is a CenterContainer).
@Frenzie

This comment has been minimized.

Member

Frenzie commented Mar 17, 2018

ui/widget/bookstatuswidget
ui/widget/opdsbrowser

@Frenzie

lgtm

@poire-z

This comment has been minimized.

Contributor

poire-z commented Mar 17, 2018

Added.
Also simplified Screensaver. I think it may help: when going into screensaver, there are a bunch of scheduleIn/nextTick, so it may be that the page is rendered and has the time to be painted to framebuffer, before the screensaver is later painted. With this, the screensaver may be painted quicker, and may be they'll fly in the same refresh (haven't investigated that much, just some unverified thoughts).

I managed to see FileManager (not being painted thanks to this) while in ReaderUI: when you Open last document, the FileManager instance seems to stay behind the ReaderUI. So, this PR may prevent the FileManager painting I talked about above, and avoid this glitch.

@poire-z poire-z merged commit 5e47a83 into koreader:master Mar 17, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@poire-z poire-z deleted the poire-z:uiman_repaint branch Mar 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment