baskerville / plato

Document reader
Other
1.3k stars 109 forks source link

Invert Colors and Make Bitonal glitches #79

Closed bricewge closed 4 years ago

bricewge commented 4 years ago

Enabling together "Invert Colors" and "Make Bitonal" glitches, only one half is bitonal and the other half has it's colors inverted. Taking a screenshot of it won't show the behaviour, you can see it in this video.

baskerville commented 4 years ago

I guess it might have to do with the fact that I'm using the legacy ioctl calls and you have a Mark 7 device. I'll provide a test binary. Side note: you can select a menu entry without closing the menu by taping and holding the entry.

baskerville commented 4 years ago

Could you try the attached binary?

plato-0.8.3-test1.zip

bricewge commented 4 years ago

Nope, the glitch is still here.

baskerville commented 4 years ago

I'm afraid it might have to do with a firmware bug then. @NiLuJe: Can't EPDC_FLAG_ENABLE_INVERSION and EPDC_FLAG_FORCE_MONOCHROME work together on the Forma?

NiLuJe commented 4 years ago

IIRC, it can, but there's a weird bug where the EPDC will sometimes ignore any postprocessing flags (any combination of flags and/or dithering_mode).

I've seen weird sporadic issues like that over the years, but it's much worse and somewhat reproducible on Mk. 7 devices, as the comment linked above shows ;).

It's been my experience that bitdepth and/or rotation play a large part in it.

That said, I've never seen it happen quite like the OP ^^.

@baskerville: Do you simply add the FORCE_MONOCHROME flag to whatever waveform mode ought to be used usually, or do you also force A2 or DU?

NiLuJe commented 4 years ago

At a very quick glance, I certainly can't reproduce that on Nickel (32bpp, Portrait (-> 3)), at least w/ A2. (And I've never seen it happen on other devices w/ DU, either).

TL;DR: That works:

21:39:57 [76ef81f6] ioctl(3, MXCFB_SEND_UPDATE, {update_region={top=0, left=0, width=1440, height=1920}, waveform_mode=NTX_WFM_MODE_A2, update_mode=UPDATE_MODE_PARTIAL, update_marker=2276, temp=TEMP_USE_AMBIENT, flags=EPDC_FLAG_ENABLE_INVERSION|EPDC_FLAG_FORCE_MONOCHROME, dither_mode=EPDC_FLAG_USE_DITHERING_PASSTHROUGH, quant_bit=0, alt_buffer_data={phys_addr=0, width=0, height=0, alt_update_region={top=0, left=0, width=0, height=0}}}, 0x7edaeb08) = 0

So does piling dithering on top of it, although you generally don't want to do that, because the monochrome quantization pass is applied first, rendering dithering useless ;).

baskerville commented 4 years ago

Do you simply add the FORCE_MONOCHROME flag to whatever waveform mode ought to be used usually, or do you also force A2 or DU?

The former.

NiLuJe commented 4 years ago

I'd try the latter then, I'd fully expect FORCE_MONOCHROME to be unreliable on anything that isn't A2 or DU ;).

(Also, technically useless there: the only reasonable reason to want to enforce FORCE_MONOCHROME is to benefit from faster bitonal waveform modes).

baskerville commented 4 years ago

I'd try the latter then, I fully expect FORCE_MONOCHROME to be unreliable on anything that isn't A2 or DU ;).

The former works on the Glo HD and the Aura ONE.

NiLuJe commented 4 years ago

Although, on that note, extra fun quirk on Mk.7: A2 is a tad too aggressive (and/or buggy ^^), and generally will behave weirdly (as in, not updating the non B&W bits) if the content to refresh isn't bitonal before FORCE_MONOCHROME.

Especially if it's drawn over complex content (i.e., drawing not-quite-bitonal stuff on top of an image, for instance).

NiLuJe commented 4 years ago

That possibly explains why some UIs have stopped using FORCE_MONOCHROME altogether, even for button highlights, and/or switched away from A2 to DU (or even AUTO, which will generally prefer DU over A2)).

(Especially on newer screens/EPDC, which have gotten better at dealing with stuff without needing micro-management like that ;p).

NiLuJe commented 4 years ago

TL;DR: The only combination that would provide reliable behavior on Mk. 7 for a global "Bitonal" switch that could apply to effectively any kind of content appears to be DU + EPDC_FLAG_USE_DITHERING_Y1. Which actually means no FORCE_MONOCHROME in practice, since it'd effectively murder the Y1 dithering ;).

(dither_mode ORDERED @ quant_bit 1 looks like crap unless you're at rota UR (and even then, it's not necessarily great), which happens to be landscape on the Forma, hence the older, software dithering).


Downside: That dithering flag is >= Mk. 6 on Kobo (but will be harmlessly ignored on earlier devices).

Also, YMMV on lower dpi devices, but it doesn't affect text rendering too much, which is a plus in my book ;).

(But, unlike dither_mode, it's not offloaded to the PxP, it's the kernel driver doing the work).

NiLuJe commented 4 years ago

Oh, duh. It's half and half in the video because the first broken refresh is regional since it's a menu popping off -_-". (c.f., also the clock region when it updates later).

So, yeah, I can definitely trigger the weird crap mentioned in the FBInk comment about postprocessing flags getting entirely dropped by the driver if I use EPDC_FLAG_ENABLE_INVERSION | EPDC_FLAG_FORCE_MONOCHROME on a refresh that's not using A2 or DU. Again, still not 100%, but that certainly works fairly consistently as a potential trigger.

(Note that nickel itself does use FORCE_MONOCHROME w/ A2 for menu highlights).


Also, fun fact: the mess I mentioned above about A2 on Mk.7? Turns out it's mostly FORCE_MONOCHROME's fault, too ;p.


So, yeah, FORCE_MONOCHROME is super finicky on Mk.7 ^^.

And this is definitely the same issue as mentioned in the FBInk comment.

baskerville commented 4 years ago

Although, on that note, extra fun quirk on Mk.7: A2 is a tad too aggressive (and/or buggy ^^), and generally will behave weirdly (as in, not updating the non B&W bits) if the content to refresh isn't bitonal before FORCE_MONOCHROME.

Is this also true for DU?

NiLuJe commented 4 years ago

Nope, see my latest edits in my previous messages ;).

On Tue, Feb 18, 2020, 10:24 Bastien Dejean notifications@github.com wrote:

Although, on that note, extra fun quirk on Mk.7: A2 is a tad too aggressive (and/or buggy ^^), and generally will behave weirdly (as in, not updating the non B&W bits) if the content to refresh isn't bitonal before FORCE_MONOCHROME.

Is this also true for DU?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/baskerville/plato/issues/79?email_source=notifications&email_token=AAA3KZQH3KZAAICOTV44YMDRDOSOFA5CNFSM4J2OSAIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMBGTOA#issuecomment-587360696, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA3KZVLMMKAF5SSNRHUJY3RDOSOFANCNFSM4J2OSAIA .

NiLuJe commented 4 years ago

That said, I can break DU if I pile on a bunch of different post-processing flags (FORCE_MONOCHROME, INVERSION, and dithering, via flags or dither_mode).

The straw that breaks the camel back appears to be FORCE_MONOCHROME here, as I can't make it go haywire without it.


In short, I stand by this conclusion ;).

NiLuJe commented 4 years ago

Note that my final test implies that it's still highly plausible that DU + FORCE_MONOCHROME might still randomly break in fun & unexpected ways ;).

It appears slightly harder to break than A2, but the offending party is FORCE_MONOCHROME, not A2 ;).

baskerville commented 4 years ago

Note that my final test implies that it's still highly plausible that DU + FORCE_MONOCHROME might still randomly break in fun & unexpected ways ;).

I've tried DU + Y1, but unfortunately, it seemed as slow as AUTO, and Make Bitonal is supposed to be very fast…

NiLuJe commented 4 years ago

Yeah, it's entirely done in-kernel by the CPU, and (besides iterating over each pixel) it involves (roughly) a scanline * 3 alloc, so it does make sense that performance would take a hit :/.

Oh, well :s.

NiLuJe commented 4 years ago

Okay, finally tried this out.

Two things:

TL;DR: FORCE_MONOCHROME is effectively unusable for that purpose on Mk. 7.

(The new ioctl setup works fine, though 👍 ;)).

NiLuJe commented 4 years ago

@baskerville: On an unrelated note, popping up the menu in the Reader yields a bunch of 2px high refreshes (the horizontal menu borders?).

