izuzak / atom-pdf-view

Support for viewing PDF files in Atom.
https://atom.io/packages/pdf-view
MIT License
106 stars 30 forks source link

PDFs containing Japanese characters #76

Closed yohasebe closed 8 years ago

yohasebe commented 8 years ago

The plugin (0.25.0) as it is does not show characters in Japanese (and probably many other languages) properly. They just appear as blank characters.

Solving this problem is quite easy, though. Adding

PDFJS.cMapUrl = "../cmaps/";
PDFJS.cMapPacked = true;

to ~/.atom/packages/pdf-view/node_modules/pdfjs-dist/build/pdf.js just makes my PDFs look fine. Hope the fix is applied in the next release.

Thanks!

izuzak commented 8 years ago

Can you provide a link to an example PDF where you're seeing this? Which version of Atom are you using and which OS? Did you see the same problem in previous versions of pdf-view?

As you can see, this project uses pdfjs-dist as a dependency, and it seems that your fix is related to that dependency. What do you think about reporting that problem to the dependency?

yohasebe commented 8 years ago

I started using Atom and atom-pdf-view just two days ago and have been seeing this problem since then. Currently I'm on OSX 10.11 Beta with Atom 1.0.15.

Here is the link to a sample PDF that is problematic, just for your reference: https://www.dropbox.com/s/wz07fkgvo2nslsp/atom.pdf?dl=0

Actually, the PDFs that I have been trying on atom-pdf-view are all generated with LaTeX (TeX Live 2015). I just found a moment ago that PDFs generated otherwise are okay even if they contained Japanese. As you say, it looks like a problem of PDF.js rendering files generated with LaTeX.

I'll probably report it rather to the dependency. Thanks for your great work anyway.

izuzak commented 8 years ago

Thanks for following up -- yeah, I see the same problem in pdf-view. Let me know if you end up opening those issues and it turns out that this needs to be fixed here. So far, it sounds like it should be fixed either in pdfjs-dist or the place that's generating PDFs. Happy to reopen this then.

maruta commented 8 years ago

@izuzak Thank you so much for developing this important package. I think the parameters (cMapUrl and cMapPacked) are expected to be given by atom-pdf-view to pdfjs-dist through the global variable PDFJS. https://github.com/mozilla/pdfjs-dist/blob/v1.3.148/build/pdf.js#L7611-7622 So, could you reopen this issue?

And,I found that if I replace the following code in atom-pdf-view/lib/pdf-editor-view.js https://github.com/izuzak/atom-pdf-view/blob/b450c7fe8ea0237d050cc44e26fa273fcfb70bf6/lib/pdf-editor-view.js#L12-14 by the following one

global.PDFJS = {workerSrc: "temp",cMapUrl:"temp",cMapPacked:true};
require('./../node_modules/pdfjs-dist/build/pdf.js');
PDFJS.workerSrc = "file://" + path.resolve(__dirname, "../node_modules/pdfjs-dist/build/pdf.worker.js");
PDFJS.cMapUrl = "file://" + path.resolve(__dirname, "../node_modules/pdfjs-dist/cmaps")+"/";

the problem seems to be solved.

I'm not familiar with coding, so please forgive me if miss the point.

izuzak commented 8 years ago

Thanks for the explanation and for the code snippet to fix this, @maruta -- I just applied that fix and published a new version. Give it a try and let me know if things are okay now.

:bow:

maruta commented 8 years ago

@izuzak I tried new version and found it works well. Thanks a lot!