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

[api-minor] Enables subpixel anti-aliasing for most of the content. #6551

Merged
merged 1 commit into from
Nov 18, 2015

Conversation

yurydelendik
Copy link
Contributor

The existing mozOpaque logic does not work. Allowing subpixel aa by default, for special blend modes temporary canvas will be created (thus delaying output). The latter requires more memory, it is a trade-off in favor of having a better text output for majority content.

Initially canvas will be created hidden (to hide black flickering), however after first draw callback it will be shown. (Alternative was fill it white before render command, however it will break existing custom solutions)

I noticed that CachedCanvases are broken (global cache can be cleared in the middle of other page rendering) -- added local caches instead.

@yurydelendik
Copy link
Contributor Author

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

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

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

@yurydelendik yurydelendik changed the title [WIP] Enables subpixel anti-aliasing for most of the content. Enables subpixel anti-aliasing for most of the content. Oct 22, 2015
this.ctx.clearRect(0, 0, width, height);
} else {
this.ctx.mozOpaque = true;
var trasparentCanvas = CachedCanvases.getCanvas(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: spelling trasparentCanvas -> transparentCanvas.

@yurydelendik
Copy link
Contributor Author

/botio test

@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/d2f49970dbf4ee6/output.txt

@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/5b8bf0a7d2d1215/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

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

Total script time: 18.91 mins

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

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

@pdfjsbot
Copy link

From: Bot.io (Linux)


Failed

Full output at http://107.21.233.14:8877/5b8bf0a7d2d1215/output.txt

Total script time: 20.14 mins

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

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

@yurydelendik yurydelendik force-pushed the subaa branch 2 times, most recently from a9bcf9c to 402a732 Compare October 22, 2015 13:51
@yurydelendik
Copy link
Contributor Author

Addresses #5196 and #4252.

See also https://bugzilla.mozilla.org/show_bug.cgi?id=1216906

(Past work at #4556)

@yurydelendik yurydelendik changed the title Enables subpixel anti-aliasing for most of the content. [api-minor] Enables subpixel anti-aliasing for most of the content. Oct 23, 2015
@yurydelendik
Copy link
Contributor Author

Minor version update due to transform property for RenderParameters

@timvandermeij
Copy link
Contributor

Will this patch also fix #5197?

@yurydelendik
Copy link
Contributor Author

Will that also fix #5197?

Not sure it will, {alpha: false} is recient api and mozOpaque is Firefox specific. So IE9 might be out of luck -- I could not find anything similar on MS side.

@Snuffleupagus
Copy link
Collaborator

Is it correct to assume that this patch also fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1161147?

@yurydelendik
Copy link
Contributor Author

Correct.

@brendandahl
Copy link
Contributor

If I'm following the logic in https://mxr.mozilla.org/mozilla-central/source/dom/html/HTMLCanvasElement.cpp correctly {alpha: false} and mozOpaque should achieve the same thing. Can we just remove all mozOpaques?

@yurydelendik
Copy link
Contributor Author

{alpha: false} and mozOpaque should achieve the same thing

Correct, but it does not work on Nightly for Mac OSX. If I remove mozOpaque, the subpixel-aa is broken again (and still continue to work on Chrome)

@yurydelendik
Copy link
Contributor Author

/botio-windows preview

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://107.22.172.223:8877/91506f4d9673871/output.txt

@timvandermeij
Copy link
Contributor

Left is the current master, right is this patch (click to enlarge the uncompressed image). The differences are more visible if you open two tabs, one with https://mozilla.github.io/pdf.js/web/viewer.html and one with http://107.22.172.223:8877/91506f4d9673871/web/viewer.html, put both on the same position in the document and then switch tabs back and forth. In the comparison below, take a close look at the letters p (in "alternative paths" on the left side, fourth sentence from the bottom) and h to notice the difference. The italic words are also less blurry with this patch applied (example: "General terms" below is more crisp, i.e. it appears to have less "drop shadow").

comparison

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

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

Total script time: 19.00 mins

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

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

@pdfjsbot
Copy link

From: Bot.io (Linux)


Failed

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

Total script time: 19.75 mins

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

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

@brendandahl
Copy link
Contributor

geothermal and issue1655 are now black. Though, these seem to have regressed at some point.

@yurydelendik
Copy link
Contributor Author

geothermal and issue1655 are now black. Though, these seem to have regressed at some point.

That's normal, not a regression -- those are skipped pages. I made them white again.

/botio test

@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/680655fcb371a39/output.txt

@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/6f43ad757305c2b/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://107.22.172.223:8877/680655fcb371a39/output.txt

Total script time: 18.96 mins

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

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

@pdfjsbot
Copy link

From: Bot.io (Linux)


Failed

Full output at http://107.21.233.14:8877/6f43ad757305c2b/output.txt

Total script time: 19.58 mins

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

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

@brendandahl
Copy link
Contributor

/botio makeref

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://107.22.172.223:8877/0839da959e79d99/output.txt

@brendandahl
Copy link
Contributor

r+ merge away when make ref is done

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/0839da959e79d99/output.txt

Total script time: 18.90 mins

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

@pdfjsbot
Copy link

From: Bot.io (Linux)


Success

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

Total script time: 20.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.

5 participants