Add an option to *always* flash on chapter boundaries

[NiLuJe]

Minor QoL change, because I like the concept ;).

Also, when this (or refresh rate is set to chapter) is enabled, also flash on the second page of a chapter, as chapter headings usually mean extra whitespace, meaning more chance for visible ghosting/contrast deviations.

(Note that the current algo only flashes when paging forward, I didn't touch that).

---

This change is 

[NiLuJe]

A more complex CRe-only algorithm could try to simply detect vertical rivers between pages by keeping track of the amount of lines of text rendered, and flash if there's a significant discrepancy between pages.

But, in practice, that only matters when scene breaks are involved, and it'd make handling the crazy people reading with vertical padding between paragraphs harder, maybe?

[NiLuJe]

Speaking of scene breaks, I don't think we do that currently, but flashing when there's (significant) image content should probably be NTH, too.

I think all the bits necessary are here, because we already detect those cases for dithering.

EDIT: Yup, done.

[NiLuJe]

(Those extra forced flashes will reset the refresh_rate counter).

[poire-z]

I didn't know we had that :/ I may like it, dunno yet :)

A more complex CRe-only algorithm could try to simply detect vertical rivers between pages by keeping track of the amount of lines of text rendered, and flash if there's a significant discrepancy between pages.

That might be too dependant on the content. You may get the same number of lines, but have a little shift because of some bigger font size header or just sup/sub that shifted the line height for one single line, or some small padding/margin added to DIV/P/BLOCKQUOTE containers by the publisher.
Althought it might be fine for a regular styletweaked novel, I'd prefer not to have to add a self.ui.document:getDrawnTextStatistics() that might not have a real scientific base to it :)

If I end up liking it, I guess the only little annoying thing is that for the kind of book I'm reading currently, with a heading every 2 pages (so, a chapter, but I don't mind having 1000 chapters and a crowder progress bar), it will flash every 2 page - and the only way to disable that is the toggle you added - which is global. So, I'll have to toggle the global setting back on when switching to another book. Some local override ("Disable for current book") might come handy later (if I really like it :)

[NiLuJe]

That might be too dependant on the content.

Agreed, hence my next § ;). That was mainly just me thinking out loud ;).

If I end up liking it, I guess the only little annoying thing is that for the kind of book I'm reading currently, with a heading every 2 pages (so, a chapter, but I don't mind having 1000 chapters and a crowder progress bar), it will flash every 2 page - and the only way to disable that is the toggle you added - which is global. So, I'll have to toggle the global setting back on when switching to another book. Some local override ("Disable for current book") might come handy later (if I really like it :).

Well, it is global by design (i.e., like refresh_rate), and disabled by default ;p. And refresh_rate is also never -1 by default.

But that ties in to something I wanted to ask @yparitcher: what do I need to do to make this accessible to dispatcher/gesture/profile, because that'd take care of @poire-z's concerns nicely.

[taosxx]

Also, when this (or refresh rate is set to chapter) is enabled, also flash on the second page of a chapter, as chapter headings usually mean extra whitespace, meaning more chance for visible ghosting/contrast deviations.

Please, make it possible to override this manually without having to compile KOreader. For me, there's nothing more distracting in Nickel while reading than the flash between the first and second page of the chapter (okay, a complete flash after every 6 page turns is worse ;-) ). For some reason, I never read books that would benefit from a second flash -- even if high-contrast ornaments or images are used as chapter headings, for me, some ghosting is much less intrusive than the flash afterwards.

[yparitcher]

But that ties in to something I wanted to ask @yparitcher: what do I need to do to make this accessible to dispatcher/gesture/profile, because that'd take care of @poire-z's concerns nicely.

1. make sure what you want to change is callable via an event
2. add a entry to this table following the comment at the beginning.

   koreader/frontend/dispatcher.lua

   Line 33 in 01ed79f

   local settingsList = {

3. add a entry to this table for where in the menu order you want it displayed.

   koreader/frontend/dispatcher.lua

   Line 143 in 01ed79f

   local dispatcher_menu_order = {

What is the best way to add these instructions to the docs for the future?

[yparitcher]

once you make a event/dispatcher for this you might want to add one for the regular refresh frequency :)

[NiLuJe]

Okay, good to know I got it right in #6530 then ;p.

So, yeah, have to make new events for those.

[NiLuJe]

@taosxx:

Also, when this (or refresh rate is set to chapter) is enabled, also flash on the second page of a chapter, as chapter headings usually mean extra whitespace, meaning more chance for visible ghosting/contrast deviations.

Please, make it possible to override this manually without having to compile KOreader. For me, there's nothing more distracting in Nickel while reading than the flash between the first and second page of the chapter (okay, a complete flash after every 6 page turns is worse ;-) ). For some reason, I never read books that would benefit from a second flash -- even if high-contrast ornaments or images are used as chapter headings, for me, some ghosting is much less intrusive than the flash afterwards.

What's your actual setup? refresh_rateset to -1?

There's no silver bullet here.

I can either create an inconsistency by having rr -1 behave as usual, but flash_on_boundaries flash on both, OR add an insane new layer of complexity by adding two new settings to handle both cases: rr -1 for the usual, rr -2 for both; flash_on_chapter for rr -1 behavior, flash_on_boundaries for rr -2 behavior.

I'm not particularly happy about either, but since I don't personally use rr -1, the selfish and easy solution would be the former ;p.

[poire-z]

Bit strange to have one of these positive and the other negative:

[ } Always flash on chapter boundaries
[ } Don't flash on the second page of a new chapter

[ ] Also flash on a chapter 2nd page would read better (with may be a enabled_func to have it greyed out when the first one is not enabled ?

[NiLuJe]

Bit strange to have one of these positive and the other negative:

[ } Always flash on chapter boundaries
[ } Don't flash on the second page of a new chapter

[ ] Also flash on a chapter 2nd page would read better (with may be a enabled_func to have it greyed out when the first one is not enabled ?

That's the new default behavior. I'm not a fan of inverted settings (i.e., whose defaults is true, which would be necessary to make this behavior default if presented this way).

EDIT: Duh. Thinko.

[NiLuJe]

Okay, managed not to break anything when switching to Events, so this should be good to go ;).

[poire-z]

Just mentionning that (as I don't mind much this strange phrasing as we won't visit that menu often):
The checkbox does not have to reflect exactly the true/false value of the internal setting.
By negating the checked_func, you could use the positive phrasing (and if you don't want it to be checked even when not enabled for aesthetic reason, include the enabled tests into the checked_func tests.

[NiLuJe]

Just mentionning that (as I don't mind much this strange phrasing as we won't visit that menu often):
The checkbox does not have to reflect exactly the true/false value of the internal setting.
By negating the checked_func, you could use the positive phrasing (and if you don't want it to be checked even when not enabled for aesthetic reason, include the enabled tests into the checked_func tests.

True, but I'm also not a fan of those either, and I managed to subtly bork stuff when doing just that in the past ;p.

[Galunid]

Sidenote, would you be interested in adding this to Reader timer (something like Till the next chapter)?

[NiLuJe]

Sidenote, would you be interested in adding this to Reader timer (something like Till the next chapter)?

Not sure I follow? (I'm wholly unfamiliar with the readtimer plugin).

AFAICT, that plugin deals in time, not chapters, so, not the right scope?

[taosxx]

What's your actual setup? refresh_rateset to -1?

Exactly.

Oh, and thanks for adding the option to disable the second page flash :-)

[poire-z]

@NiLuJe : can you have a look at how that works with a Wikipedia EPUB, where there are many small chapters.
I sometimes get no flash on a page starting with a H2 (big black square), but I get the 2nd one.
I sometimes get no flash on others sub-chapters, or only on the 2nd page.
May be it's a side effect of handling the second page when it's already a first page, or something like that (haven't looked at the code).
May be it's ok if some are ignored because of side effects of 1st/2nd page checks and there was a flash not long ago, but it's a bit confusing :)

(I have a small patch to readertoc to get ticks for all TOC entries and not limit them to some level - but I don't think this should be at play there.)

[NiLuJe]

@poire-z: Yep, there's quite probably potential for false-negatives with small chapters. I've rewritten the heuristic, I'll see how it fares now. Do you happen to have an example article I could check?

[poire-z]

Been testing with Soufisme.FR.epub (just look it up and Save as EPUB).

[NiLuJe]

It's almost perfect with the revamped code (i.e., it basically flashes on every page :D), the only thing that doesn't flash is 3. Doctrine.... because it doesn't have a tick, for some reason?

[poire-z]

because it doesn't have a tick, for some reason?

Probably because #5500 (comment) : only lower headings (one of H3 or H4) get ticks. It may feel like H2 got a tick (and a flash) probably because they have a H3 or H4 on the same page ?
My small patch to readertoc to get ticks for all TOC entries and not limit them to some level (available at that comment) is actually to kill that.

#6540 indeed makes it consistent (but flash on every page :/) , and it works for me on "3 Doctrine"
but somehow, the 2nd page flash is not done on the 2nd page of Sommaire because it's not in the TOC :)

(Can't we find some heuristic for such book so that it doesn't flash on every page ? :)

[poire-z]

(Can't we find some heuristic for such book so that it doesn't flash on every page ? :)

Just for info, I helped myself with:

--- frontend/apps/reader/modules/readertoc.lua
+++ frontend/apps/reader/modules/readertoc.lua
@@ -53,2 +53,3 @@
     self.expanded_nodes = {}
+    self.avoid_chapter_flash = nil
 end
@@ -62,2 +63,10 @@
     if UIManager.FULL_REFRESH_COUNT == -1 or G_reader_settings:isTrue("refresh_on_chapter_boundaries") then
+        if self.avoid_chapter_flash == nil then
+            -- No flash on chapter if too many ticks
+            self.avoid_chapter_flash = (#self:getTocTicks(0) / self.ui.document:getPageCount()) > 0.3
+        end
+        if self.avoid_chapter_flash then
+            self.pageno = pageno
+            return
+        end
         local flash_on_second = G_reader_settings:nilOrFalse("no_refresh_on_second_chapter_page")

Not a super reliable check (if most of all the ticks are in some small part of the book), but good enough to not have it with Wikipedia EPUBs.

[NiLuJe]

Another approach would be counting consecutive chapter flashes (or near consecutive flashes), and throttle them. The event handler where the current heuristic is should have all the info.