Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove mozCurrentTransform/mozCurrentTransformInverse usage #15281

Merged
merged 1 commit into from
Aug 6, 2022

Conversation

Snuffleupagus
Copy link
Collaborator

These canvas-context properties are Mozilla-specific, and has obviously never been implemented anywhere else. Currently they are in the process of being removed, see bug 1782651 and bug 1294360, which in practice means that in e.g. Firefox Nightly the addContextCurrentTransform-function is now being used in the built-in PDF Viewer (which was obviously never intended).

We should thus be able to replace these Mozilla-specific properties with CanvasRenderingContext2D.getTransform(), which is available in all browsers that we currently support: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/getTransform#browser_compatibility

@Snuffleupagus
Copy link
Collaborator Author

Please note: Based on local testing I'm expecting smaller movement in some test-cases, since it appears that ctx.mozCurrentTransform and ctx.getTransform() can differ slightly in the decimal-part of the returned numbers.

/botio test

@pdfjsbot
Copy link

pdfjsbot commented Aug 5, 2022

From: Bot.io (Windows)


Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented Aug 5, 2022

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/3ff6d6eee85cb14/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Aug 5, 2022

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/3ff6d6eee85cb14/output.txt

Total script time: 25.82 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 1044
  different first/second rendering: 2

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

@pdfjsbot
Copy link

pdfjsbot commented Aug 5, 2022

From: Bot.io (Windows)


Failed

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

Total script time: 29.89 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 528

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

@calixteman
Copy link
Contributor

I checked few failing ref tests and I didn't see anything really wrong: most of time some pixels are slightly different and it could be caused by some rounding errors.
@Snuffleupagus, do you have any idea why those differences ? Did you observe something really wrong ?

@calixteman
Copy link
Contributor

Oh I missed your second message.

@Snuffleupagus
Copy link
Collaborator Author

Interestingly enough, when testing locally on Windows I got a lot fewer "failures" (~100) and in addition the movement in each case was generally smaller compared to the bots. Possibly the particular graphics back-end, as used by the browser, affects things here.

@timvandermeij
Copy link
Contributor

Does this also, at least partially, fix #7799?

@Snuffleupagus
Copy link
Collaborator Author

Does this also, at least partially, fix #7799?

Partially yes, but the issue also talks about tracking the transform internally (to avoid querying the canvas) and that's not attempted here at all.
In this PR I purposely only try and move us away from using a, soon completely, removed Mozilla-specific API.

These canvas-context properties are Mozilla-specific, and has obviously never been implemented anywhere else. Currently they are in the process of being removed, see [bug 1782651](https://bugzilla.mozilla.org/show_bug.cgi?id=1782651) and [bug 1294360](https://bugzilla.mozilla.org/show_bug.cgi?id=1294360), which in practice means that in e.g. Firefox Nightly the `addContextCurrentTransform`-function is now being used in the *built-in* PDF Viewer (which was obviously never intended).

We should thus be able to replace these Mozilla-specific properties with `CanvasRenderingContext2D.getTransform()`, which is available in all browsers that we currently support: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/getTransform#browser_compatibility
@timvandermeij timvandermeij merged commit d559037 into mozilla:master Aug 6, 2022
@timvandermeij
Copy link
Contributor

Looks good, and gets rid of quite a lot of code. Let's do this!

/botio makeref

@pdfjsbot
Copy link

pdfjsbot commented Aug 6, 2022

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/41369f649c146f3/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Aug 6, 2022

From: Bot.io (Windows)


Received

Command cmd_makeref from @timvandermeij received. Current queue size: 1

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

@pdfjsbot
Copy link

pdfjsbot commented Aug 6, 2022

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/41369f649c146f3/output.txt

Total script time: 22.21 mins

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

@pdfjsbot
Copy link

pdfjsbot commented Aug 6, 2022

From: Bot.io (Windows)


Success

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

Total script time: 23.00 mins

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mozCurrentTransformInverse is deprecated Canvas transform reset when drawing content in Chrome
4 participants