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

Web worker integration. #692

Merged
merged 129 commits into from
Oct 21, 2011
Merged

Web worker integration. #692

merged 129 commits into from
Oct 21, 2011

Conversation

brendandahl
Copy link
Contributor

This merges all of Julian's web worker code from pull request #635. Some notable things:

  • fixed external graphics state problems
  • removed workerpage and now just use the one page object
  • removed worker.js and moved the remaining code into pdf.js ( I think having a worker.js is confusing since usually worker.js is what start's the worker e.g. new Worker('worker.js'))
  • refactored TilingPattern and TilingPatternIR to be one object and have a 'getIR()' function, which is more simliar to the style for other objects. At some point we'll want to split these up again but that will be later.
  • renamed JpegStreamIR to JpegImage since this isn't really a stream or IR object
  • removed all console.logs

See and use #690 for further discussion on ideas to streamline this code. Use this issue if you see anything you really don't want do go into master.

And of course a huge thanks to Julian for doing this!

…oesnt have a document to attach the canvas to
var w = imgData.width;
var h = imgData.height;
// scale the image to the unit square
ctx.scale(1 / w, -1 / h);
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a few line spaces where it makes sense? They are free! :)

@jviereck
Copy link
Contributor

why does this code has not been converted to:
if (imageObj && imageObj.imageMask) {
...
}?

The thing is, that the new code base make an early decision depending on if the object has an imageMask or not. That's around this lines of code:

https://github.com/andreasgal/pdf.js/pull/692/files#L2R4629

The if statement you mentioned is only true if we execute the new function paintImageMaskXObject. But as we already know, that the image has an imageMask (otherwise paintImageXObject had been executed), there is no need for this check anymore.

return null;
};
constructor.shadingFromIR = function pattern_shadingFromIR(ctx, raw) {
var obj = window[raw[0]];
Copy link
Contributor

Choose a reason for hiding this comment

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

window!!! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Julian, can you please fix this hack? We thought of adding a switch but we don't know the possible class names. This might need to be conceptually revisited, not sure.

@brendandahl
Copy link
Contributor Author

@pdfjsbot test

@pdfjsbot
Copy link

Processing command test by user brendandahl. Queue size: 0

Live script output is available (after queueing is done) at: http://184.73.87.52:8989/2487205.txt

[bot:processed:2487205]

@pdfjsbot
Copy link

ERROR(s) found

Output:

========== Killing any stray processes

========== Cloning pull request repo
Cloning into ....

========== Running 'make lint'
gjslint ./charsets.js ./cidmaps.js ./crypto.js ./fonts.js ./glyphlist.js ./metrics.js ./pdf.js utils/cffStandardStrings.js utils/fonts_utils.js worker/console.js worker/message_handler.js worker/pdf_worker_loader.js worker/processor_handler.js web/compatibility.js web/viewer.js test/driver.js examples/helloworld/hello.js extensions/firefox/bootstrap.js extensions/firefox/components/pdfContentHandler.js
----- FILE  :  /home/ubuntu/pdf.js-bot/tmp/tests/300730c335ea64fd572cfcf85cc839bb29c5f46f/pdf.js -----
Line 7475, E:0001: Extra space at end of line

Found 1 errors, including 0 new errors, in 1 files (18 files OK).
\u0007
Some of the errors reported by GJsLint may be auto-fixable using the script
fixjsstyle. Please double check any changes it makes and report any bugs. The
script can be run by executing:

fixjsstyle ./charsets.js ./cidmaps.js ./crypto.js ./fonts.js ./glyphlist.js ./metrics.js ./pdf.js utils/cffStandardStrings.js utils/fonts_utils.js worker/console.js worker/message_handler.js worker/pdf_worker_loader.js worker/processor_handler.js web/compatibility.js web/viewer.js test/driver.js examples/helloworld/hello.js extensions/firefox/bootstrap.js extensions/firefox/components/pdfContentHandler.js 
make: *** [lint] Error 1

========== Merging upstream into pull request clone

========== Cloning reference images repo into test/ref/
Initialized empty Git repository in /home/ubuntu/pdf.js-bot/tmp/tests/300730c335ea64fd572cfcf85cc839bb29c5f46f/test/ref/.git/

========== Checking for consistency with reference repo

========== Running 'make bot_test'
cd test; \
    python -u test.py \
    --browserManifestFile=resources/browser_manifests/browser_manifest.json \
    --manifestFile=test_manifest.json
Launching firefox
Xlib:  extension "RANDR" missing on display ":1".
TEST-PASS | eq test tracemonkey-eq | in firefox
TEST-PASS | forward-back-forward test tracemonkey-fbf | in firefox
TEST-PASS | load test html5-canvas-cheat-sheet-load | in firefox
TEST-PASS | load test intelisa-load | in firefox
TEST-PASS | load test pdfspec-load | in firefox
TEST-PASS | load test shavian-load | in firefox
TEST-PASS | eq test sizes | in firefox
TEST-PASS | eq test openweb-cover | in firefox
TEST-PASS | eq test plusminus | in firefox
TEST-PASS | load test openoffice-pdf | in firefox
TEST-PASS | load test openofficecidtruetype-pdf | in firefox
TEST-PASS | load test openofficearabiccidtruetype-pdf | in firefox
TEST-PASS | load test arabiccidtruetype-pdf | in firefox
TEST-PASS | load test complexttffont-pdf | in firefox
TEST-PASS | eq test thuluthfont-pdf | in firefox
TEST-PASS | eq test wnv_chinese-pdf | in firefox
TEST-PASS | eq test i9-pdf | in firefox
TEST-PASS | load test hmm-pdf | in firefox
TEST-PASS | eq test rotation | in firefox
TEST-PASS | load test ecma262-pdf | in firefox
TEST-PASS | load test jai-pdf | in firefox
TEST-PASS | eq test cable | in firefox
TEST-PASS | eq test pdkids | in firefox
TEST-PASS | eq test artofwar | in firefox
TEST-PASS | eq test wdsg_fitc | in firefox
TEST-PASS | eq test unix01 | in firefox
TEST-PASS | eq test fit11-talk | in firefox
TEST-PASS | eq test fips197 | in firefox
TEST-PASS | load test txt2pdf | in firefox
TEST-PASS | load test f1040 | in firefox
TEST-PASS | load test hudsonsurvey | in firefox
TEST-PASS | eq test extgstate | in firefox
TEST-PASS | eq test usmanm-bad | in firefox
TEST-PASS | load test vesta-bad | in firefox
TEST-PASS | load test ibwa-bad | in firefox
TEST-PASS | eq test tcpdf_033 | in firefox
TEST-PASS | eq test pal-o47 | in firefox
TEST-PASS | eq test simpletype3font | in firefox

All tests passed.
Process firefox is still running. Killing.
Runtime was 1744 seconds

========== Cleaning up

All done.


_____________________________ stderr:

Bot response time: 29.94 mins

startIdx = gfx.executeIRQueue(IRQueue, startIdx, next);
if (startIdx == length) {
self.stats.render = Date.now();
if (callback) callback();
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a problem but I usually prefer to this kind of expression in 2 lines.

@vingtetun
Copy link
Contributor

I have a few other nits but nothing that can't be fixed later.

The architecture of all this is going to change later but this work will be tracked in #issue 690, let's push the f**** button!

congrats guys!

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.

6 participants