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

Double Pages Not Able to Cycle Through Offset to line up properly #796

Closed
5 tasks done
kanjieater opened this issue Feb 6, 2022 · 5 comments
Closed
5 tasks done
Labels
wontfix This will not be worked on

Comments

@kanjieater
Copy link

kanjieater commented Feb 6, 2022

Steps to reproduce

  1. Have a comic with an even number of pages

  2. Select Double Pages (no cover)
    image

  3. Switch to Double Pages

image
4. It shows the same view instead of moving ahead by one page and offsetting the cover properly

Expected behavior

Desired behavior would be to allow a user to change where the double page is for even numbered comics, just like it does for odd-numbered pages comics (even numbered page to odd numbered page or odd numbered page to even numbered page) as it previously worked in Komga - I believe it was working prior to my last issue #772 (though I just confirmed the bug existed on 0.137.0 too).

Left Right

Actual behavior

Komga shows the same view instead of moving ahead by one page and offsetting the cover properly. With double pages sometimes they can even get off by one if there's an additional page of explanation or info, so being able to switch on the fly is necessary.

Logs

No response

Komga version

0.148.3

Operating system

Windows 10

Other details

No response

Acknowledgements

  • I have searched the existing issues and this is a new ticket, NOT a duplicate or related to another open issue.
  • I have written a short but informative title.
  • I have checked the FAQ.
  • I have updated the app to the latest version.
  • I will fill out all of the requested information in this form.
@kanjieater kanjieater added the bug Something isn't working label Feb 6, 2022
@kanjieater kanjieater changed the title Double Pages not offset properly for even number of pages Double Pages Not Able to Cycle Through Offset to line up properly Feb 6, 2022
@gotson
Copy link
Owner

gotson commented Feb 7, 2022

Desired behavior would be to allow a user to change where the double page is for even numbered comics, just like it does for odd-numbered pages comics

You misunderstand what the feature is about. You cannot dynamically change what double pages are shown by toggling the cover/no-cover. At best it's a random side effect, depending on the book.

Komga builds spreads in an absolute manner, depending on the pages, their dimensions (if they have been analyzed), and the display mode (single, double no cover, double).

If you have a page, say number 3, that is landscape, it will change the subsequent spreads, because it would be shown as a single page, not a double page.

You need to verify the pages and their dimensions to see how the spreads would be built. You can check /api/v1/books/<bookId>/pages to get the list of pages and their dimensions (if any).

@gotson gotson removed the bug Something isn't working label Feb 7, 2022
@kanjieater
Copy link
Author

Ok, gotcha. That's how tachiyomi, cdisplayx, and cover work as well though so I've never run into this issue with other readers. The side effect fixes the issue for odd numbered pages, but as I showed, even-numbered comics aren't shown properly.
So from here how can we use Komga to see them properly?
I really would I hope I dont have to go through every comic and add a dummy page but thats the only other solution I can think of rather than considering the aforementioned solution as a desired behavior rather than a side effect.

@gotson
Copy link
Owner

gotson commented Feb 8, 2022

So from here how can we use Komga to see them properly?

The file is badly ripped.

I have no intention on adding dynamic double-pagesin the Webreader that would change depending on the page you currently are at.

@gotson gotson closed this as completed Feb 8, 2022
@gotson gotson added the wontfix This will not be worked on label Feb 8, 2022
@kanjieater
Copy link
Author

kanjieater commented Feb 8, 2022

The file is badly ripped.

I have no intention on adding dynamic double-pagesin the Webreader that would change depending on the page you currently are at.

The file is not badly ripped. I'm not sure what you mean by this. As far as I know there's no official spec for this, so it's hard to say what should or shouldn't be done. I'm certainly not sure what the best practices here are.

Single
image
The single page is treated like a single page layout.

Double
image
The page is automatically aligned to the left (as it should be as Double page should assume there is a single cover).

Double Pages (no cover)
image
The page is displayed the same as single for some reason. It should show two pages. This is where the bug is.

In cover Double Page with Cover shows the image correctly. (thought I would have expected Double Page with No Cover to be the one to show it right?)
image

