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

Type3 smoothing: pre-scale image in the paintImageMaskXObject #1763

Merged
merged 3 commits into from Jun 6, 2012

Conversation

yurydelendik
Copy link
Contributor

Per MDN ( https://developer.mozilla.org/en/Canvas_tutorial/Using_images#drawImage_example_2 ):

"Images can become blurry when scaling up or grainy if they're scaled down too much. Scaling is probably best not done if you've got some text in it which needs to remain legible."

Using the simple algorithm to pre-scale the image.

@yurydelendik
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

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

Live output at: http://107.21.233.14:8877/15cf34772c57a0e/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

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

Total script time: 13.90 mins

  • Lint: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

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

@pdfjsbot
Copy link

From: Bot.io (Linux)


Failed

Full output at http://107.21.233.14:8877/15cf34772c57a0e/output.txt

Total script time: 19.76 mins

  • Lint: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/15cf34772c57a0e/reftest-analyzer.xhtml#web=eq.log

@yurydelendik
Copy link
Contributor Author

Fixes #1601, #1613, #1679, #1760 and probably #1416.

@@ -1094,6 +1094,40 @@ var CanvasGraphics = (function CanvasGraphicsClosure() {
}
}
}
function rescaleImage(pixels, widthScale, heightScale) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a resize in image.js too. Do we want to try and share code between the two?
https://github.com/mozilla/pdf.js/blob/master/src/image.js#L131

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is specialized algorithm is addressed only this specific problem. The algorithm in the image.js does not take in account pixels "in between" steps of copy operation, this does. Which gives you this smoothing effect. Also, I don't this we have to use this (in canvas.js) algorithm in some other place.

@brendandahl
Copy link
Contributor

Ref tests seem mostly good, however issue1597-page1 seems to have some new line artifacts(but not on crhome windows) and fit11-talk seems to be much darker (this one could be fine).

P.S. That's a nice big fix list.

@brendandahl
Copy link
Contributor

Also, is the real issue that we're scaling multiple times whereas we should be scaling once to the actual output size? I thought FF by default did a bicubic rescale which should give much better/faster results than we can get in JS.

@yurydelendik
Copy link
Contributor Author

Also, is the real issue that we're scaling multiple times whereas we should be scaling once to the actual output size? I thought FF by default did a bicubic rescale which should give much better/faster results than we can get in JS.

Bicubic is good for up-scaling. In this case it's down-scaling and pieces/pixels (mostly alpha layer values) are lost during the resizing using standard canvas algorithm. The MDN note above warns about that.

The scaling multiple times is not happening and not a issue here -- the images are scaled and painted once. The current algorithm just makes sure the all pixels are used in scaling and destination image is painted as is without being rescaled (see mozCurrentTransformInverse usage).

Ref tests seem mostly good, however issue1597-page1 seems to have some new line artifacts(but not on crhome windows) and fit11-talk seems to be much darker (this one could be fine).

Contains hi-res scanned image issue1597-page1. I don't think the algorithm is at fault here. Can we take this regression to close some major issues? Actually we already have this artifacts on color images as well.

@yurydelendik
Copy link
Contributor Author

ctx.drawImage(tmpCanvas, 0, -h / heightScale);
} else
tmpCtx.putImageData(imgData, 0, 0);
ctx.drawImage(tmpCanvas, 0, -h);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing brackets?

@yurydelendik
Copy link
Contributor Author

Missing brackets?

Yes. Thank you

/botio test

@pdfjsbot
Copy link

pdfjsbot commented Jun 5, 2012

From: Bot.io (Windows)


Received

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

Live output at: http://107.22.172.223:8877/8b48372744eb010/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Jun 5, 2012

From: Bot.io (Linux)


Received

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

Live output at: http://107.21.233.14:8877/0dd33aac634dec3/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Jun 5, 2012

From: Bot.io (Windows)


Failed

Full output at http://107.22.172.223:8877/8b48372744eb010/output.txt

Total script time: 16.48 mins

  • Lint: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.22.172.223:8877/8b48372744eb010/reftest-analyzer.xhtml#web=eq.log

@pdfjsbot
Copy link

pdfjsbot commented Jun 5, 2012

From: Bot.io (Linux)


Failed

Full output at http://107.21.233.14:8877/0dd33aac634dec3/output.txt

Total script time: 24.98 mins

  • Lint: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/0dd33aac634dec3/reftest-analyzer.xhtml#web=eq.log

@brendandahl
Copy link
Contributor

/botio makeref

@pdfjsbot
Copy link

pdfjsbot commented Jun 6, 2012

From: Bot.io (Windows)


Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented Jun 6, 2012

From: Bot.io (Linux)


Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented Jun 6, 2012

From: Bot.io (Windows)


Success

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

Total script time: 16.33 mins

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

@pdfjsbot
Copy link

pdfjsbot commented Jun 6, 2012

From: Bot.io (Linux)


Success

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

Total script time: 24.80 mins

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

brendandahl added a commit that referenced this pull request Jun 6, 2012
Type3 smoothing: pre-scale image in the paintImageMaskXObject
@brendandahl brendandahl merged commit 1c1447e into mozilla:master Jun 6, 2012
@brendandahl
Copy link
Contributor

Forgot to note, doesn't look like it fixes #1416.

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

Successfully merging this pull request may close these issues.

None yet

3 participants