baskerville / plato

Document reader
Other
1.23k stars 102 forks source link

[Bug] After waking from sleep, 'Invert Colors' setting is ignored #343

Open YodaDaCoda opened 7 months ago

YodaDaCoda commented 7 months ago
  1. Enable 'Invert Colors' setting in menu
  2. Wait for the ereader to go to sleep
  3. Wake the ereader up, and note that the colours are not inverted

NB: only applies if the ereader goes to sleep based on timeout, not if i put it to sleep using the switch or the magnetic case.

Workaround: Toggle 'Invert Colors' off and back on

Device: Kobo Glo (N613)

Version: Plato 0.9.40

NiLuJe commented 7 months ago

It's likely a hardware/kernel bug on those old devices.

YodaDaCoda commented 7 months ago

Does plato just toggle a system setting that inverts the colours then?

I enabled the 'Invert Screen' setting in the developer settings of the device, then tested with and without Plato's 'Invert Colors' settings enabled, and it still returns to 'day mode' after waking from sleep. The Kobo UI retains the dark mode after waking.

Does Plato have a hook that can ensure the 'Invert Colors' setting is re-applied upon wake? Would be happy to have a go at it myself if someone can point me in the right direction.

NiLuJe commented 7 months ago

IIRC (it varied, and the name changed at one point), Nickel's invert thingy does everything in software. It is as such blissfully ignorant of hardware/driver quirks.

What's toggled by Plato (and KOReader, for that matter, except on a few specific devices where it's extra broken or plain not available) is instead a driver flag that lets the display driver deal with it. We have no way of knowing when and if it breaks, we're basically just passing an extra flag to refresh requests saying "please invert the buffer on flip". What happens after that is out of our hands. ;).

IIRC, Plato doesn't have a software fallback, and just disables the feature on the aforementioned "broken/unavailable" devices.

YodaDaCoda commented 7 months ago

Interesting. I added some logging to the update() method in kobo1.rs and found the flag is being passed as expected during sleep/wake, so the problem must be in the driver.

As I see it there's 3 options...

  1. Un-invert the frame immediately before entering sleep, with the expectation that upon wake the setting can take hold and re-invert the screen
  2. Un-invert and re-invert the screen upon wake
  3. Accept it and move on

I've been trying to tackle option 1, but I'm not particularly familiar with the codebase or Rust in general so I'm not sure I'm headed down the right track with this.

diff --git a/crates/plato/src/app.rs b/crates/plato/src/app.rs
index bf3138a..5628302 100644
--- a/crates/plato/src/app.rs
+++ b/crates/plato/src/app.rs
@@ -590,6 +590,18 @@ pub fn run() -> Result<(), Error> {
                     context.frontlight.set_intensity(0.0);
                     context.frontlight.set_warmth(0.0);
                 }
+
+                // workaround for dodgy displaydriver on older devices
+                if CURRENT_DEVICE.mark() < 11 {
+                    if context.settings.inverted {
+                        context.fb.toggle_inverted();
+                        let result = context.fb.update(&context.fb.rect(), UpdateMode::Full);
+                        if let Ok(token) = result {
+                            context.fb.wait(token).ok();
+                        }
+                    }
+                }
+
                 if context.settings.wifi {
                     Command::new("scripts/wifi-disable.sh")
                             .status()
NiLuJe commented 7 months ago

You're assuming the driver reliably breaks on wakeup, which is an assumption I wouldn't be entirely willing to make ;). (And as another nitpick, that Mk. sweep is waaaay too wide).

Also, my experience with flag bugs on newer devices is that the driver doesn't rally give a crap if you try to tone done the broken flags for a while, if it wants to be broke, it's going to be broke ;p.

The simplest workaround is probably to forgo the PxP inversion and just invert the buffers in software during blit.

YodaDaCoda commented 6 months ago

I understood some of these words 😂 Not sure what 'blit' is, or what you mean by Mk. sweep, I assume it has something to do with passing UpdateMode::Full to fb.update() ?

On my device at least, the behaviour is consistent (I recognise that it might vary for other devices, even of the same model).

I've been combing through trying to find how I might go about inverting the data that's sent to the framebuffer device - I assume it's kept in memory somewhere, but damned if I can figure out where. The update_data that's sent to the send_update_v* methods seems to consist of various flags and such (dependant on version) but nothing that's standing out to me as data to be written. Any chance you can point me in the right direction?

NiLuJe commented 6 months ago

I'm not familiar with Plato's graphics backend, but I imagine there's a screen buffer somewhere (or a direct mmap to the hardware backed framebuffer).

You (probably) won't find it near the refresh ioctls though, because blitting and refreshing are two very separate things on eInk ;).

As for the Mk. sweep, I meant the generation check, i.e., if CURRENT_DEVICE.mark() < 11 {. I wouldn't expect this to affect anything >= Mk. 6. I'd even say Mk. 5, except the Aura is Mk. 5, and it is very broken there (to the point where it just crashes on older kernels).