-
Notifications
You must be signed in to change notification settings - Fork 47
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 threads.js #160
Remove threads.js #160
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I'm glad you figured out the webpack magic for importing workers.
src/loaders/index.js
Outdated
@@ -1,6 +1,7 @@ | |||
import { openArray } from 'zarr'; | |||
// eslint-disable-next-line import/extensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this eslint vestigial?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably.
function decode(self, fileDirectory, buffer) { | ||
const decoder = getTiffDecoder(fileDirectory); | ||
const result = decoder.decode(fileDirectory, buffer); | ||
self.postMessage([result], [result]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is result
an ArrayBuffer
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
GeoTIFF recently ventured into threads.js which made bundling this application very difficult unless we tell upstream users to carry a plugin, which I think is not ideal.
We now re-use the old WebWorker code from GeoTIFF that they had and bundle it directly as an inline web worker. I'll open an issue with them to see if they want to go back to this.
This also inadvertently fixes #97 . See andywer/threads.js#211 for someone experiencing a similar issue and andywer/threads.js#232 (comment) for the GeoTIFF contributor getting feedback.