futurepress / epub.js

Enhanced eBooks in the browser.
http://futurepress.org
Other
6.39k stars 1.09k forks source link

Columns shift right when switching pages #1218

Open cthu1hoo opened 2 years ago

cthu1hoo commented 2 years ago

I have an application (https://git.tt-rss.org/fox/the-epube) using EpubJS 0.3.88. Every time pages are switched, column shifts slightly to the right, after repeating this a few times text column moves out of viewport and previous column is shown, it looks like this:

image

This seems to only happen in a single column mode. I'm somewhat at a loss at how to investigate this further. I remember this working properly before so it could be some kind of Chrome update.

Epub is rendered into an iframe, I've tried to remove all theming (i.e. fonts, sizes) but it didn't help.

OriIdan commented 2 years ago

This happens because the offset is not calculated correctly. Try different window size, this may help. We had a similar problem, what we ended up doing was rerendering every 20 pages or so.

-- Ori Idan CEO Helicon Books http://www.heliconbooks.com

On Sun, Sep 12, 2021 at 6:49 PM cthu1hoo @.***> wrote:

I have an application (https://git.tt-rss.org/fox/the-epube) using EpubJS 0.3.88. Every time pages are switched, column shifts slightly to the right, after repeating this a few times text column moves out of viewport and previous column is shown, it looks like this:

[image: image] https://user-images.githubusercontent.com/47687909/132994005-401910a1-54fa-42b1-a173-b8305ce571b3.png

This seems to only happen in a single column mode. I'm somewhat at a loss at how to investigate this further. I remember this working properly before so it could be some kind of Chrome update.

Epub is rendered into an iframe, I've tried to remove all theming (i.e. fonts, sizes) but it didn't help.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/futurepress/epub.js/issues/1218, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB43QFOSBIJ3DN5C3X6IRLUBTDYLANCNFSM5D4HEFWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

cthu1hoo commented 2 years ago

Changing window size won't be possible since it's a phone. Could you post an example of how you re-render the view? Thanks.

OriIdan commented 2 years ago

‎ׂFirst find the offset of a page (the number added or subtracted when paging) and increment (or decrement it by 1). This should help. In addition, I asked our developer to point me to the lines of code she used, and I will post here her answer as soon as I get it (it will probably be Tuesday morning).

On Sun, Sep 12, 2021 at 7:14 PM cthu1hoo @.***> wrote:

Changing window size won't be possible since it's a phone. Could you post an example of how you re-render the view? Thanks.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/futurepress/epub.js/issues/1218#issuecomment-917665134, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB43QDDKPNUV4FIUIFFF6TUBTGWNANCNFSM5D4HEFWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

cthu1hoo commented 2 years ago

Thanks in advance.

‎ׂFirst find the offset of a page (the number added or subtracted when paging) and increment (or decrement it by 1).

This sort of rounding error could be different between devices (i.e. screen sizes) so its unlikely to fix things properly for everyone.

OriIdan commented 2 years ago

We did it based on screen size, so only for sizes we have found the problem. Later on we added the rerendering that I will send you the code later.

On Sun, Sep 12, 2021 at 7:25 PM cthu1hoo @.***> wrote:

Thanks in advance.

‎ׂFirst find the offset of a page (the number added or subtracted when paging) and increment (or decrement it by 1).

This sort of rounding error could be different between devices (i.e. screen sizes) so its unlikely to fix things properly for everyone.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/futurepress/epub.js/issues/1218#issuecomment-917666943, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB43QASALPDJEV6MHRFBW3UBTIABANCNFSM5D4HEFWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

OriIdan commented 2 years ago

The offset is: this.layout.delta

On Sun, Sep 12, 2021 at 7:27 PM Ori Idan @.***> wrote:

We did it based on screen size, so only for sizes we have found the problem. Later on we added the rerendering that I will send you the code later.

On Sun, Sep 12, 2021 at 7:25 PM cthu1hoo @.***> wrote:

Thanks in advance.

‎ׂFirst find the offset of a page (the number added or subtracted when paging) and increment (or decrement it by 1).

This sort of rounding error could be different between devices (i.e. screen sizes) so its unlikely to fix things properly for everyone.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/futurepress/epub.js/issues/1218#issuecomment-917666943, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB43QASALPDJEV6MHRFBW3UBTIABANCNFSM5D4HEFWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

cthu1hoo commented 2 years ago

Thanks; On one of my test affected widths (502px) book.rendition.manager.layout.delta += 2 seems to be doing the trick. Still, this is a really ugly hack, all things considered.

OriIdan commented 2 years ago

What we did was to count the number of page turns. Following is a simplified code of what we have done.

rendition.on("relocated" func(loc) { chap = loc.end.href; if((prevChap == null) || (prevChap != chap)) { prevChap = chap; swipes = 0; } else if(swipes >= 20) { swipes = 0; rendition.resize(); } }

Our actual code is much more complicated as we do a lot more. The idea is that if this is a new chapter (new spine item) there is no need to re render as it is done anyway by epub.js if we are still in the same spine item for more than 20 page turns (swipes) we re render.

On Mon, Sep 13, 2021 at 8:24 AM cthu1hoo @.***> wrote:

Thanks; On one of my test affected widths (502px) book.rendition.manager.layout.delta += 2 seems to be doing the trick. Still, this is a really ugly hack, all things considered.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/futurepress/epub.js/issues/1218#issuecomment-917852329, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB43QDKVGVSVOGDWVVKFV3UBWDHVANCNFSM5D4HEFWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

cthu1hoo commented 2 years ago

Hmmm. To me, rendition.resize() does nothing unless a different (from current) width is passed to the method. If I do something like

rendition.resize($("#reader").width()+1)
rendition.resize($("#reader").width())

to force it, I run into another problem with Epubjs, that is, it seems that getting current precise location from the library is impossible (even the library itself is unaware of it), so any attempts to render current page again or open current CFI again might open an incorrect page because current location is returned as an imprecise range. Could you maybe advise me with this? Thanks.

OriIdan commented 2 years ago

Strange, we have tested it and for us it solved the problem of imprecise offset calculation. rendition.resize(); display(currCfi);

I did not write the code myself, I am just copying the code from our application.

On Tue, Sep 14, 2021 at 6:11 PM cthu1hoo @.***> wrote:

Hmmm. To me, rendition.resize() does nothing unless a different (from current) width is passed to the method. If I do something like

rendition.resize($("#reader").width()+1) rendition.resize($("#reader").width())

to force it, I run into another problem with Epubjs, that is, it seems that getting current precise location from the library is impossible (even the library itself is unaware of it), so any attempts to render current page again or open current CFI again might open an incorrect page because current location is returned as an imprecise range. Could you maybe advise me with this? Thanks.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/futurepress/epub.js/issues/1218#issuecomment-919244962, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB43QDD4XVWGF52CGU3W7TUB5QYJANCNFSM5D4HEFWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

cthu1hoo commented 2 years ago

Strange. Which exact Epubjs version are you using?

Edit: I've tried on 0.3.73, 0.3.88, and latest git master, and .resize() with no arguments doesn't seem to do anything for me.

OriIdan commented 2 years ago

We have tested it on all new versions. I am not sure what version exactly we use now.

On Tue, Sep 14, 2021 at 6:22 PM cthu1hoo @.***> wrote:

Strange. Which exact Epubjs version are you using?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/futurepress/epub.js/issues/1218#issuecomment-919255453, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB43QH6W3HFA6UYHJRZOL3UB5SCBANCNFSM5D4HEFWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

wuyu8512 commented 2 years ago

You need to limit the width to a multiple of 8

SamuelPremji commented 1 year ago

It happens to me when I'm selecting some text. Any fix?

cappallo commented 5 months ago

As near as I can tell as of v3.93, the issue is still around and has to do with line 113 in src/layout.js:

var section = Math.floor(width / 12);

I rounded my width when I call renderTo to be a multiple of 12, and it seems to have stopped drifting. Not ideal, though.