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

Ability to disable chapter transitions #669

Closed
4 tasks done
raxod502 opened this issue Apr 14, 2024 · 12 comments
Closed
4 tasks done

Ability to disable chapter transitions #669

raxod502 opened this issue Apr 14, 2024 · 12 comments
Labels
Feature request New feature or request

Comments

@raxod502
Copy link
Contributor

Describe your suggested feature

As has been reported by several users on the Mihon Discord server, some sources generate a chapter for every page, which results in a chapter transition being shown between every page. There is an existing reader setting for "Always show chapter transition" which can be disabled, but this does not prevent chapter transitions from being shown, rather it inhibits what seems to be a random subset of them from appearing. The desired behavior is that chapter transitions are not shown at all when configured as such for a particular source.

Other details

image

Acknowledgements

  • I have searched the existing issues and this is a new ticket, NOT a duplicate or related to another open or closed issue.
  • I have written a short but informative title.
  • I have updated the app to version 0.16.5.
  • I will fill out all of the requested information in this form.
@raxod502 raxod502 added the Feature request New feature or request label Apr 14, 2024
@Smol-Ame
Copy link
Contributor

Not exactly, here's the real answerScreenshot_20240414_154047.png

@raxod502
Copy link
Contributor Author

That doesn't seem to match up with the behavior that I observe. Even with the current page and next/previous dozen already downloaded in advance, whenever I change from one page to the next or previous, the message "Loading pages..." is displayed momentarily and I am presented with a chapter separator.

I'm currently using "Paged (left to right)" as a reader setting, and "Auto download while reading" is set to "Next 5 unread chapters", although I don't think that setting would be relevant when all the chapters have already been downloaded manually.

Did I misunderstand what to expect from this feature, or am I doing something wrong?

@scb261
Copy link

scb261 commented Apr 15, 2024

I just used a different entry in my library to test this, and I have reproduced it. The first transition is skipped, but then it's always shown if you keep reading without exiting. I have also tried turning off the internet, it still shows "loading page" and displays it normally.

Tried one more thing - not downloaded, but cached beforehand. Turned off the internet. The same thing - pages are shown, but there is still a chapter transition with "loading page".

My guess is that it doesn't properly trigger preloading the next chapter when there is only one page. But it seems that when opening the reader, it does prepare the next chapter even if there is only one page.

@raxod502
Copy link
Contributor Author

raxod502 commented Apr 18, 2024

I think the problem isn't limited to chapters with only one page. I use a particular source where each chapter has exactly two pages, and the problem still occurs. It seems that Mihon does not optimize well for the case of a large number of chapters. It's impossible to swipe between chapters faster than a rate of about one per five seconds, since it needs to repeatedly re-load data, presumably from the chapter index, before the next page is displayed.

The message is defined here:

<string name="transition_pages_loading">Loading pages…</string>

And displayed here:

text = context.stringResource(MR.strings.transition_pages_loading)

That function is called from here:

when (state) {
is ReaderChapter.State.Loading -> setLoading()
is ReaderChapter.State.Error -> setError(state.error)
is ReaderChapter.State.Wait, is ReaderChapter.State.Loaded -> {
// No additional view is added
}
}

Here is a sample of logs (debug logging enabled) for the aforementioned two-page-per-chapter source, which currently has about 2,900 chapters. I am swiping to the next page/chapter as fast as possible. You can see that the maximum throughput is 6.6 seconds per chapter. Most of the time is spent either waiting for the "Loading pages" spinner, or having gestures not be recognized because the app is frozen during the first part of that loading process.

04-17 17:47:34.214 10979 10979 D VerticalPagerViewer: onReaderPageSelected: 2/2
04-17 17:47:34.214 10979 10979 D VerticalPagerViewer: Request preload next chapter because we're at page 2 of 2
04-17 17:47:37.095 10979 10979 I Choreographer: Skipped 159 frames!  The application may be doing too much work on its main thread.
04-17 17:47:37.107 10979 11132 I HWUI    : Davey! duration=2659ms; Flags=0, FrameTimelineVsyncId=66389203, IntendedVsync=298713245691162, Vsync=298715893189350, InputEventId=666364776, HandleInputStart=298715900966980, AnimationStart=298715900967875, PerformTraversalsStart=298715901226380, DrawStart=298715903209656, FrameDeadline=298713262291162, FrameInterval=298715900741150, FrameStartTime=16650932, SyncQueued=298715904146220, SyncStart=298715905162862, IssueDrawCommandsStart=298715905241394, SwapBuffers=298715906021790, FrameCompleted=298715906586243, DequeueBufferDuration=19043, QueueBufferDuration=249186, GpuCompleted=298715906586243, SwapBuffersCompleted=298715906356832, DisplayPresentTime=298696024342296, CommandSubmissionCompleted=298715906021790,
04-17 17:47:37.431 10979 10979 D VerticalPagerViewer: onTransitionSelected: Next(from=/437/, to=/438/)
04-17 17:47:37.431 10979 10979 D VerticalPagerViewer: Request preload destination chapter because we're on the transition
04-17 17:47:37.751 10979 10979 D VerticalPagerViewer: onReaderPageSelected: 1/2
04-17 17:47:37.752 10979 10979 D ReaderViewModel: Setting /438/ as active
04-17 17:47:37.752 10979 11066 D StandaloneCoroutine: Loading /438/
04-17 17:47:38.078 10979 10979 D VerticalPagerViewer: onReaderPageSelected: 2/2
04-17 17:47:40.284 10979 10979 I Choreographer: Skipped 113 frames!  The application may be doing too much work on its main thread.
04-17 17:47:40.304 10979 11132 I HWUI    : Davey! duration=1889ms; Flags=0, FrameTimelineVsyncId=66390364, IntendedVsync=298717208112591, Vsync=298719089368909, InputEventId=494521517, HandleInputStart=298719090028831, AnimationStart=298719090037986, PerformTraversalsStart=298719090295147, DrawStart=298719091967999, FrameDeadline=298717241362833, FrameInterval=298719089518414, FrameStartTime=16648286, SyncQueued=298719092821311, SyncStart=298719093943788, IssueDrawCommandsStart=298719094023256, SwapBuffers=298719098604636, FrameCompleted=298719098942364, DequeueBufferDuration=17416, QueueBufferDuration=228597, GpuCompleted=298719098942364, SwapBuffersCompleted=298719098932883, DisplayPresentTime=298697289866914, CommandSubmissionCompleted=298719098604636,
04-17 17:47:40.418 10979 10979 D VerticalPagerViewer: onTransitionSelected: Next(from=/438/, to=/439/)
04-17 17:47:40.418 10979 10979 D VerticalPagerViewer: Request preload destination chapter because we're on the transition
04-17 17:47:40.418 10979 11043 D ReaderViewModel: Preloading /439/
04-17 17:47:40.418 10979 11043 D UndispatchedCoroutine: Loading pages for 439 - Redacted
04-17 17:47:44.367 10979 10979 D VerticalPagerViewer: onReaderPageSelected: 1/2
04-17 17:47:44.367 10979 10979 D ReaderViewModel: Setting /439/ as active
04-17 17:47:44.367 10979 11060 D StandaloneCoroutine: Loading /439/
04-17 17:47:44.368 10979 10979 D ReaderChapter: Recycling chapter 437 - Redacted
04-17 17:47:44.710 10979 10979 D VerticalPagerViewer: onReaderPageSelected: 2/2
04-17 17:47:44.710 10979 10979 D VerticalPagerViewer: Request preload next chapter because we're at page 2 of 2
04-17 17:47:44.710 10979 11051 D ReaderViewModel: Preloading /440/
04-17 17:47:44.710 10979 11051 D UndispatchedCoroutine: Loading pages for 440 - Redacted
04-17 17:47:44.820 10979 11003 W System  : A resource failed to call close.
04-17 17:47:46.974 10979 10979 D VerticalPagerViewer: onTransitionSelected: Next(from=/439/, to=/440/)
04-17 17:47:46.974 10979 10979 D VerticalPagerViewer: Request preload destination chapter because we're on the transition
04-17 17:47:46.978 10979 10979 I Choreographer: Skipped 122 frames!  The application may be doing too much work on its main thread.
04-17 17:47:46.996 10979 11132 I HWUI    : Davey! duration=2036ms; Flags=0, FrameTimelineVsyncId=66393430, IntendedVsync=298723750885701, Vsync=298725781578263, InputEventId=739249126, HandleInputStart=298725783630274, AnimationStart=298725783631332, PerformTraversalsStart=298725783825872, DrawStart=298725785463526, FrameDeadline=298723784144738, FrameInterval=298725783416000, FrameStartTime=16645021, SyncQueued=298725786175522, SyncStart=298725787162176, IssueDrawCommandsStart=298725787246974, SwapBuffers=298725788255722, FrameCompleted=298725788769435, DequeueBufferDuration=16398, QueueBufferDuration=242229, GpuCompleted=298725788769435, SwapBuffersCompleted=298725788591741, DisplayPresentTime=298721780402611, CommandSubmissionCompleted=298725788255722,
04-17 17:47:49.563 10979 10979 D VerticalPagerViewer: onReaderPageSelected: 1/2
04-17 17:47:49.563 10979 10979 D ReaderViewModel: Setting /440/ as active
04-17 17:47:49.564 10979 11015 D StandaloneCoroutine: Loading /440/
04-17 17:47:49.564 10979 10979 D ReaderChapter: Recycling chapter 438 - Redacted
04-17 17:47:49.700 10979 11003 W System  : A resource failed to call close.
04-17 17:47:49.900 10979 10979 D VerticalPagerViewer: onReaderPageSelected: 2/2
04-17 17:47:52.327 10979 10979 I Choreographer: Skipped 125 frames!  The application may be doing too much work on its main thread.
04-17 17:47:52.333 10979 10979 D VerticalPagerViewer: onTransitionSelected: Next(from=/440/, to=/441/)
04-17 17:47:52.334 10979 10979 D VerticalPagerViewer: Request preload destination chapter because we're on the transition
04-17 17:47:52.334 10979 11047 D ReaderViewModel: Preloading /441/
04-17 17:47:52.334 10979 11047 D UndispatchedCoroutine: Loading pages for 441 - Redacted
04-17 17:47:52.339 10979 11117 I HWUI    : Davey! duration=2094ms; Flags=0, FrameTimelineVsyncId=66395736, IntendedVsync=298729044697566, Vsync=298731125314816, InputEventId=86723367, HandleInputStart=298731133297391, AnimationStart=298731133588936, PerformTraversalsStart=298731133802437, DrawStart=298731135768745, FrameDeadline=298729061297566, FrameInterval=298731132876696, FrameStartTime=16644938, SyncQueued=298731136613594, SyncStart=298731137544787, IssueDrawCommandsStart=298731137618436, SwapBuffers=298731138534126, FrameCompleted=298731139772367, DequeueBufferDuration=16398, QueueBufferDuration=229777, GpuCompleted=298731139772367, SwapBuffersCompleted=298731138842273, DisplayPresentTime=298726990978745, CommandSubmissionCompleted=298731138534126,
04-17 17:47:57.629 10979 10979 D VerticalPagerViewer: onReaderPageSelected: 1/2
04-17 17:47:57.629 10979 10979 D ReaderViewModel: Setting /441/ as active
04-17 17:47:57.629 10979 11047 D StandaloneCoroutine: Loading /441/
04-17 17:47:57.630 10979 10979 D ReaderChapter: Recycling chapter 439 - Redacted
04-17 17:47:58.061 10979 10979 D VerticalPagerViewer: onReaderPageSelected: 2/2
04-17 17:47:58.061 10979 10979 D VerticalPagerViewer: Request preload next chapter because we're at page 2 of 2
04-17 17:47:58.061 10979 11260 D ReaderViewModel: Preloading /442/

The performance analysis above is corroborated by the fact that Mihon hangs completely on a black screen for about 5 seconds when selecting any chapter from the index. So, basically, loading a chapter takes 5-6 seconds when you have about 2,900 chapters. There is clearly some performance issue to be addressed, since the required work to load a single page should be in the milliseconds. (Sources with small chapter counts do not have any of the above issues.)

@scb261
Copy link

scb261 commented Apr 18, 2024

The speed of preparing a chapter, probably, should be a separate issue, if an issue at all.

In the case of 1-page chapters, it doesn't prepare chapters at all.

In the case of 2-page chapters, as I can see from the logs above, preparing the next chapter happens on the second page. It means that with a normal reading speed, it shouldn't be an issue. I have also tested this on my side with 2-page chapters, and it works fine if you scroll at a regular speed.

@raxod502
Copy link
Contributor Author

It means that with a normal reading speed, it shouldn't be an issue

Unfortunately, that doesn't quite turn out to be how it works, for two reasons:

  1. The chapter loading sometimes completely freezes the app, so you can't zoom or pan for some seconds. (In cases where Mihon tries to repeatedly load a chapter, for example in cases where the source cannot provide working data for specific chapters, the Android system has intervened and suggested that I kill the app for nonresponsivity. Stack traces and log entries suggest that the app isn't responding in a timely manner to user inputs.
  2. With the two-page-per-chapter source that I use, the second page of each chapter is the caption for the first page, which does not typically take more than five seconds to read. This means one must intentionally wait for the loading to finish before one can move to the next chapter.

There are also plenty of one-page-per-chapter sources where each page is small enough to take less than 5-10 seconds to read, e.g. a single panel per page. The long chapter loading times combine with the fallback behavior (to show a transition page when loading has not finished even if the user has disabled transition pages) to produce the experience described in the original post.

The speed of preparing a chapter, probably, should be a separate issue, if an issue at all

Well, the app freezing for more than 5 seconds every time you change pages is quite an issue in my personal opinion, but it only affects a subset of sources.

I only found out that slow chapter loading times were the cause when investigating this issue, which is why it's here. If the maintainers desire for that sub-problem to be split out into a separate issue I am happy to do so.

@raxod502
Copy link
Contributor Author

Okay, I finally found the explicit logic that adds a transition when the next chapter is not loaded yet:

// Add next chapter transition and pages.
nextTransition = ChapterTransition.Next(chapters.currChapter, chapters.nextChapter)
.also {
if (nextHasMissingChapters || forceTransition ||
chapters.nextChapter?.state !is ReaderChapter.State.Loaded
) {
newItems.add(it)
}
}

I'm looking into the loading times, but I'm also looking into why it takes so long to initiate a download queue with a large number of chapters (because that is getting in the way of my testing the first issue)... this appears to be an error in the usage of UniFile where unneeded data is repeatedly retrieved. I'll file a separate issue or pull request about that one and then come back to debugging this problem.

@scb261
Copy link

scb261 commented Apr 19, 2024

Ok, considering your description and the number of chapters, I now realized that we are testing it on the same library entry. I scroll it 1 page per second, loads fine for me. It looks more like a device-specific issue if it takes 5 seconds to load it. Yet again, it should be a separate issue if an issue at all.

@raxod502
Copy link
Contributor Author

It turns out that the performance degrades linearly with the number of downloaded chapters due to an oversight in the UniFile library findFile implementation. If you have 2,400 downloaded chapters then it takes about 10-15 seconds to load a single chapter, or to initiate the download for a single chapter.

I will file a new issue or pull request for this issue, which in my opinion is a serious problem (initiating the download for a chapter should take milliseconds, not 15 seconds, and the delay is serial per chapter, so initiating download for 100 chapters takes 20 minutes of full CPU utilization for the phone - it makes downloading near-unusable for large numbers of chapters).

I doubt strongly that this is a device-specific issue given that I can point to the specific code that is causing the problem, which has clear logic errors. More details to follow.

@raxod502
Copy link
Contributor Author

I have filed #705 and will start working on a plan for how the filesystem performance could be improved. Any insight is appreciated, but regardless I will propose some specific changes in the other issue thread for review before implementing.

@raxod502
Copy link
Contributor Author

As mentioned in #705 (comment), I expect that #728 and tachiyomiorg/UniFile#2, together, may fix this problem as well as the issue with download performance, since I believe they both have the same root cause. Now that it is actually possible for me to download a large number of chapters to produce an accurate test case, I will verify whether there are any further optimizations which are needed before chapter transitions become seamless.

@raxod502
Copy link
Contributor Author

raxod502 commented May 4, 2024

This issue is resolved by #728, as explained in #705 (comment).

The behavior of this setting is still unintuitive, I would argue that when chapter transitions are disabled, a loading screen rather than a chapter transition should be displayed if a chapter is not yet loaded, but this specific issue (there being no actual way to disable transitions properly when a large number of chapters have been downloaded) is resolved. I may file a separate, more specific issue for other usability improvements.

@raxod502 raxod502 closed this as completed May 4, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Feature request New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants