Fix some issues with lineWidth < 1 after transform (bug 1753075, bug 1743245, bug 1710019) #14536

calixteman · 2022-02-05T21:45:44Z

it aims to fix:
previously we were trying to adjust lineWidth to have something correct after the current transform is applied but this approach was not correct because finally the pixel is rescaled with the same factors in both directions.
And sometimes those factors must be different (see bug 1753075).
So the idea of this patch is to apply a scale matrix to the current transform just before setting lineWidth and stroking. This scale matrix is computed in order to ensure that after transform, a pixel will have its two thickness greater than 1.

marco-c · 2022-02-05T22:36:53Z

Is this fixing also the bugs linked to bug 1753075 (1743245 and 1657838) and the issues linked to #13211 (#14521)?
If yes, we might want to add the PDFs from those bugs/issues to the reftests too.

Snuffleupagus

Hopefully @brendandahl can review this, since I don't feel that I know all of this code well enough to do so properly myself.

src/display/canvas.js

calixteman · 2022-02-07T18:53:10Z

/botio test

pdfjsbot · 2022-02-07T18:53:11Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/c42fbebf5cddf10/output.txt

pdfjsbot · 2022-02-07T18:53:11Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/8b2e26e6d5e2944/output.txt

pdfjsbot · 2022-02-07T19:16:30Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/8b2e26e6d5e2944/output.txt

Total script time: 23.29 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 299
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/8b2e26e6d5e2944/reftest-analyzer.html#web=eq.log

pdfjsbot · 2022-02-07T19:18:24Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/c42fbebf5cddf10/output.txt

Total script time: 25.19 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 162

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

Snuffleupagus · 2022-02-07T19:31:26Z

Besides the regressions related to text-stroking, there also seem to be some regressions related to dashed lines in a number of test-cases (e.g. tracemonkey-eq and issue1169 regressed a bit).

calixteman · 2022-02-07T19:46:28Z

Yeah I noticed the ones about the dashed lines but I don't see them locally.

calixteman · 2022-02-07T21:22:37Z

/botio test

pdfjsbot · 2022-02-07T21:22:38Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/406cc048f6e7986/output.txt

pdfjsbot · 2022-02-07T21:22:38Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/28a131d7086d818/output.txt

pdfjsbot · 2022-02-07T21:46:01Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/406cc048f6e7986/output.txt

Total script time: 23.36 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 211
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/406cc048f6e7986/reftest-analyzer.html#web=eq.log

pdfjsbot · 2022-02-07T21:47:26Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/28a131d7086d818/output.txt

Total script time: 24.78 mins

Font tests: Passed
Unit tests: FAILED
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 118

Image differences available at: http://54.193.163.58:8877/28a131d7086d818/reftest-analyzer.html#web=eq.log

Snuffleupagus · 2022-02-07T21:51:40Z

Yeah I noticed the ones about the dashed lines but I don't see them locally.

I've just tested this patch locally on Windows, using a regular screen (no hiDPI), and I can reproduce this directly in the viewer at various zoom levels.

src/display/canvas.js

calixteman · 2022-02-14T17:06:46Z

/botio test

pdfjsbot · 2022-02-14T17:06:48Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/720a4abd1c02227/output.txt

pdfjsbot · 2022-02-14T17:06:48Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/23a4ceccf87846f/output.txt

pdfjsbot · 2022-02-14T17:32:00Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/23a4ceccf87846f/output.txt

Total script time: 25.18 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 107

Image differences available at: http://54.193.163.58:8877/23a4ceccf87846f/reftest-analyzer.html#web=eq.log

pdfjsbot · 2022-02-14T17:32:22Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/720a4abd1c02227/output.txt

Total script time: 25.56 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 183

Image differences available at: http://54.241.84.105:8877/720a4abd1c02227/reftest-analyzer.html#web=eq.log

calixteman · 2022-02-24T17:51:36Z

/botio test

pdfjsbot · 2022-02-24T17:51:38Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/e6ca30b862798b6/output.txt

pdfjsbot · 2022-02-24T17:51:38Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/92225866e595c28/output.txt

pdfjsbot · 2022-02-24T18:15:33Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/e6ca30b862798b6/output.txt

Total script time: 23.91 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  errors: 14
  different ref/snapshot: 762

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

pdfjsbot · 2022-02-24T18:16:24Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/92225866e595c28/output.txt

Total script time: 24.76 mins

