Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Page browser: show nightmode thumbnails when in nightmode #11091

Merged
merged 2 commits into from Nov 10, 2023

Conversation

poire-z
Copy link
Contributor

@poire-z poire-z commented Nov 9, 2023

Just prevent page thumbnails ImageWidgets to be nightmode inverted, unlike all other ones which are expected to be double inverted to get their original colors shown.
The same thumbnail can be used and cached in both day and night modes, unless "nightmode_images" is enabled and have crengine itself invert images, making thumbnails different.

(Had a hard time writing that commit message :) was initially mentionning "triple inverting"... hard to put into words what's happening in nightmode and the simple tweaks there).

See #10811 (comment)

Pinging @NiLuJe for your OK the the ImageWidget original_in_nightmode = true idea/wording.


This change is Reviewable

Just prevent page thumbnails ImageWidgets to be nightmode
inverted, unlike all other ones which are expected to be
double inverted to get their original colors shown.
The same thumbnail can be used and cached in both day and
night modes, unless "nightmode_images" is enabled and have
crengine itself invert images, making thumbnails different.
@@ -250,6 +250,10 @@ end
function ReaderThumbnail:getPageThumbnail(page, width, height, batch_id, when_generated_callback)
self:setupCache()
self.current_target_size_tag = string.format("w%d_h%d", width, height)
if self.ui.rolling and Screen.night_mode and self.ui.document.configurable.nightmode_images == 1 then
-- We'll get a different bb in this case: it needs it own cache hash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

über nit: final it should be an its ;).

@@ -250,6 +250,10 @@ end
function ReaderThumbnail:getPageThumbnail(page, width, height, batch_id, when_generated_callback)
self:setupCache()
self.current_target_size_tag = string.format("w%d_h%d", width, height)
if self.ui.rolling and Screen.night_mode and self.ui.document.configurable.nightmode_images == 1 then
-- We'll get a different bb in this case: it needs it own cache hash
self.current_target_size_tag = self.current_target_size_tag .. "_ni"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I usually abbreviate to nm

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(ni meant "nightmode images", but ok :)

Copy link
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the cosmetics nits, sounds good.

Might want to check whether it behaves both with soft (emu or disabled in dev settings) & hard (kobo) inversion, as I don't recall how this may all affect this ;).

@poire-z
Copy link
Contributor Author

poire-z commented Nov 9, 2023

Might want to check whether it behaves both with soft (emu or disabled in dev settings) & hard (kobo) inversion, as I don't recall how this may all affect this ;).

My emulator and device are old, and so soft. I don't have a Kobo with hardware nightmode.
But all the stuff in this PR happens at the UI level - which is quite before both soft & hard nightmode come into play I believe.

@NiLuJe
Copy link
Member

NiLuJe commented Nov 10, 2023

My emulator and device are old, and so soft.

Every Kobo (except for the Aura, unless it's on a recent FW; and except for the sunxi ones) supports hardware inversion, so yours should be using HW inversion ;).

@poire-z
Copy link
Contributor Author

poire-z commented Nov 10, 2023

Well, ok :) (I thought there was a toggle in the menu to switch between hard/soft, but looks like there isn't).
Anyway, it works on my Kobo - but nightmode is still ugly :)

@NiLuJe
Copy link
Member

NiLuJe commented Nov 10, 2023

Oh, yeah, I also mistakenly thought I added a switch for that in dev settings, but, nope, that's dithering only. (It indeed makes no sense to disable HW inversion: either it works and it's free performance, or it doesn't and we just kill it by default).

@poire-z poire-z merged commit 894cb31 into koreader:master Nov 10, 2023
2 of 3 checks passed
@poire-z poire-z deleted the nightmode_pagebrowser branch November 10, 2023 19:37
@poire-z poire-z added this to the 2023.11 milestone Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants