Make use of textAdvanceScale consistent during combineTextItems. Fix for #7878.

[rossj]

Currently, while generating the text content of a page, multiple text draw operations may be combined into a single textContentItem. The running size of this textContentItem is kept as text is added and as the text transformation is changed; however, the summing of this size inconsistently applies the textAdvanceScale factor.

This pull request fixes the discrepancy by only using the textAdvanceScale upon completion / flushing of the textContentItem. The result is that the textContentItem's size is calculated correctly, improving text highlighting in cases like #7878.

More details:

Currently, textAdvanceScale is applied when adding the width / height of actual text content here. The textAdvanceScale is not applied when accounting for size changes due to text advance transformation changes, such as here.

I believe this issue might not appear often as it requires there to be a non-zero difference between the text advance distance and the previously calculated lastAdvanceWidth in order for the non-application of the scaling factor to have an effect.

[timvandermeij]

/botio-linux preview

[pdfjsbot]

From: Bot.io (Linux)

---

Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/77b19eac6be9ae1/output.txt

[pdfjsbot]

From: Bot.io (Linux)

---

Success

Full output at http://107.21.233.14:8877/77b19eac6be9ae1/output.txt

Total script time: 2.27 mins

Published

• Viewer: http://107.21.233.14:8877/77b19eac6be9ae1/web/viewer.html
• Extension: http://107.21.233.14:8877/77b19eac6be9ae1/extensions/firefox/pdf.js.xpi

[timvandermeij]

/botio test

[pdfjsbot]

From: Bot.io (Windows)

---

Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.22.172.223:8877/ed74e45fe736535/output.txt

[pdfjsbot]

From: Bot.io (Linux)

---

Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/c720ff5dbd84647/output.txt

[pdfjsbot]

From: Bot.io (Windows)

---

Failed

Full output at http://107.22.172.223:8877/ed74e45fe736535/output.txt

Total script time: 26.45 mins

• Font tests: Passed
• Unit tests: Passed
• Regression tests: FAILED

Image differences available at: http://107.22.172.223:8877/ed74e45fe736535/reftest-analyzer.html#web=eq.log

[pdfjsbot]

From: Bot.io (Linux)

---

Failed

Full output at http://107.21.233.14:8877/c720ff5dbd84647/output.txt

Total script time: 26.76 mins

• Font tests: Passed
• Unit tests: Passed
• Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/c720ff5dbd84647/reftest-analyzer.html#web=eq.log

[timvandermeij]

The tests reveal some unexpected changes. In particular, firefox-issue1936-text-page1 seems bad. Could you check why that is the case?

[rossj]

@you are right about the issues with some files -- sorry for not noticing these.

I tracked the issue down to the fact that textMatrix scaling was already being applied to TJ offsets during processing of the showSpacedText op, which I was then re-applying when "flushing" the textContentItem. Removing these first scaling operations fixes the problem files.

I should mention that I do find the scaling operations that I removed a bit odd, and I don't quite understand mathematically how they correspond with the equations in section 9.4.4 "Text Space Details" of the PDF spec. I believe my new updates are in line with the provided text matrix equations, but I could be missing something.

[timvandermeij]

Looks good. Could you add the file from #7878 as a regression test? Check https://github.com/mozilla/pdf.js/pull/7714/files as an example of how to add a test case. You have to do three things: update .gitignore to ignore the PDF file, add the PDF file itself and add an entry to test_manifest.json like the one over there. This helps us to prevent any regressions in the future.

When that is done, please squash the commits into one commit. See https://github.com/mozilla/pdf.js/wiki/Squashing-Commits for how to do that easily. Thanks!

[rossj]

Sure, I'll make those changes. Is it best to name the new test file after the issue or to give it a more descriptive name?

I've also just noticed that evaluator.js isn't properly detecting the font in the #7878 file as monospace, despite the FixedPitch font flag being set and the Font providing a single W width. This results in the default glyph width of 1000 being used instead of the correct width of 600. This doesn't effect highlighting the #7878 PDF very much because each glyph is essentially positioned explicitly, but I can imagine for longer string draws the calculated text/highlight width would be much too large.

Do you suggest I look into the monospace detection as part of this PR or should I make a new issue?

[brendandahl]

Is it best to name the new test file after the issue or to give it a more descriptive name?

Issue number is preferred for referencing it later.

Do you suggest I look into the monospace detection as part of this PR or should I make a new issue?

New PR would be best.

[rossj]

Squashed. Please let me know if anything else should be changed. Thanks!

[timvandermeij]

/botio-linux preview

[pdfjsbot]

From: Bot.io (Linux)

---

Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/b8d955a2ca1c613/output.txt

[pdfjsbot]

From: Bot.io (Linux)

---

Success

Full output at http://107.21.233.14:8877/b8d955a2ca1c613/output.txt

Total script time: 2.20 mins

Published

• Viewer: http://107.21.233.14:8877/b8d955a2ca1c613/web/viewer.html
• Extension: http://107.21.233.14:8877/b8d955a2ca1c613/extensions/firefox/pdf.js.xpi

[timvandermeij]

/botio test

[pdfjsbot]

From: Bot.io (Windows)

---

Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.22.172.223:8877/7f8db47a07acdba/output.txt

[pdfjsbot]

From: Bot.io (Linux)

---

Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/1d42593fde57d1c/output.txt

[pdfjsbot]

From: Bot.io (Windows)

---

Failed

Full output at http://107.22.172.223:8877/7f8db47a07acdba/output.txt

Total script time: 26.01 mins

• Font tests: Passed
• Unit tests: Passed
• Regression tests: FAILED

Image differences available at: http://107.22.172.223:8877/7f8db47a07acdba/reftest-analyzer.html#web=eq.log

[pdfjsbot]

From: Bot.io (Linux)

---

Failed

Full output at http://107.21.233.14:8877/1d42593fde57d1c/output.txt

Total script time: 26.28 mins

• Font tests: Passed
• Unit tests: Passed
• Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/1d42593fde57d1c/reftest-analyzer.html#web=eq.log

[yurydelendik]

/botio makeref

Thank you for the patch!

[pdfjsbot]

From: Bot.io (Linux)

---

Received

Command cmd_makeref from @yurydelendik received. Current queue size: 0

Live output at: http://107.21.233.14:8877/1380ed48fb3de45/output.txt

[pdfjsbot]

From: Bot.io (Windows)

---

Received

Command cmd_makeref from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/024b0ad867c5812/output.txt

[pdfjsbot]

From: Bot.io (Linux)

---

Success

Full output at http://107.21.233.14:8877/1380ed48fb3de45/output.txt

Total script time: 24.98 mins

• Lint: Passed
• Make references: Passed
• Check references: Passed

[pdfjsbot]

From: Bot.io (Windows)

---

Success

Full output at http://107.22.172.223:8877/024b0ad867c5812/output.txt

Total script time: 25.49 mins

• Lint: Passed
• Make references: Passed
• Check references: Passed