Font tests: Passed
Unit tests: FAILED
Integration Tests: FAILED
Regression tests: FAILED

  errors: 7
  different ref/snapshot: 98

Image differences available at: http://54.193.163.58:8877/92225866e595c28/reftest-analyzer.html#web=eq.log

calixteman · 2022-02-25T17:06:26Z

/botio-linux test

pdfjsbot · 2022-02-25T17:06:27Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/76821138ab7fa2c/output.txt

calixteman · 2022-02-25T17:08:39Z

/botio-windows test

pdfjsbot · 2022-02-25T17:08:40Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/616fdbfd2bb9add/output.txt

Snuffleupagus · 2022-02-25T17:13:20Z

Note that a bunch of tests failed with .... | render : TypeError: this.rescaleAndStrokeNoSave is not a function, since that method doesn't exist in the latest version of the patch :-P

pdfjsbot · 2022-02-25T17:29:49Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/76821138ab7fa2c/output.txt

Total script time: 23.34 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  errors: 14
  different ref/snapshot: 761
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/76821138ab7fa2c/reftest-analyzer.html#web=eq.log

pdfjsbot · 2022-02-25T17:33:33Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/616fdbfd2bb9add/output.txt

Total script time: 24.88 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  errors: 7
  different ref/snapshot: 100

Image differences available at: http://54.193.163.58:8877/616fdbfd2bb9add/reftest-analyzer.html#web=eq.log

…743245, bug 1710019) - it aims to fix: - https://bugzilla.mozilla.org/show_bug.cgi?id=1753075; - https://bugzilla.mozilla.org/show_bug.cgi?id=1743245; - https://bugzilla.mozilla.org/show_bug.cgi?id=1710019; - issue mozilla#13211; - issue mozilla#14521. - previously we were trying to adjust lineWidth to have something correct after the current transform is applied but this approach was not correct because finally the pixel is rescaled with the same factors in both directions. And sometimes those factors must be different (see bug 1753075). - So the idea of this patch is to apply a scale matrix to the current transform just before setting lineWidth and stroking. This scale matrix is computed in order to ensure that after transform, a pixel will have its two thickness greater than 1.

calixteman · 2022-02-25T17:38:57Z

@Snuffleupagus merci
/botio test

pdfjsbot · 2022-02-25T17:38:58Z

From: Bot.io (Linux m4)

Received

Command cmd_test from @calixteman received. Current queue size: 1

Live output at: http://54.241.84.105:8877/bd02df6acdb4d89/output.txt

pdfjsbot · 2022-02-25T17:38:58Z

From: Bot.io (Windows)

Received

Command cmd_test from @calixteman received. Current queue size: 1

Live output at: http://54.193.163.58:8877/567ff23c77bbbf0/output.txt

pdfjsbot · 2022-02-25T18:24:27Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/bd02df6acdb4d89/output.txt

Total script time: 24.27 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: FAILED
Regression tests: FAILED

  different ref/snapshot: 181

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

pdfjsbot · 2022-02-25T18:27:35Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/567ff23c77bbbf0/output.txt

Total script time: 24.83 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 101

Image differences available at: http://54.193.163.58:8877/567ff23c77bbbf0/reftest-analyzer.html#web=eq.log

brendandahl · 2022-03-02T17:46:19Z

/botio makeref

pdfjsbot · 2022-03-02T17:46:21Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/733e2bdb15418cd/output.txt

pdfjsbot · 2022-03-02T17:46:21Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/18ab1d9fd068547/output.txt

pdfjsbot · 2022-03-02T18:07:17Z

From: Bot.io (Windows)

Success

Full output at http://54.193.163.58:8877/18ab1d9fd068547/output.txt

Total script time: 20.93 mins

Lint: Passed
Make references: Passed
Check references: Passed

pdfjsbot · 2022-03-02T18:08:02Z

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/733e2bdb15418cd/output.txt

Total script time: 21.67 mins

Lint: Passed
Make references: Passed
Check references: Passed

calixteman force-pushed the thin_line branch from bceee3b to 8fc8f1c Compare February 5, 2022 21:46

marco-c linked an issue Feb 5, 2022 that may be closed by this pull request

v2.7-2.8 incorrectly render dashed line in FPDF/FPDI-created files. #13211

Closed

Snuffleupagus reviewed Feb 6, 2022

View reviewed changes

src/display/canvas.js Show resolved Hide resolved

src/display/canvas.js Outdated Show resolved Hide resolved

Snuffleupagus added the core label Feb 6, 2022

