Skip to content

Commit

Permalink
Fix precision rounding issues in LineWrapper (#1583)
Browse files Browse the repository at this point in the history
Handle JS quirks with large decimal precision checks resulting from the calculations of next lines in the LineWrapper
  • Loading branch information
hollandjake authored Jan 11, 2025
1 parent 2511122 commit 40a0f38
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 4 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## pdfkit changelog

### Unreleased

- Fix precision rounding issues in LineWrapper

### [v0.16.0] - 2024-12-29

- Update fontkit to 2.0
Expand Down
9 changes: 5 additions & 4 deletions lib/line_wrapper.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { EventEmitter } from 'events';
import LineBreaker from 'linebreak';
import { PDFNumber } from './utils';

const SOFT_HYPHEN = '\u00AD';
const HYPHEN = '-';
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
@@ -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);
}
62 changes: 62 additions & 0 deletions tests/unit/line_wrapper.spec.js
Original file line number Diff line number Diff line change
@@ -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…");
});
});

0 comments on commit 40a0f38

Please sign in to comment.