koreader / koreader

An ebook reader application supporting PDF, DjVu, EPUB, FB2 and many more formats, running on Cervantes, Kindle, Kobo, PocketBook and Android devices
http://koreader.rocks/
GNU Affero General Public License v3.0
16.54k stars 1.25k forks source link

Is it possible to enable 'Dual Pages' also in portrait format? #7129

Closed grabi678 closed 3 years ago

grabi678 commented 3 years ago

This would be helpful for reading certain publications on e-ink readers with 8" or higher screen size.

poire-z commented 3 years ago

Not easily - it's not just a matter of removing this enabled_func check: https://github.com/koreader/koreader/blob/47568ae9b48271ef010e58d1deb583e6aa5821aa/frontend/ui/data/creoptions.lua#L34-L54

crengine (our EPUB engine) will itself prevent going dual page in some conditions: https://github.com/koreader/koreader/blob/47568ae9b48271ef010e58d1deb583e6aa5821aa/frontend/apps/reader/modules/readerrolling.lua#L1019-L1032

Would need to remove bits in this check: https://github.com/koreader/crengine/blob/997a9a8fbe4486942997adf05c3516af32512b08/crengine/src/lvdocview.cpp#L3441-L3444 Dunno how this would affect CoolReader thus.

Would look like this: image

Not really nice, but might be useful to people with bad progressive glasses with a very narrow reading view and a stuck neck :)

So, not currently possible, but might be. Waiting for more feedback if we should.

grabi678 commented 3 years ago

Yes, there are myopic readers who can easily read small fontsizes without glasses. Aside from that devices are getting bigger such as InkPad X . In my opinion this would give the reader a more fexible way to use his device. If it is not too much effort I would appreceate this option could be enabled also for portrait format.

zwim commented 3 years ago

Yes, I am severely nearsighted. I did a quick test with the changes suggested by @poire-z. On my Tolino4HD (6" 1448x1072) the result is not really usable. But on the Epos2 (8"1440x1920), when using a smaller font-size books are good readable with two columns in portrait. The only thing is, that the font-size has to be chosen so, that there are approximately 40 characters in a line. I would suspect, that on larger devices the experience would be even better.

poire-z commented 3 years ago

Ok, I can get that working. 2 questions:

(Although we have room on that left most bottom panel, so we could have 1 or 2 more toggles - but let's pretend I didn't wrote that :)

zwim commented 3 years ago
* We have one setting for this, so people enabling dual pages in landscape mode will get it enabled too when switching to portrait mode. Is that ok? Real use cases for reading in dual page in landscane, but when switching to portrait, expecting a single page? (I hope not, I don't want to complicate the handling :)

I don't think switching from landscape to portrait is a big problem, because the content will have to get rendered anyway. So there are different line and page breaks.

Maybe it would be a good idea not to close the bottom menu on rotation. Then the user can decide if he/she wants two pages/columns or one page/column on rotation.

* There's a thing in crengine: with a color screen, it draws a thin grey line as a separator between the 2 pages - but on a grey/eInk screen, it doesn't ...? What do you prefer? (Hoping we'll have a consensus and it doesn't have to be a setting :)

I have no real preference about the gray separator line.

zwim commented 3 years ago

Another idea: in portrait mode two pages are more two columns. Would it be possible to name the option something like "two columns" in portrait and "two pages" in landscape mode?

poire-z commented 3 years ago

Maybe it would be a good idea not to close the bottom menu on rotation. Then the user can decide if he/she wants two pages/columns or one page/column on rotation.

Well, that's a bit more difficult than for all the other cases, as there's a change in geometry and everything is closed to be later recreated with the new size. So, not taking this painful idea :)

Another idea: in portrait mode two pages are more two columns. Would it be possible to name the option something like "two columns" in portrait and "two pages" in landscape mode?

May be from how it would look - but they are still 2 pages: a tap will increment the page count in the footer by 2, the Skim To +1/-1 buttons will do nothing every other tap. Solving this (that I don't intend to) would make them 2 columns in all cases. So, rather not introduce a change in wording that doesn't really reflect in any real behaviour change.

Frenzie commented 3 years ago

Another idea: in portrait mode two pages are more two columns. Would it be possible to name the option something like "two columns" in portrait and "two pages" in landscape mode?

I think that's confusing more than anything, even if it's more accurate in some ways. :-)

WS64 commented 3 years ago

I only read in dual page and also would love to try it in portrait mode. My opinion, only one dual page mode, valid for both landscape and portrait.

poire-z commented 3 years ago

OK, #7138 should be in tomorrow's nightly. Please play with it tomorrow, and let me know if there's an obvious usability issue before next release (possibly next weekend). (I removed the thin line separator, so you'll stop having it on color screens.)

zwim commented 3 years ago

Thank you. I will play with it.

Am 13. Jänner 2021 19:13:14 MEZ schrieb poire-z notifications@github.com:

OK, #7138 should be in tomorrow's nightly. Please play with it tomorrow, and let me know if there's an obvious usability issue before next release (possibly next weekend). (I removed the thin line separator, so you'll stop having it on color screens.)

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/koreader/koreader/issues/7129#issuecomment-759626872

-- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

zwim commented 3 years ago

@poire-z: The only thing which is slightly annoying with two columns/pages in portrait mode is, that progress bar in the top status bar only uses the right half. Apart from that I like the new feature very much!

NiLuJe commented 3 years ago

That's... fun. Did it work in landscape?

zwim commented 3 years ago

It's the same behavior in landscape and portrait.

Am 14. Jänner 2021 19:12:58 MEZ schrieb NiLuJe notifications@github.com:

That's... fun. Did it work in landscape?

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/koreader/koreader/issues/7129#issuecomment-760371096

-- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

poire-z commented 3 years ago

Thanks for the feedback.

that progress bar in the top status bar only uses the right half.

I guess it's because there's a cut in the line, to not have a long line kill the feeling of 2 pages. With the separator I removed: image

Having the progress bar span over the separator would look bad. Now that we removed the separator, we could make it longer (actually, just like on a single page), but that may still kill the feeling of 2 pages (but not for you as you consider them 2 columns :) (But ok, we have a footer that spans the 2 pages...)

The code involved is around https://github.com/koreader/crengine/blob/a90c0be089b08e0b0af64289fb0fdc8ba6d5ca6b/crengine/src/lvdocview.cpp#L1683-L1773

Frenzie commented 3 years ago

Fwiw (since I don't really use the feature), I've never cared for that separator in the middle.

zwim commented 3 years ago

With the following patch would allow to show the full header is shown in dual page/columns (on portrait and landscape).

diff --git a/crengine/src/lvdocview.cpp b/crengine/src/lvdocview.cpp
index 8db1d4e3..d60df169 100644
--- a/crengine/src/lvdocview.cpp
+++ b/crengine/src/lvdocview.cpp
@@ -1326,7 +1326,7 @@ void LVDocView::getPageHeaderRectangle(int pageIndex, lvRect & headerRc) {
        int h = getPageHeaderHeight();
        headerRc.bottom = headerRc.top + h;
        headerRc.top += HEADER_MARGIN;
-       headerRc.left += HEADER_MARGIN;
+       headerRc.left = HEADER_MARGIN;    //xxxx
        headerRc.right -= HEADER_MARGIN;
    }
 }
