Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Unreleased

- Fix null values in table cells rendering as `[object Object]`
- Fix further LineWrapper precision issues

### [v0.17.0] - 2025-04-12

Expand Down
6 changes: 3 additions & 3 deletions lib/line_wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ class LineWrapper extends EventEmitter {
}

wordWidth(word) {
return (
return PDFNumber(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not be rounded only when storing the number to the pdf file?

wordWidth is called in many intermediate steps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to ensure that during the line_wrapper computations we are keeping a float32 number so we could either wrap all instances of wordWidth with the rounder, or just do it here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or alternatively we wrap every comparison operation with PDFNumber to ensure the rounding never affects it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or alternatively we wrap every comparison operation with PDFNumber to ensure the rounding never affects it

Lets take the current approach, adding tests to ensure correctness. Later we can try to minimize number of roundings

this.document.widthOfString(word, this) +
this.characterSpacing +
this.wordSpacing
this.characterSpacing +
this.wordSpacing,
);
}

Expand Down
25 changes: 22 additions & 3 deletions lib/utils.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,27 @@
export function PDFNumber(n) {
const fArray = new Float32Array(1);
const uArray = new Uint32Array(fArray.buffer);

export function PDFNumber(n, roundUp = false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this function called with roundUp = true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by default it will roundDown

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could simplify and only implement roundDown. If needed later implement roundUp

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only reason i left both was that if we want to do float64 > float32 it would need to be rounded up

// PDF numbers are strictly 32bit
// so convert this number to the nearest 32bit number
// so convert this number to a 32bit number
// @see ISO 32000-1 Annex C.2 (real numbers)
return Math.fround(n);
const rounded = Math.fround(n);
if (roundUp) {
if (rounded >= n) return rounded;
} else if (rounded <= n) return rounded;

// Will have to perform 32bit float truncation
fArray[0] = n;

// Get the 32-bit representation as integer and shift bits
if (!(roundUp ^ (n > 0))) {
uArray[0] += 1;
} else {
uArray[0] -= 1;
}

// Return the float value
return fArray[0];
}

/**
Expand Down
30 changes: 29 additions & 1 deletion tests/unit/utils.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { normalizeSides } from '../../lib/utils';
import { normalizeSides, PDFNumber } from '../../lib/utils';

describe('normalizeSides', () => {
test.each([
Expand Down Expand Up @@ -54,3 +54,31 @@ describe('normalizeSides', () => {
});
});
});

describe('PDFNumber', () => {
test.each([
[0, 0],
[0.04999999701976776], //float32 rounded down
[0.05],
[0.05000000074505806], //float32 rounded up
[1],
[-1],
[-5.05],
[5.05],
])('PDFNumber(%f) -> %f', (n) => {
expect(PDFNumber(n)).toBeLessThanOrEqual(n);
expect(PDFNumber(n, false)).toBeLessThanOrEqual(n);
});
test.each([
[0],
[0.04999999701976776], //float32 rounded down
[0.05],
[0.05000000074505806], //float32 rounded up
[1],
[-1],
[-5.05],
[5.05],
])('PDFNumber(%f, true) -> %f', (n) => {
expect(PDFNumber(n, true)).toBeGreaterThanOrEqual(n);
});
});