johnfactotum / foliate-js

Render e-books in the browser
https://johnfactotum.github.io/foliate-js/reader.html
MIT License
318 stars 43 forks source link

Mis-sized / aligned columns on device with non-integer scale factor #2

Closed rkjnsn closed 1 year ago

rkjnsn commented 1 year ago

I use a non-integer scale factor on my laptop, which I'm guessing might be the cause of this bug. (I'm not entirely sure I understand all the factors that go into how Firefox determines the scale factor to use, but it seems to have arrived at a devicePixelRatio of 1.7647058823529411 with my current settings.)

When viewing a reflowable ebook on the first page of a chapter, the right column is cut off: screenshot1

The layout on the next page is then shifted, showing the rest of the right column from the previous page, and cutting off even more of the new right column: screenshot2

Subsequent page turns continue shifting by increasing amounts.

johnfactotum commented 1 year ago

This doesn't seem to have anything to do with device pixel ratio. It seems to occur only when setting the zoom level to something other than 100%, and only on specific combinations of zoom levels, viewport height, and content size. The bug doesn't occur if the zoom level is one, whether device pixel ratio is integer or not.

It also occurs on WebKitGTK, but it seems much harder to reproduce than Firefox. It seems to require really specific zoom levels like 59% or 72% or something of the sort. So perhaps device pixel ratio does affect it somewhat, as WebKitGTK only does integer scaling (I think).

I'm not sure why this is happening, but it seems to have something to do with expanding the iframe. And it seems that it can be fixed by:

diff --git a/paginator.js b/paginator.js
index 9ff9b63..0f114cb 100644
--- a/paginator.js
+++ b/paginator.js
@@ -278,6 +278,8 @@ class View {
             this.#element.style[side] = `${expandedSize}px`
             this.#iframe.style[otherSide] = '100%'
             this.#element.style[otherSide] = '100%'
+            if (this.document)
+                this.document.documentElement.style[side] = `${expandedSize}px`
             if (this.#overlayer) {
                 this.#overlayer.element.style.margin = '0'
                 this.#overlayer.element.style[side] = `${expandedSize}px`

There is another bug when the zoom level is not 100%: it sometimes fails to go to the next section. I believe this can be fixed by simply rounding when calculating the number of pages.

diff --git a/paginator.js b/paginator.js
index 9ff9b63..68217f9 100644
--- a/paginator.js
+++ b/paginator.js
@@ -423,7 +423,7 @@ export class Paginator {
         return Math.floor(((this.start + this.end) / 2) / this.size)
     }
     get pages() {
-        return this.viewSize / this.size
+        return Math.round(this.viewSize / this.size)
     }
     // allows one to process rects as if they were LTR and horizontal
     #getRectMapper() {
johnfactotum commented 1 year ago

It seems for whatever reason, it's still consistently broken on Firefox if the zoom level is 30% and the content is more than one screenful.

johnfactotum commented 1 year ago

The problem seems to go away if you floor the column-width:

diff --git a/paginator.js b/paginator.js
index 8849c94..c42bc37 100644
--- a/paginator.js
+++ b/paginator.js
@@ -240,7 +240,7 @@ class View {
         const doc = this.document
         Object.assign(doc.documentElement.style, {
             boxSizing: 'border-box',
-            columnWidth: `${columnWidth}px`,
+            columnWidth: `${Math.trunc(columnWidth)}px`,
             columnGap: `${gap}px`,
             columnFill: 'auto',
             ...(vertical

But I still don't understand why it's happening or why flooring it helps.