@@ -1699,6 +1699,7 @@ void LVDocView::drawPageHeader(LVDrawBuf * drawbuf, const lvRect & headerRc,
     //lUInt32 cl2 = getBackgroundColor();
     //lUInt32 cl3 = 0xD0D0D0;
     //lUInt32 cl4 = 0xC0C0C0;
+    lUInt32 clxxx = 0xC0C0C0;
    drawbuf->SetTextColor(cl1);
    //lUInt32 pal[4];
    int percent = getPosPercent();
@@ -1952,9 +1953,10 @@ void LVDocView::drawPageTo(LVDrawBuf * drawbuf, LVRendPageInfo & page,
            if (page.index & 1) {
                // right
                phi &= ~PGHDR_AUTHOR;
+               phi &= ~PGHDR_TITLE;
             } else {
                // left
-               phi &= ~PGHDR_TITLE;
+//             phi &= ~PGHDR_TITLE;
                 phi &= ~PGHDR_PERCENT;
                 phi &= ~PGHDR_PAGE_NUMBER;
                phi &= ~PGHDR_PAGE_COUNT;
poire-z commented 3 years ago

^ the progress bar is now wide, but I don't see the book title, just the author:

image

Another idea if we don't want to disrupt how it shows currently is just to have the progress bar being 2 segments, so it would just skip the blank.

Pinging CoolReader's @virxkane - as this is the main status bar in CoolReader, to be involved in the discussion if we're going to change it (only how it look in "2 pages modes / VisiblePages == 2")

zwim commented 3 years ago

Interresting: One book shows Author+Title grafik but other books show just the Author grafik

virxkane commented 3 years ago

Pinging CoolReader's @virxkane - as this is the main status bar in CoolReader, to be involved in the discussion if we're going to change it (only how it look in "2 pages modes / VisiblePages == 2")

Sorry, but I have accumulated a lot of unfinished business, and this code is not familiar to me, so I cannot help right now. Later, I or someone else will have to deal with this :(

virxkane commented 3 years ago

Interresting: One book shows Author+Title

It seems there is a limitation on the length, in the usual one-page mode, also, if it does not fit, then the author and the title are displayed alternately. I.e on one page - the author, on the next - the title.

poire-z commented 3 years ago

Sorry, but I have accumulated a lot of unfinished business, and this code is not familiar to me, so I cannot help right now. Later, I or someone else will have to deal with this :(

That's alright! :) I was not so much requesting your help on the code - than your opinion on the top status bar in the various CoolReader frontends. When in 2-pages mode (which might be rare, dunno, may be not rare in the QT versions used on PC screen), would you (and your users) be fine with it being a single bar taking the full width (so, no break in the middle) - or does the fact that 2-pages mean 2 top status bar (with each different infos) feels like some UI thing of value? If it doesn't, we can just make it the same wide top status bar in both 1-page and 2-pages mode. It if does, well, we'll think again :)

zwim commented 3 years ago

Interresting: One book shows Author+Title

It seems there is a limitation on the length, in the usual one-page mode, also, if it does not fit, then the author and the title are displayed alternately. I.e on one page - the author, on the next - the title.

@virxkane: Thanks, that was the problem.

The following diff is not quite beautiful (empty statement), but does the trick to show what it will look like :)

diff --git a/crengine/src/lvdocview.cpp b/crengine/src/lvdocview.cpp
index 8db1d4e3..c1891ac8 100644
--- a/crengine/src/lvdocview.cpp
+++ b/crengine/src/lvdocview.cpp
@@ -1326,7 +1326,7 @@ void LVDocView::getPageHeaderRectangle(int pageIndex, lvRect & headerRc) {
        int h = getPageHeaderHeight();
        headerRc.bottom = headerRc.top + h;
        headerRc.top += HEADER_MARGIN;
-       headerRc.left += HEADER_MARGIN;
+       headerRc.left = HEADER_MARGIN;    //xxxx
        headerRc.right -= HEADER_MARGIN;
    }
 }
@@ -1717,12 +1717,13 @@ void LVDocView::drawPageHeader(LVDrawBuf * drawbuf, const lvRect & headerRc,
 //     cl4 = cl1;
 //     //pal[0] = cl1;
 // }
+/* xxxx Maybe some other dependencies on left-page can be cleaned, too.
    if ( leftPage )
        drawbuf->FillRect(info.left, gpos - 2, info.right, gpos - 2 + 1, cl1);
         //drawbuf->FillRect(info.left+percent_pos, gpos-gh, info.right, gpos-gh+1, cl1 ); //cl3
         //      drawbuf->FillRect(info.left + percent_pos, gpos - 2, info.right, gpos - 2
         //                      + 1, cl1); // cl3
-
+*/
    int sbound_index = 0;
    bool enableMarks = !leftPage && (phi & PGHDR_CHAPTER_MARKS) && sbounds.length()<info.width()/5;
    int w = GetWidth();
@@ -1893,7 +1894,7 @@ void LVDocView::drawPageHeader(LVDrawBuf * drawbuf, const lvRect & headerRc,
        }
        int w = info.width() - 10;
        if (authorsw + titlew + 10 > w) {
-           if ((pageIndex & 1))
+           if ((pageIndex & getVisiblePageCount()))
                text = title;
            else {
                text = authors;
@@ -1951,9 +1952,11 @@ void LVDocView::drawPageTo(LVDrawBuf * drawbuf, LVRendPageInfo & page,
        if (getVisiblePageCount() == 2) {
            if (page.index & 1) {
                // right
-               phi &= ~PGHDR_AUTHOR;
+//             phi &= ~PGHDR_AUTHOR;
+        ; /* empty statement!!!! */
             } else {
                // left
+               phi &= ~PGHDR_AUTHOR;
                phi &= ~PGHDR_TITLE;
                 phi &= ~PGHDR_PERCENT;
                 phi &= ~PGHDR_PAGE_NUMBER;
virxkane commented 3 years ago

than your opinion on the top status bar in the various CoolReader frontends.

I don't use 2-page mode, so my opinion is this: it should be the most obvious and intuitive. Or what is most simple: leave it as it is, i.e. what the 2-page mode looks like now. Clipboard08 In landscape mode, it should probably look as much like a paper book as possible because that's the first association that comes up, i.e. 2 status bars. book_zzz

But in portrait mode, when displaying 2 pages, it seems intuitively that there should be one status bar. In addition, in this mode, the 2nd status bar may simply not fit.

virxkane commented 3 years ago

I think it makes sense to also ask @hius07's opinion.

poire-z commented 3 years ago

^^ makes sense. CoolReader currently can't get 2-pages in portrait mode (needs an additional param to setVisiblePageCount()), so we won't sadden anybody by going at it differently :) @zwim : you could go at tweaking it by just having early: bool isPortraitMode = fullRect.bottom > fullRect.right; and applying tweaks a bit more cleanly if (we have two pages && isPortraitMode)

Or may be cleaner: just not take the 2 pages paths when isPortraitMode.

(An additional improvement to landscape mode would be to have the progress bar displays ticks and progress on the 2 segments.)

zwim commented 3 years ago

I have created PR https://github.com/koreader/crengine/pull/408. So we can discuss easier.

poire-z commented 3 years ago

Some discussion has continued in https://github.com/koreader/crengine/pull/408 - but getting back on it here as it's more visible than there.

If all that happens to be working well, are we all fine to go with that too in landscape mode, for consistency? With twoVisiblePagesAsOnePageNumber=true, meaning: single wide header, current page number and total page count being the number of screen views - so what we see would need to be thought of as 2-columns on a single page, and no longer 2-pages. (We then would need to rename "Dual Page" to "Two Columns"). I'd prefer this to be hardcoded and not a user toggle, it's a KOReader choice to present things that way (CoolReader would get the current behaviour, until they decide to set the new property and may be need some frontend tweaks) - and it's the same in landscape and portrait, no reason (except old habits if really hard to lose) to change the meaning/page-number-steps in the footer whether in portrait vs landscape. (If not, and we'd still want to keep dual-page and make it a first class citizen, a few other tweaks would be needed, like in the SkimTo widget, replacing +1 with +2 - so we don't get half the taps ineffective :)

A few more questions / requests for opinion related to all this.

A This issue would not exist in "2-column-as-1-page" mode, but it exists in the current "2-pages" mode: when showing 2 pages, which page number should the footer and header show: the page number of the left page or the one of the right page ? Because currently, header and footer disagree: image On this, when showing the first and second page of a book, which unique page number feels the right one to show?

B This also happens in one-page mode: when one (or two) pages shows more than one chapter, which chapter to use to display in the footer ? The first one, or the last one in view? Currently in 2-pages mode: image which would be in 2-column-as-1-page: image I would personally expect all to show as current chapter: "4.5 Property". Thoughts?

C With "2-column-as-1-page", which is just masquerading page numbers while still having crengine working with them as 2-pages, how far should we push the "2-column-as-1-page" principle with page-breaks ? This publisher page break would be a column break: image Should we add a blank "column" on the right so it really becomes a page/view break? Or is it just ok to consider page-breaks as column-breaks ?

Frenzie commented 3 years ago

I would generally say the first one. Possible exception if it's just one or two lines.

On Wed, Jan 20, 2021, 14:23 poire-z notifications@github.com wrote:

Some discussion has continued in koreader/crengine#408 https://github.com/koreader/crengine/pull/408 - but getting back on it here as it's more visible than there.

If all that happens to be working well, are we all fine to go with that too in landscape mode, for consistency? With twoVisiblePagesAsOnePageNumber=true, meaning: single wide header, current page number and total page count being the number of screen views - so what we see would need to be thought of as 2-columns on a single page, and no longer 2-pages. (We then would need to rename "Dual Page" to "Two Columns"). I'd prefer this to be hardcoded and not a user toggle, it's a KOReader choice to present things that way (CoolReader would get the current behaviour, until they decide to set the new property and may be need some frontend tweaks) - and it's the same in landscape and portrait, no reason (except old habits if really hard to lose) to change the meaning/page-number-steps in the footer whether in portrait vs landscape. (If not, and we'd still want to keep dual-page and make it a first class citizen, a few other tweaks would be needed, like in the SkimTo widget, replacing +1 with +2 - so we don't get half the taps ineffective :)

A few more questions / requests for opinion related to all this.

A This issue would not exist in "2-column-as-1-page" mode, but it exists in the current "2-pages" mode: when showing 2 pages, which page number should the footer and header show: the page number of the left page or the one of the right page ? Because currently, header and footer disagree: [image: image] https://user-images.githubusercontent.com/24273478/105178216-083d8100-5b28-11eb-9d12-2b08e9c78540.png On this, when showing the first and second page of a book, which unique page number feels the right one to show?

B This also happens in one-page mode: when one (or two) pages shows more than one chapter, which chapter to use to display in the footer ? The first one, or the last one in view? Currently in 2-pages mode: [image: image] https://user-images.githubusercontent.com/24273478/105178626-9285e500-5b28-11eb-98e5-60ebb40329e0.png which would be in 2-column-as-1-page: [image: image] https://user-images.githubusercontent.com/24273478/105178859-dd9ff800-5b28-11eb-854c-c7b7d8e6da6f.png I would personally expect all to show as current chapter: "4.5 Property". Thoughts?

C With "2-column-as-1-page", which is just masquerading page numbers while still having crengine working with them as 2-pages, how far should we push the "2-column-as-1-page" principle with page-breaks ? This publisher page break would be a column break: [image: image] https://user-images.githubusercontent.com/24273478/105179562-c6153f00-5b29-11eb-853b-ad21e86f6516.png Should we add a blank "column" on the right so it really becomes a page/view break? Or is it just ok to consider page-breaks as column-breaks ?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/koreader/koreader/issues/7129#issuecomment-763602579, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQBIJNALTENRFKJXEJMLS23KL5ANCNFSM4V4NQHUA .

hius07 commented 3 years ago

I'd like a publisher page break to be a page break, so to add a blank column.

zwim commented 3 years ago

To question A: I would go for the number of the right page (as the pagenumber is on the right half of the view). To question B: The last one (The books have checked do it that way). To C: I would go page break is just a column break. A new chapter may start on the left side/column, but please not on every pagebrak.

poire-z commented 3 years ago

To question A: I would go for the number of the right page (as the pagenumber is on the right half of the view).

It's not only used there: it could be in the footer where page number could appear on the left side, it's what appears in the middle in the SkimTo widget, it's what is part of the bookmark when bookmarking or highlighting... So, I don't think we should think about it only from a visual position point of view.

To question B: The last one (The books have checked do it that way).

What do you mean with The books have checked do it that way ? That you did check some paper book ? But a paper book doesn't tell you what chapter is displayed when you have it opened and are reading 2 pages :/ (?)

I'd like a publisher page break to be a page break, so to add a blank column. To C: I would go page break is just a column break.

Will need more opinions on that one :) It's quite a lot more cumbersome to add the blank column (as the setting would have to be forwarded and handled lower in the rendering code - without that, I don't have to, it's just upper in the page number interface)

A new chapter may start on the left side/column, but please not on every pagebrak.

You mean you're fine with a new chapter may start on the right column ? (Because with page break = page break/blank column means all chapters will start on the left column.) (I think the question is not about adding a blank left page to make it look like in paper book where new chapters start on the right page - but the contrary, whether to add a blank right page or not so chapters start on the left page, which makes it really unlike paper books :/ so may be that's another question :) :/)

NiLuJe commented 3 years ago

Not the target audience (like, at all), but, FWIW:

A. First page. B. As you mentioned, first heading. C. Column break.

Frenzie commented 3 years ago

Not the target audience (like, at all)

I'm a possibly somewhat unique form of target audience, namely the large window on desktop type. I also want that in paged media, like Zathura (and just about any other desktop reader, but Zathura feels more similar than Evince or Adobe Reader). But a very mild form of want, or I'd have added it years ago. ;-)

zwim commented 3 years ago

It's not only used there: it could be in the footer where page number could appear on the left side, it's what appears in the middle in the SkimTo widget, it's what is part of the bookmark when bookmarking or highlighting... So, I don't think we should think about it only from a visual position point of view.

OK, then the first page (or maybe both 3-4/120).

What do you mean with The books have checked do it that way ? That you did check some paper book ? But a paper book doesn't tell you what chapter is displayed when you have it opened and are reading 2 pages :/ (?)

Some of my printed books are really clever, they know on which chapter you read: grafik

To C: I would go page break is just a column break.

I meant pagebreak should start a new column.

Frenzie commented 3 years ago

Some of my printed books are really clever

= use LaTeX xD

NiLuJe commented 3 years ago

OK, then the first page (or maybe both 3-4/120).

That's an interesting thought ;).

I meant pagebreak should start a new column.

i.e., the behavior in @poire-z's original screenshot, right?

hius07 commented 3 years ago

It's quite a lot more cumbersome to add the blank column

No problem, let it be a column break.

poire-z commented 3 years ago

To question B: The last one (The books have checked do it that way). Some of my printed books are really clever, they know on which chapter you read:

:) Nice book, which seems to answer the opposite of what you wrote, doesn't it?: on the right page, it shows the first chapter in this right page (3.10.2...), not the last chapter in this right page (3.10.3...).

I meant pagebreak should start a new column.

I got that. It's the next part of your answer that I didn't get (A new chapter may start on the left side/column, but please not on every pagebrak.)

hius07 commented 3 years ago

So, what are the advantages of Dual Pages over Two Columns in landscape? Two alt bars and double page counting? That's all? Is it worth the loss of unification?

poire-z commented 3 years ago

what are the advantages of Dual Pages over Two Columns in landscape? Two alt bars and double page counting? That's all? Is it worth the loss of unification?

I'm with you at wanting them unified :) mostly for the unification and avoiding the complexities of handling the 2 cases (and fixing the Dual Pages bugs I've seen that would just not be there with Two Columns). It's just that in landscape, it seems to me it's less natural/intuitive to think of them as 2 columns. (But I don't use landscape, so, the more users tell me it's fine, the sooner I'll forget that thought :)

zwim commented 3 years ago

To question B: The last one (The books have checked do it that way). Some of my printed books are really clever, they know on which chapter you read:

:) Nice book, which seems to answer the opposite of what you wrote, doesn't it?: on the right page, it shows the first chapter in this right page (3.10.2...), not the last chapter in this right page (3.10.3...).

Yes, the first on the actual (=right) page (but not the first on the actual double page).

It's the next part of your answer that I didn't get (A new chapter may start on the left side/column, but please not on every pagebrak.)

I meant: If a new chapter (in LaTex = h1 in html) starts, it would look good, if it would start on the left column/page. But that is not a strong vote. The other section/subsection... (=h2, h3, ...) should not start a new column per se. Only if there is a pagebreak before, they shall start on a new column.

poire-z commented 3 years ago

Also pinging @WS64 as a dual-page user, about the above questions and discussions.

Some other questions to dual page users - can't test that at the moment: how do Reading statistics do with the current dual page, that skips one page number out of two? Do you end up with Page reads = half of the total number of pages? (If that's the case, it might be the definitive argument to drop two-pages, as 2-column on a single page would just fit better with how we count pages).