koreader-base icon indicating copy to clipboard operation
koreader-base copied to clipboard

Use inkview to adjust image colors to look more bright

Open rjd22 opened this issue 2 years ago • 8 comments

Needs: https://github.com/koreader/koreader/pull/9143 Fixes: #8872

Adjust image colors for pocketbook color devices to supply brighter image colors. Make it a config option to we can adjust it to a sensible default for all devices later.


This change is Reviewable

rjd22 avatar May 29 '22 11:05 rjd22

@NiLuJe I think we can improve the performance a lot if we could detect if the image in the framebuffer is a grayscale image or contains colors. This way we could make it so it doesn't apply the filters when there are no colors.

rjd22 avatar May 29 '22 11:05 rjd22

At minima this needs to be disabled altogether if the target BB is > 8bpp (check self.bb:getType(); or make pb_fb.depth public).

EDIT: Which it does, via devcaps, my bad ;p.


As for your actual query, that's... technically feasible, but it'd probably be better if the engine told us about it. PicDocument knows at decode time (amount of color components/channels in the source image), and would probably be the easiest to tweak to forward that information. Both CRe and MµPDF should be able to get that info at decode time, too, but it might be gnarlier to forward the info.

Otherwise, you're left with a "post processing approach", e.g., sample an 8x8 tile in the center of the fb and see if it averages to a color pixel (or just if a majority of those pixels individually have different R/G/B values).

NiLuJe avatar May 29 '22 13:05 NiLuJe

Random IV query: what's the difference between the two functions?

NiLuJe avatar May 29 '22 13:05 NiLuJe

You should also hook into the hardware dithering flag mechanism to see if there's image content at all on the screen, too.

(c.f., the dither argument to the refresh*Imp functions).

NiLuJe avatar May 29 '22 13:05 NiLuJe

Random IV query: what's the difference between the two functions?

You can find some more info here. I might also implement the other methods later:

https://github.com/koreader/koreader/issues/8872#issuecomment-1096624979

rjd22 avatar May 29 '22 13:05 rjd22

You should also hook into the hardware dithering flag mechanism to see if there's image content at all on the screen, too.

(c.f., the dither argument to the refresh*Imp functions).

I've tried only adjusting colors with if dither then. But it looks like this flag is not always true when an image is shown. Might this be possible of am I doing something wrong?

Edit: After adding the dither flag to the debug is seems like the dither flag is not send at all. Is there a flag somewhere I need to enable?:

code:

local function _updatePartial(fb, x, y, w, h, dither)
    x, y, w, h = _getPhysicalRect(fb, x, y, w, h)

    fb.debug("refresh: inkview partial", x, y, w, h, dither)

    if dither then
        _adjustAreaColours(fb)
    end

    inkview.PartialUpdate(x, y, w, h)
end

function framebuffer:refreshPartialImp(x, y, w, h, d)
    self.debug("refresh: dither", d)

    _updatePartial(self, x, y, w, h, d)
end

log:

07/05/22-15:10:52 DEBUG refresh: dither
07/05/22-15:10:52 DEBUG refresh: inkview partial 0 0 1404 1872

rjd22 avatar Jul 05 '22 12:07 rjd22

It might contingent on running at 8bpp, which obviously doesn't work here ;p. I'll double-check later ;).

NiLuJe avatar Jul 05 '22 16:07 NiLuJe

Okay, no, but some of the logic is hidden away behind the canHWDither Device cap, so try to enable that ;).

NiLuJe avatar Jul 05 '22 19:07 NiLuJe

@NiLuJe I've tested it with the flag canHWDither but it didn't work completely,

Most worked correctly but when I was viewing an jpeg image dither was false causing the saturation not to trigger. For that reason I think this can be merged as is. I am using a different color method now that seem to have a more natural result.

I'm keeping the dither variables in there for now. Maybe we can solve this in the future.

rjd22 avatar Nov 07 '22 13:11 rjd22

Most worked correctly but when I was viewing an jpeg image dither was false causing the saturation not to trigger.

Viewing a JPEG how?

Both PicDocument & ImageViewer enforce dithering, the only place where it's optional is in a PDF/CBZ, where users have to toggle the option manually in the bottom menu (because not all PDFs are image-centric).

NiLuJe avatar Nov 07 '22 14:11 NiLuJe

@NiLuJe by opening the jpeg directly from the file menu. I don't know if PicDocument & ImageViewer is used in that case.

Edit: Thank you for the extra input. I indeed had to enable hardware dithering manually when viewing the images. I will update the PR now.

rjd22 avatar Nov 07 '22 17:11 rjd22

Good to go ? Approved https://github.com/koreader/koreader/pull/9756 is waiting for it.

poire-z avatar Nov 11 '22 22:11 poire-z

Not yet. I might make the requested patch somewhere on Monday.

rjd22 avatar Nov 12 '22 07:11 rjd22

