Revamp "flash_ui" handling #7118
Conversation
That currently involves very naughty hacks :D.
Went mostly unnoticed until these experiments, because few things besides the paging chevrons use it.
Otherwise, the duplicates pollute the widget, and as they have a tap handler, will respond to it, which caused, among possibly a host of other things, visible double highlights with the tighter timings. The whole "update" behavior could probably use a once over, because it looks hella gross.
(Re-opening the FM would hide/show one too many time, the wrong way, which meant we lost the bg).
It does more hard than good, and is only actually necessary when there's a reagl refresh in the mix
Forgot that we weren't fencing REAGL updates on Mk. 7 ;).
We need a fixed background, or inversion does wonky things (it's a simple invertRect post-paint, and text is always black. So if you paint black text on a nil background that happens to be black just because of the previous invertRect, you get a fully black rectangle ;p).
Again, it's because invert does a plain invertRect, and that test is black, and that we have non rectangular shapes mixed in...
Otherwise the inverted state would have to take care of the borders, too.
on a button Helpful for the SkimTo widget, when you cross a flashing border, to avoid waiting for the highlight itself, which would delay the flash (e.g., callback).
unschedule the delayed show. Prevent spurious refreshes with bogus dimensions.
show_parent is accurate
heuristic to actually check for covers_fullscreen And flag ReaderProgress as covers_fullscreen, because it does ;p
|
Depends on koreader/koreader-base#1283 |
|
OK, I guess this happened only with the case that I didn't think about: where there's a timeout, and it's the timeout that is closing it, right ? (I assumed only dismiss was closing it...)
May be, dunno :) The only thing I see is limiting the size of the icons - but I thought this issue was cause by the Font size widget with its increasing size - so may be it wasn't it :) We'll see on various geometry/devices. Thanks for all the work (haven't look at each in details, but it indeed often felt like a mess the re-init of many little things on a simple update. |
I actually see a differences with PDFs:
|
Yep, IIRC, that's the exact flow of things ;). (In practice, you ended up with a full-screen "ui" refresh a short time after the actual lookup popup was shown, which ended up being mostly invisible unless you happened to be hunting for weird refreshes ;).) |
Gah, PDF >_<". |
Weird remnant from ye olde' times: koreader@607c4ba
Remove this, and PDF will be like EPUBs, so all good I think :) koreader/frontend/ui/data/koptoptions.lua Line 323 in 1c9c40f |
|
@poire-z: Great minds ;p. |
Yeah, I didn't try too hard, mostly keeping to Button & IconButton. Hopefully haven't broken anything, because those buttons should stay put (in fact, Menu already handles the hide/show dance mostly properly). On a related note, good first candidates for a good spring cleaning would perhaps be KeyValuePage or Menu, as they're both generally much less insane than ConfigDialog to get into ^^. |
|
The upside is, if anything's broken, results should be (mostly) consistent between the emu and actual eInk devices ;). |
* I'd failed to notice that ButtonTable *also* instantiates seven billion Buttons on each update. Unfortunately, that one is way trickier to fix properly, so, work around its behavior in Button. (This fixes multiple issues with stuff using ButtonTable, which is basically anything with a persistent set of buttons. A good and easy test-case is the dictionary popup, e.g., the Highlight button changes text, and the next/prev dic buttons change state. All that, and more, was broken ;p). * Handle corner-cases related to VirtualKeyboard (e.g., Terminal & Text Editor), which screwed with both TouchMenu & Button heuristics because it's weird. * Flag a the dictionary switch buttons as vsync (They trigger a partial repaint of the dictionary content). * Flag the ReaderSearch buttons as vsync They very obviously trigger a partial repaint, much like SkimTo ;p.
…ssible (#7166) * QuickDictLookup, ImageViewer, NumberPicker: Smarter `update` that will re-use most of the widget's layout instead of re-instantiating all the things. * SpinWidget/DoubleSpinWidget: The NumberPicker change above renders a hack to preserve alpha on these widgets almost unnecessary. Also fixed said hack to also apply to the center, value button. * Button: Don't re-instantiate the frame in setText/setIcon when unnecessary (e.g., no change at all, or no layout change). * Button: Add a refresh method that repaints and refreshes a *specific* Button (provided it's been painted once) all on its lonesome. * ConfigDialog: Free everything that's going to be re-instatiated on update * A few more post #7118 fixes: * SkimTo: Always flag the chapter nav buttons as vsync * Button: Fix the highlight on rounded buttons when vsync is enabled (e.g., it's now entirely visible, instead of showing a weird inverted corner glitch). * Some more heuristic tweaks in Menu/TouchMenu/Button/IconButton * ButtonTable: fix the annoying rounding issue I'd noticed in #7054 ;). * Enable dithering in TextBoxWidget (e.g., in the Wikipedia full view). This involved moving the HW dithering align fixup to base, where it always ought to have been ;). * Switch a few widgets that were using "partial" on close to "ui", or, more rarely, "flashui". The intent being to limit "partial" purely to the Reader, because it has a latency cost when mixed with other refreshes, which happens often enough in UI ;). * Minor documentation tweaks around UIManager's `setDirty` to reflect that change. * ReaderFooter: Force a footer repaint on resume if it is visible (otherwise, just update it). * ReaderBookmark: In the same vein, don't repaint an invisible footer on bookmark count changes.
So, err, this started as a simple investigation into why the SkimTo widget was so weirdly slow on Mk. 7 (spoiler: koreader/koreader-base@926c294), and it turned into a weird revamp of how flash_ui highlights are handled ;p.
In short:
This revealed a fun amount of bugs and weird quirks, from widgets being completely reinstanciated on each page (hi, ConfigDialog! So glad you can munch on almost 30MB of RAM all on your lonesome!), to only Buttons being reinstanciated a billion times, which caused ghost widgets in the event queue, meaning double highlights ;).
KeyValuePage did some weird things with its Return arrow Button, too. (And highlights were generally already broken there, especially visible in the Dictionary/Wikipedia lookup pages).
I'm not super-proud of the "UIManager:getPreviousRefreshRegion" hack, but it turned out to be mildly useful ;p.
@poire-z: There was a minor quirk with the recently implemented show_delay on IM, where the scheduled show wasn't cleared on close, which caused a stray refresh on the full screen.
This change is