koreader-base
koreader-base copied to clipboard
Use inkview to adjust image colors to look more bright
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.
@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.
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).
Random IV query: what's the difference between the two functions?
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).
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
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 therefresh*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
It might contingent on running at 8bpp, which obviously doesn't work here ;p. I'll double-check later ;).
Okay, no, but some of the logic is hidden away behind the canHWDither
Device cap, so try to enable that ;).
@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.
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 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.
Good to go ? Approved https://github.com/koreader/koreader/pull/9756 is waiting for it.
Not yet. I might make the requested patch somewhere on Monday.
This should be good to go now :)
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?
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?
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.
Unless that cover is extremely small, making the area check fail, this shouldn't be the case.
Verbose debug logs would be helpful ;).
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.
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.
The simplest way is to enable them through the file browser → tools → developer options.
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:
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.
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.
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 ;).
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).
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.
Ah, FB2. I honestly don't recall if the image detection thingy is even supposed to work in those. @poire-z?
In the meantime (since @poire-z is on a well deserved vacation ;o)), check if EPUB works ;).
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