From ed7cfc31fc242f91c6e40a523b920c1d8ef724ab Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Fri, 10 Apr 2026 02:08:55 +0800 Subject: [PATCH] fix(layout): fix off-by-one page count on fractional DPR devices, closes #3787 (#3813) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `pages` getter used Math.ceil(viewSize / containerSize) which inflates the count by 1 when floating-point drift makes the ratio slightly above an integer (e.g. 4.00000001 → 5). Use Math.round to absorb sub-pixel drift, matching the approach already used in #getPagesBeforeView. Co-authored-by: Claude Opus 4.6 (1M context) --- .../document/paginator-multiview.test.ts | 34 +++++++++++++++++++ packages/foliate-js | 2 +- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/apps/readest-app/src/__tests__/document/paginator-multiview.test.ts b/apps/readest-app/src/__tests__/document/paginator-multiview.test.ts index bbff5a15..9d1eb2e5 100644 --- a/apps/readest-app/src/__tests__/document/paginator-multiview.test.ts +++ b/apps/readest-app/src/__tests__/document/paginator-multiview.test.ts @@ -278,6 +278,40 @@ describe('Paginator multi-view architecture', () => { // With tolerance, this should detect index 3 (not 2) expect(detectPrimary(visibleStart)).toBe(3); }); + + it('should not inflate page count due to fractional DPR rounding (pages getter)', () => { + // On fractional DPR devices, getBoundingClientRect() on the view element + // returns a width that is a near-integer multiple of the container size, + // but with tiny floating-point drift. Math.ceil inflates the count by 1. + // Real scenario: containerSize=785.45458984375, viewSize should be + // exactly 4*containerSize but getBoundingClientRect() returns a value + // with sub-pixel error. + const containerSize = 785.45458984375; + const viewSize = containerSize * 4 + 0.0001; // tiny FP drift + + // Bug: Math.ceil gives 5 instead of 4 + const buggyPages = Math.ceil(viewSize / containerSize); + expect(buggyPages).toBe(5); // confirms the bug exists + + // Fix: Math.round absorbs sub-pixel drift + const fixedPages = Math.round(viewSize / containerSize); + expect(fixedPages).toBe(4); + }); + + it('should not inflate rendered page count due to fractional DPR rounding', () => { + // Same issue for #renderedPages which sums multiple view sizes + const containerSize = 785.45458984375; + const viewSizes = [containerSize * 3 + 0.00005, containerSize * 2 + 0.00008]; + const totalViewSize = viewSizes.reduce((a, b) => a + b, 0); + + // Bug: Math.ceil over-counts + const buggyPages = Math.ceil(totalViewSize / containerSize); + expect(buggyPages).toBe(6); // should be 5 but ceil rounds up + + // Fix: Math.round + const fixedPages = Math.round(totalViewSize / containerSize); + expect(fixedPages).toBe(5); + }); }); describe('Adjacent index with fromIndex parameter', () => { diff --git a/packages/foliate-js b/packages/foliate-js index fe33bb51..d3baa52d 160000 --- a/packages/foliate-js +++ b/packages/foliate-js @@ -1 +1 @@ -Subproject commit fe33bb510871bd0489493c49638a5b82c1de5df5 +Subproject commit d3baa52d05047b2aa02779294f02a81fac855188