From 40a0f383a71efde376305eebb1077a0415726089 Mon Sep 17 00:00:00 2001 From: Jake Holland Date: Sat, 11 Jan 2025 12:16:17 +0000 Subject: [PATCH] Fix precision rounding issues in LineWrapper (#1583) Handle JS quirks with large decimal precision checks resulting from the calculations of next lines in the LineWrapper --- CHANGELOG.md | 4 +++ lib/line_wrapper.js | 9 ++--- lib/utils.js | 6 ++++ tests/unit/line_wrapper.spec.js | 62 +++++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 lib/utils.js create mode 100644 tests/unit/line_wrapper.spec.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 57456725..3454cf26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## pdfkit changelog +### Unreleased + +- Fix precision rounding issues in LineWrapper + ### [v0.16.0] - 2024-12-29 - Update fontkit to 2.0 diff --git a/lib/line_wrapper.js b/lib/line_wrapper.js index a64148c0..d564c345 100644 --- a/lib/line_wrapper.js +++ b/lib/line_wrapper.js @@ -1,5 +1,6 @@ import { EventEmitter } from 'events'; import LineBreaker from 'linebreak'; +import { PDFNumber } from './utils'; const SOFT_HYPHEN = '\u00AD'; const HYPHEN = '-'; @@ -26,9 +27,9 @@ class LineWrapper extends EventEmitter { // calculate the maximum Y position the text can appear at if (options.height != null) { this.height = options.height; - this.maxY = this.startY + options.height; + this.maxY = PDFNumber(this.startY + options.height); } else { - this.maxY = this.document.page.maxY(); + this.maxY = PDFNumber(this.document.page.maxY()); } // handle paragraph indents @@ -230,7 +231,7 @@ class LineWrapper extends EventEmitter { if ( this.height != null && this.ellipsis && - this.document.y + lh * 2 > this.maxY && + PDFNumber(this.document.y + lh * 2) > this.maxY && this.column >= this.columns ) { if (this.ellipsis === true) { @@ -274,7 +275,7 @@ class LineWrapper extends EventEmitter { // if we've reached the edge of the page, // continue on a new page or column - if (this.document.y + lh > this.maxY) { + if (PDFNumber(this.document.y + lh) > this.maxY) { const shouldContinue = this.nextSection(); // stop if we reached the maximum height diff --git a/lib/utils.js b/lib/utils.js new file mode 100644 index 00000000..c58274ad --- /dev/null +++ b/lib/utils.js @@ -0,0 +1,6 @@ +export function PDFNumber(n) { + // PDF numbers are strictly 32bit + // so convert this number to the nearest 32bit number + // @see ISO 32000-1 Annex C.2 (real numbers) + return Math.fround(n); +} diff --git a/tests/unit/line_wrapper.spec.js b/tests/unit/line_wrapper.spec.js new file mode 100644 index 00000000..0d1d83fc --- /dev/null +++ b/tests/unit/line_wrapper.spec.js @@ -0,0 +1,62 @@ +import PDFDocument from "../../lib/document"; +import LineWrapper from '../../lib/line_wrapper'; + +describe("LineWrapper", () => { + let document; + + beforeEach(() => { + document = new PDFDocument({ + compress: false, + margin: 0, + }); + }); + + test("ellipsis is present only on last line of multiline text", () => { + // There is a weird edge case where ellipsis occurs on lines + // in the middle of text due to number rounding errors + // + // There is probably a simpler combination of values but this is one I found in the wild + document.y = 402.1999999999999; + document.fontSize(7.26643598615917) + const wrapper = new LineWrapper(document, {width: 300, height: 50.399999999999864, ellipsis: true}) + let wrapperOutput = ""; + wrapper.on("line", (buffer) => { + wrapperOutput += buffer; + document.y += document.currentLineHeight(true) + }) + wrapper.wrap("- A\n- B\n- C\n- D\n- E\n- F", {}) + expect(wrapperOutput).toBe("- A\n- B\n- C\n- D\n- E\n- F"); + }) + + test("line break is handled correctly when at weird heights", () => { + // There is probably a simpler combination of values but this is one I found in the wild + document.y = 1/3; + document.fontSize(Math.fround(42.3/3)); + let lineHeight = document.currentLineHeight(true); + const wrapper = new LineWrapper(document, {width: 300, height:lineHeight*3}) + let wrapperOutput = ""; + wrapper.on("line", (buffer) => { + wrapperOutput += buffer; + document.y += lineHeight + }) + // Limit to 3/4 lines + wrapper.wrap("A\nB\nC\nD", {}) + expect(wrapperOutput).toBe("A\nB\nC\n"); + }); + + test("line break is handled correctly with ellipsis", () => { + // There is probably a simpler combination of values but this is one I found in the wild + document.y = 1/3; + document.fontSize(Math.fround(42.3/3)); + let lineHeight = document.currentLineHeight(true); + const wrapper = new LineWrapper(document, {width: 300, height:lineHeight*3, ellipsis: true}) + let wrapperOutput = ""; + wrapper.on("line", (buffer) => { + wrapperOutput += buffer; + document.y += lineHeight + }) + // Limit to 3/4 lines + wrapper.wrap("A\nB\nC\nD", {}) + expect(wrapperOutput).toBe("A\nB\nC…"); + }); +});