[pid  7075] 15:49:06 [75eac1f6] ioctl(3, MXCFB_WAIT_FOR_UPDATE_COMPLETE, {update_marker=163, collision_test=0}, 0x7ef456a0) = 0
[pid  7075] 15:49:06 [75eac1f6] ioctl(3, MXCFB_SEND_UPDATE, {update_region={top=0, left=0, width=1440, height=119}, waveform_mode=WAVEFORM_MODE_AUTO, update_mode=UPDATE_MODE_PARTIAL, update_marker=165, temp=TEMP_USE_AMBIENT, flags=0, dither_mode=EPDC_FLAG_USE_DITHERING_PASSTHROUGH, quant_bit=0, alt_buffer_data={phys_addr=0, width=0, height=0, alt_update_region={top=0, left=0, width=0, height=0}}}, 0x7ef45798) = 0
[pid  7075] 15:49:06 [75eac1f6] ioctl(3, MXCFB_SEND_UPDATE, {update_region={top=119, left=0, width=1440, height=2}, waveform_mode=WAVEFORM_MODE_AUTO, update_mode=UPDATE_MODE_PARTIAL, update_marker=166, temp=TEMP_USE_AMBIENT, flags=0, dither_mode=EPDC_FLAG_USE_DITHERING_PASSTHROUGH, quant_bit=0, alt_buffer_data={phys_addr=0, width=0, height=0, alt_update_region={top=0, left=0, width=0, height=0}}}, 0x7ef45798) = 0
[pid  7075] 15:49:06 [75eac1f6] ioctl(3, MXCFB_WAIT_FOR_UPDATE_COMPLETE, {update_marker=164, collision_test=0}, 0x7ef45720) = 0
[pid  7075] 15:49:06 [75eac1f6] ioctl(3, MXCFB_SEND_UPDATE, {update_region={top=1487, left=0, width=1440, height=2}, waveform_mode=WAVEFORM_MODE_AUTO, update_mode=UPDATE_MODE_PARTIAL, update_marker=167, temp=TEMP_USE_AMBIENT, flags=0, dither_mode=EPDC_FLAG_USE_DITHERING_PASSTHROUGH, quant_bit=0, alt_buffer_data={phys_addr=0, width=0, height=0, alt_update_region={top=0, left=0, width=0, height=0}}}, 0x7ef45798) = 0
[pid  7075] 15:49:06 [75eac1f6] ioctl(3, MXCFB_SEND_UPDATE, {update_region={top=1489, left=0, width=1440, height=310}, waveform_mode=WAVEFORM_MODE_AUTO, update_mode=UPDATE_MODE_PARTIAL, update_marker=168, temp=TEMP_USE_AMBIENT, flags=0, dither_mode=EPDC_FLAG_USE_DITHERING_PASSTHROUGH, quant_bit=0, alt_buffer_data={phys_addr=0, width=0, height=0, alt_update_region={top=0, left=0, width=0, height=0}}}, 0x7ef45798) = 0
[pid  7075] 15:49:06 [75eac1f6] ioctl(3, MXCFB_SEND_UPDATE, {update_region={top=1799, left=0, width=1440, height=2}, waveform_mode=WAVEFORM_MODE_AUTO, update_mode=UPDATE_MODE_PARTIAL, update_marker=169, temp=TEMP_USE_AMBIENT, flags=0, dither_mode=EPDC_FLAG_USE_DITHERING_PASSTHROUGH, quant_bit=0, alt_buffer_data={phys_addr=0, width=0, height=0, alt_update_region={top=0, left=0, width=0, height=0}}}, 0x7ef45798) = 0
[pid  7075] 15:49:06 [75eac1f6] ioctl(3, MXCFB_SEND_UPDATE, {update_region={top=1801, left=0, width=1440, height=119}, waveform_mode=WAVEFORM_MODE_AUTO, update_mode=UPDATE_MODE_PARTIAL, update_marker=170, temp=TEMP_USE_AMBIENT, flags=0, dither_mode=EPDC_FLAG_USE_DITHERING_PASSTHROUGH, quant_bit=0, alt_buffer_data={phys_addr=0, width=0, height=0, alt_update_region={top=0, left=0, width=0, height=0}}}, 0x7ef45798) = 0

I have met devices which would deadlock on x1 or 1x refresh rects, I'd probably merge those in the menu rects themselves ;).

Aszarsha commented 4 years ago

FWIW, 48a588a did not solve the issue, I can still reproduce (on my Libra). More information: the part of the text that is de-inverted is exactly the height (but full width) of the burger menu, without the top bar which remains inverted.

Furthermore, and it might be a different issue (or not [*]) : if I change the page (epub viewer, with physical buttons without exiting the menu), the inversion is then... inverted (white background, black foreground) ; monochrome is kept (horrendously aliased rendering). The whole screen is de-inverted in that case, menu including.

When that happens (and maybe in other conditions [**]), and I let the device go into sleeping mode, the "sleeping screensaver" is messed up: the text is ok, but the icon does not appear and the book's text (that was at that position) is kept on the display. Maybe because the icon is not monochrome? See photo.

[*] Do you want me to issue another bug report? [**] This might also warrant another bug report, but it seems related to that one.

baskerville commented 4 years ago

I think the best approach here would be to disable the Make Bitonal menu entry on mark 7 devices (and disable Invert Colors on the Aura).

Aszarsha commented 4 years ago

What is the use of bitonal mode? To be sure on my Libra it is a bit faster, but not much, and depending on the font used and its size the text can be barely readable.

NiLuJe commented 4 years ago

@Aszarsha: * is essentially my first bullet point in the recap above, and ** is the second ;). So nothing new.


@baskerville: If you're not happy w/ Y1, probably, yeah.

The Aura issue might be fixed in the final kernel release, but it apparently did use to deadlock at one point.

(In the same vein, earlier kernels on some Mk. 5 models did not support it either, but without crashing ;p).

baskerville commented 4 years ago

What is the use of bitonal mode?

Invert Colors and Make Bitonal were added very early on as a mean to access the corresponding hardware flags, without worrying about their usefulness.

I almost never use Invert Colors. On the devices where Make Bitonal works, it can be used to skim through pages.

Aszarsha commented 4 years ago

it can be used to skim through pages.

Well on the Libra is certainly is too slow for that purpose, it is barely any faster. Maybe that is why Kobo implemented their skimming interface, which is quite nice.