Conversation
modesty
left a comment
modesty
left a comment
There was a problem hiding this comment.
some adjustment needed, see inline comments for details
| tjX += pieceWidth; | ||
| } else { | ||
| tjX -= items[j]; | ||
| } |
There was a problem hiding this comment.
ln 912-919: Summing glyph widths alone ignores character and word spacing and can cause regression. Example: Courier, 10pt, 10 Tc [(A) 700 (B)] TJ. :
Tc adds 10 units of character spacing. After drawing A, the actual cursor advances 6 + 10 = 16 units. The 700 adjustment then moves it back 7 units, so B starts at x = 9, to the right of A.
With this change, it counts only the glyph width: 6 − 7 = −1, results in changing extraction from AB to BA.
| // Stable sort by x so reverse-laid runs come out in reading order | ||
| // while same-x or already-forward pieces keep their original order. | ||
| pieces.sort(function (a, b) { return a.x - b.x; }); | ||
| var prevEnd = null; |
There was a problem hiding this comment.
spatially reordering RTL test should be avoided before bidi processing. For a reverse-positioned Hebrew run, this sort puts the characters into left-to-right visual order, but the subsequent PDFJS.bidi(chunk, -1, font.vertical) reorders them again.
spatial reconstruction should be made direction-aware and coordinate with the existing bidi conversion.
2 participants
Relates to #421
After some chasing of code, specifications and what not, I did what one's expected to do these days and got it done with AI. The fix managed to produce correct output locally with no visible problems or regressions elsewhere. In all fairness, I didn't write the code, but I had a very good look over it and most importantly - it works. I ran the tests here on node.js (no deno or bun) and they passed. I haven't added to the tests as I don't have a suitable file but if I find one I will add it there (
test/pdf/misc). I'm keen to amend the tests if you could suggest some proper way though.Cheers