Snuffleupagus requested a review from brendandahl February 6, 2022 10:07

calixteman force-pushed the thin_line branch from 8fc8f1c to d4ead47 Compare February 7, 2022 10:51

calixteman force-pushed the thin_line branch from d4ead47 to b0b805c Compare February 7, 2022 21:21

Snuffleupagus reviewed Feb 7, 2022

View reviewed changes

src/display/canvas.js Show resolved Hide resolved

calixteman force-pushed the thin_line branch 2 times, most recently from e806833 to 13e0422 Compare February 14, 2022 16:52

calixteman force-pushed the thin_line branch from 13e0422 to f91a741 Compare February 14, 2022 17:57

marco-c changed the title ~~Fix some issues with lineWidth < 1 after transform (bug 1753075, bug 1743245)~~ Fix some issues with lineWidth < 1 after transform (bug 1753075, bug 1743245, bug 1710019) Feb 24, 2022

calixteman force-pushed the thin_line branch from 66ddbd5 to 65ffd0a Compare February 24, 2022 17:49

calixteman force-pushed the thin_line branch from 65ffd0a to 46369e4 Compare February 25, 2022 17:37

calixteman requested a review from brendandahl March 2, 2022 16:45

brendandahl merged commit 85ff7b1 into mozilla:master Mar 2, 2022

Snuffleupagus mentioned this pull request May 8, 2022

Time for a new release? #14892

Closed

Fix some issues with lineWidth < 1 after transform (bug 1753075, bug 1743245, bug 1710019) #14536

Fix some issues with lineWidth < 1 after transform (bug 1753075, bug 1743245, bug 1710019) #14536

Conversation

calixteman commented Feb 5, 2022 • edited Loading

marco-c commented Feb 5, 2022

Snuffleupagus left a comment

Choose a reason for hiding this comment

calixteman commented Feb 7, 2022

pdfjsbot commented Feb 7, 2022

From: Bot.io (Windows)

Received

pdfjsbot commented Feb 7, 2022

From: Bot.io (Linux m4)

Received

pdfjsbot commented Feb 7, 2022

From: Bot.io (Linux m4)

Failed

pdfjsbot commented Feb 7, 2022

From: Bot.io (Windows)

Failed

Snuffleupagus commented Feb 7, 2022 • edited Loading

calixteman commented Feb 7, 2022

calixteman commented Feb 7, 2022

pdfjsbot commented Feb 7, 2022

From: Bot.io (Linux m4)

Received

pdfjsbot commented Feb 7, 2022

From: Bot.io (Windows)

Received

pdfjsbot commented Feb 7, 2022

From: Bot.io (Linux m4)

Failed

pdfjsbot commented Feb 7, 2022

From: Bot.io (Windows)

Failed

Snuffleupagus commented Feb 7, 2022

calixteman commented Feb 14, 2022

pdfjsbot commented Feb 14, 2022

From: Bot.io (Linux m4)

Received

pdfjsbot commented Feb 14, 2022

From: Bot.io (Windows)

Received

pdfjsbot commented Feb 14, 2022

From: Bot.io (Windows)

Failed

pdfjsbot commented Feb 14, 2022

From: Bot.io (Linux m4)

Failed

calixteman commented Feb 24, 2022

pdfjsbot commented Feb 24, 2022

From: Bot.io (Linux m4)

Received

pdfjsbot commented Feb 24, 2022

From: Bot.io (Windows)

Received

pdfjsbot commented Feb 24, 2022

From: Bot.io (Linux m4)

Failed

pdfjsbot commented Feb 24, 2022

From: Bot.io (Windows)

Failed

calixteman commented Feb 25, 2022

pdfjsbot commented Feb 25, 2022

From: Bot.io (Linux m4)

Received

calixteman commented Feb 25, 2022

pdfjsbot commented Feb 25, 2022

From: Bot.io (Windows)

Received

Snuffleupagus commented Feb 25, 2022 • edited Loading

pdfjsbot commented Feb 25, 2022

From: Bot.io (Linux m4)

Failed

pdfjsbot commented Feb 25, 2022

From: Bot.io (Windows)

Failed

calixteman commented Feb 25, 2022

pdfjsbot commented Feb 25, 2022

From: Bot.io (Linux m4)

calixteman commented Feb 5, 2022 •

edited

Loading

Snuffleupagus commented Feb 7, 2022 •

edited

Loading

Snuffleupagus commented Feb 25, 2022 •

edited

Loading