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

Night mode: keep some images non-inverted #4853

Merged
merged 3 commits into from Mar 30, 2019

Conversation

Projects
None yet
3 participants
@poire-z
Copy link
Contributor

commented Mar 29, 2019

Have the code pre-invert some images when in night mode, so they get inverted back to normal by night mode.
This will allow images to be displayed normally in the following contexts:

  • images in credocument pages
  • images long-pressed in credocument and viewed in ImageViewer
  • cover thumbnails in File browser and History
  • full size covers when viewed in ImageViewer
  • images in Wikipedia lookup results

There is still some issues when the lua blitter is used (Kobo, Kindle), where some images may be wrongly or not inverted, or messed up. No issue seen when the C blitter is used (Android, emulator). Discussed in #2571. Closes #2571.

Includes koreader/crengine#276:

  • DrawBuf: allow drawing images inverted (to be used in night mode)
  • ldomXPointer::getRect(): adds adjusted=false parameter
  • CSS: fix parsing of "! important" with spaces
  • CSS: fix non-deterministic combinators
  • Delay some style computations until needed koreader/crengine#278

Also:
Footnote popups: fix link unhighlight when closing with keys. (Fix last item in #4840 (comment))

@poire-z

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

I'll bump base in a rebase of the last commit once it's merged.

@Frenzie Frenzie added the enhancement label Mar 29, 2019

@Frenzie Frenzie added this to the 2019.04 milestone Mar 29, 2019

target:blitFrom(self.buffer, x, y, 0, 0, rect.w, rect.h)
--print(string.format("CreDocument:drawCurrentView: Blitting took %9.3f ms", (os.clock() - start_clock) * 1000))
-- print(string.format("CreDocument:drawCurrentView: Blitting took %9.3f ms", (os.clock() - start_clock) * 1000))

This comment has been minimized.

Copy link
@NiLuJe

NiLuJe Mar 29, 2019

Member

Whitespace :D.

(Extreme nitpicking aside, that was to differentiate between comments and commented out code ;)).

This comment has been minimized.

Copy link
@NiLuJe

NiLuJe Mar 29, 2019

Member

Looking at the file and not the diff, I see those aren't right next to comments anymore, so, yeah, that works, too, never mind me ;).

This comment has been minimized.

Copy link
@Frenzie

Frenzie Mar 29, 2019

Member

Interesting concept. I spotted it before but I thought it was just an inconsistency. ;-)

(I do in fact keep myself from posting nitpicks now and then. ;-D)

This comment has been minimized.

Copy link
@poire-z

poire-z Mar 29, 2019

Author Contributor

It was extra hard to read to me with no whitespace :)
(I would have remove it if you just hadn't added it :) I think profiling code is just as easy to add and remove only when needed. Can/should I ?)

This comment has been minimized.

Copy link
@NiLuJe

NiLuJe Mar 29, 2019

Member

That was mainly to be able to do it on-device over SSH via nano (instead of a patch/stash + scp from the desktop) ;).

This comment has been minimized.

Copy link
@poire-z

poire-z Mar 29, 2019

Author Contributor

Or may be we could just have somewhere a function available for quick profiling, to avoid losing a few minutes to think about os.clock and string.format, that we could use as:

+ timeit("rendering document", function()
     original code to time
+ end)

or something even easier if some Lua constructs allow for easier, like C define...

This comment has been minimized.

Copy link
@NiLuJe

NiLuJe Mar 29, 2019

Member

Yeah, I'm not terribly fond the the "wrap it in an anonymous func" approach usually taken for that kind of thing in Lua, but that's just me not being fond of many many Lua idiosyncrasies in general ;).

This comment has been minimized.

Copy link
@NiLuJe

NiLuJe Mar 29, 2019

Member

But yeah, having that kind of thing available in any manner somewhere would definitely come in handy ;).

bump crengine: allow drawing images inverted, various fixes
- DrawBuf: allow drawing images inverted
- ldomXPointer::getRect(): adds adjusted=false parameter
- CSS: fix parsing of "! important" with spaces
- CSS: fix non-deterministic combinators
- Delay some style computations until needed

@poire-z poire-z force-pushed the poire-z:nightmode_invert_images branch from bfa7e9d to 84cc9a5 Mar 29, 2019

Night mode: keep some images non-inverted
Have the code pre-invert some images when in night mode,
so they get inverted back to normal by night mode.
This will allow images to be displayed normally in
the following contexts:
- images in credocument pages
- long-press on images in credocument viewed in ImageViewer
- cover thumbnails in File browser and History
- full size covers when viewed in ImageViewer
- images in Wikipedia lookup results

@poire-z poire-z force-pushed the poire-z:nightmode_invert_images branch from 84cc9a5 to 997b54d Mar 29, 2019

@poire-z poire-z merged commit 8f019b8 into koreader:master Mar 30, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@poire-z poire-z deleted the poire-z:nightmode_invert_images branch Mar 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.