This should be good to go now :)

rjd22 avatar Nov 14 '22 11:11 rjd22

Looks like this doesn't apply to the book cover rendering at the beginning of each file. It does apply to the illustrations in the file itself.

I'm not familiar with KOReader source code, can you please check my finding?

dmalinovsky avatar Apr 06 '23 14:04 dmalinovsky

It's done on every drawing call so that seems unlikely, but what do you mean by the book cover rendering at the beginning of each file?

Frenzie avatar Apr 06 '23 15:04 Frenzie

I mean if you open any unread book with a cover, the first thing you see is its cover. Then you click next page and see the book contents.

dmalinovsky avatar Apr 06 '23 15:04 dmalinovsky

Unless that cover is extremely small, making the area check fail, this shouldn't be the case.

Verbose debug logs would be helpful ;).

NiLuJe avatar Apr 06 '23 15:04 NiLuJe

Do you still see the same effect if you go back from the second page, or when you force a screen refresh with a long diagonal swipe?

According to the code here you should be able to see whether the function is called by grepping for adjusting image color saturation, if you enable debug logging and you're able to tail the log via SSH or something. Or after the fact of course, but that's a little more awkward. :-)

If you don't see that for the screen update something strange may be slightly off with the dither flag, but that's rather unlikely; if you do then it's something with linkinkview that can't be worked around (except by forcing two refreshes on open, or something awkward like that), but that too seems unlikely.

Frenzie avatar Apr 06 '23 15:04 Frenzie

Verbose debug logs would be helpful ;).

How do I get them? Sorry for newbie question, but search in the Wiki and quick guide revealed nothing.

Do you still see the same effect if you go back from the second page, or when you force a screen refresh with a long diagonal swipe?

Yes, it's the same.

dmalinovsky avatar Apr 06 '23 16:04 dmalinovsky

The simplest way is to enable them through the file browser → tools → developer options.

Frenzie avatar Apr 06 '23 16:04 Frenzie

Okay, here's the verbose debug log. I didn't find "saturation" word in it. It starts from the moment the file is opened, and KOReader displays the cover after opening the file:

crash.log

dmalinovsky avatar Apr 06 '23 16:04 dmalinovsky

These lines seem to be relevant:

04/06/23-12:52:15 DEBUG _refresh: Enqueued partial update for region 0 0 1404 1872 dithering: false
04/06/23-12:52:15 DEBUG setDirty partial from widget ReaderUI w/ NO region; dithering: nil
04/06/23-12:52:15 DEBUG ReaderStatistics: No timestamp for previous page 0
04/06/23-12:52:15 DEBUG CoverImage: onReaderReady
04/06/23-12:52:15 DEBUG computing and storing partial_md5_checksum
04/06/23-12:52:15 DEBUG _refresh: Enqueued partial update for region 0 0 1404 1872 dithering: false

I see that dithering is false.

dmalinovsky avatar Apr 06 '23 17:04 dmalinovsky

The good news (in a manner of speaking) is that none of the fb.debug() calls seem to show up in the log even though they should.

I'll leave the log interpretation to @NiLuJe who knows much more about the dithering stuff.

Frenzie avatar Apr 06 '23 17:04 Frenzie

The good news (in a manner of speaking) is that none of the fb.debug() calls seem to show up in the log even though they should.

That requires a retart after toggling verbose debug ;).

NiLuJe avatar Apr 06 '23 18:04 NiLuJe

These lines seem to be relevant:

Full context, please :}.

(Also, check if it's specific to a specific book, and, if it is, post a scrambled version so we can reproduce).

NiLuJe avatar Apr 06 '23 18:04 NiLuJe

That requires a retart after toggling verbose debug ;).

My apologies, here's the updated file: crash.log

(Also, check if it's specific to a specific book, and, if it is, post a scrambled version so we can reproduce).

It happens with any book I've tried to open.

dmalinovsky avatar Apr 06 '23 18:04 dmalinovsky

Ah, FB2. I honestly don't recall if the image detection thingy is even supposed to work in those. @poire-z?

NiLuJe avatar Apr 06 '23 22:04 NiLuJe

In the meantime (since @poire-z is on a well deserved vacation ;o)), check if EPUB works ;).

NiLuJe avatar Apr 06 '23 22:04 NiLuJe

It’s actually looking fine for ePub, unlike FB2.

On Thu, Apr 6, 2023 at 18:01 NiLuJe @.***> wrote:

In the meantime (since @poire-z https://github.com/poire-z is on a well deserved vacation ;o)), check if EPUB works ;).

— Reply to this email directly, view it on GitHub https://github.com/koreader/koreader-base/pull/1484#issuecomment-1499676245, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADWAHJCPHEQ4AABWKWZM2DW744KXANCNFSM5XIBCN7Q . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Denis Malinovsky

dmalinovsky avatar Apr 06 '23 22:04 dmalinovsky