All Layouts in cover show the single cover the same way for the cover interestingly enough (I'm not sure how they detect this?):
image

Is there another solution that could work for this that would be acceptable for Komga? Could we keep this open and come up with something?

@kanjieater
Copy link
Author

A couple things I gathered from Gotson on how to actually get this implemeted.

Don't want to show a normal page next to a landscape page
should probably only pad it if it's not landscape (So we can pad when there is a single page if needed).

Gotson's initial Outline

at the moment it takes the list of pages, their dimensions, and the layout mode as input
if you throw in the current page, then when you change layout mode while reading, you could change the output
the output is always a list of spreads
a spread is an array of pages, either just 1, or 2
sometimes we throw in a blank page also, so that they align well on the left/right side of the screen for the covers
the algo is pretty simple, it's a for loop that takes pages and put them in the result array

Thinking about it a little deeper after talking I think there's probably a solution that could accomplish the desired behavior of allowing a user to quickly shift between layouts and keeping things deterministic (not needing to recalculate spreads based on page numbers).

Problem Statement
So to summarize the use-case: As a user who reads with double pages, I want to be able to easily shift the pages to show the double spread as a double spread. Sometimes my comics double spreads are shifted so only one page of the double spread shows, and I would like to be able to read the pages together.

Approach
After trying this out on a few different readers there are a couple of different solutions that could work, but I think the cleanest to implement and best user experience would be to use the Double Pages & Double Pages (no cover) to keep a sequence of 2 pages (spreads) that is opposite of each other. So if you ever found your double pages to be off, you switch to the other one with a simple "D" hotkey & don't miss a beat. This is the same way it works now for comics with odd numbers of pages.

Current code

export function buildSpreads(pages: PageDtoWithUrl[], pageLayout: PagedReaderLayout): PageDtoWithUrl[][] {

So if a spreads looks like:
We can assume there is a single cover. There may be an empty page if the Cover is not landscape. Or if it is landscape we don't need the empty page

E: Empty Page
L: landscape
LS: A page that should be landscaped or stitched but is separate
SDS: A split double spread (the problem here)
Num: The physical books page number
[SomeLeftPage, SomeRightPage]: the left page, and the right page in the spread
DP: Double Page Layout (this is what properly ripped and numbered books should look like)
DPNC: Double Pages (no cover) (this is what books additional content that throws off the page number will probably look like)

So the important thing here is that there should never be a single page of the spread that lands on the same side on both DPs spread:

Imagine Pages 3 & 4 are actually a split double spread:

  1. Good Examples:
    DP: [[E, 1L], [2, 3SDS], [4SDS, 5], [6, 7]] // This one won't look as good
    DPNC: [[1L], [E, 2], [3&4SDS], [5,6], [7, E]] // So you can easily switch to this one which will look good
  2. Bad Behavior we're trying to correct:
    DP: [[E, 1L], [2, 3SDS], [4SDS, 5], [6, 7]] // This one looks the same as DPNC
    DPNC: [[1L], [2, 3SDS], [4SDS, 5], [6, 7]] // This one looks the same and doesn't fix the SDS
    As long as the a 2 is not on the left for both it's DP & DPNC layout, there's no issue.

So what about landscape pages at random points? It's important to note that it's ok for L's to end up as the only page in the spread in both layouts. That shouldn't break the alternating flow of single pages though.

  1. Good Examples:
    DP: [[E, 1L], [2, 3], [4L], [5, 6SDS], [7SDS, 8]] // This one won't look as good
    DPNC: [[1L], [E, 2], [4L], [E, 5], [6SDS, 7SDS], [8, E]] // This will look good! Last page get's filled with an E per the current algo
    So the only change on DPNC is that it needs to inject an E after any landscape page when it goes to a single page. So no empty pages before repeating landscape pages, but yes empty page when the landscape page sequence goes back to normal pages
  2. Good Example
    DP: [[E, 1L], [2L], [3, 4], [5, 6SDS], [7SDS, 8]] // This one won't look as good
    DPNC: [[1L], [2L], [E, 3], [4, 5], [6SDS, 7SDS], [8, E]] // This will look good! Last page get's filled with an E per the current algo
  3. Good Example
    DP: [[E, 1], [2, 3], [4, 5], [6, 7], [8L], [9, E] // Same as current
    DPNC: [[1, 2], [3, 4], [5, 6], [7, E], [8L], [E, 9]] // Current algo inject a E before a new L comes on the right [7, E]. Then we keep the pattern of after an L inject an E for [E, 9].

In summary the only change needed is to inject an empty page after a L for DPNC layouts.

Additional Issue 1
Unrelated bug I've seen and uncovered while thinking through this

if (pagesClone.length > 0) {

By taking the last page, you can get into a double page situation like this:
[last page, E]
which was taken from a spreads that looked like this:
[[1,2],[3, last page]]
Resulting in this little awkward situation:
[[1, 2], [3, E], [last page, E]

Additional Issue 2
I don't think we should inject an empty page if the first page is Landscape like the current algo works:
DP: [[E, 1L], [2L], [3, 4]] // Original shows E next to L
DP Revised: [[1L], [2L], [3, 4]] // Just drop the E
DPNC: [[1L], [2L], [E, 3], [4, E]] // This one wouldn't be the ideal choice but would still exist. Yes this is intentional and not Additional Issue 1 - the alternating order must be preserved - there is a right and wrong layout choice. This is the wrong choice for this book.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants