Repository navigation
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.
There was a problem hiding this comment.
Right, so as it often goes with AI - it's quick and easy, and it looks like working but it doesn't really ... So I went and learned some more about how PDF works (your directions above helped to know what to look for).
Now each glyph also has the character spacing added to its own width (considering the current fontDirection). I also learned how to generate a simple PDF and I added a test for the code.
| // 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.
There was a problem hiding this comment.
I'd be grateful for an example file that I can try and see what happens. What bothers me is that I created a document with /Direction /R2L and with all characters printed in reverse. I guess this is how R2L scripts are printed ... And the code, with no changes, prints them out in reverse order which looks correct to me. Am I missing something - if a word with characters A->B->C is to be printed R2L then it'd likely look like "CBA" on screen and it is the "CBA" text that the parser needs produce. I can't get how the R2L directive changes anything here, so I need a bit more guidance :)
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