-
-
Notifications
You must be signed in to change notification settings - Fork 755
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
fix tiff support #414
fix tiff support #414
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.
good well done
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.
Does the test now pass for you? @jeremypatrickdahan
index.js
Outdated
var tiff = (UTIF.decode(data)[0]); | ||
var ifds = UTIF.decode(data) | ||
var page = ifds[0]; | ||
UTIF.decodeImages(data, ifds); |
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 line actually doing anything?
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.
I believe it modifies ifds
in place.
I need to re-check this
I tested it out and it does in fact work. @jeremypatrickdahan can you update the TIFF test so it passes? I will merge once you do. |
Ok, the original tiff image in the samples directory had strange values in the test (pixels coordinates outside of the image). I changed it to one from http://www.petitcolas.net/watermarking/image_database/ and changed the values accordingly, double checking the values manually in Photoshop. The line |
Sweet! can you regenerate the lock file? Don't manually resolve the merge conflicts. You can just delete it and re-install. |
@jeremypatrickdahan Big changes happened in the repo! to resolve your merge conflicts I would:
Thanks for your work! |
I'm a bit confused... I did run |
Tiff support seems broken in the current version (f7b5e5b).
Code in this PR is tested and working.
npm test
shows a lot of lint errors, but the style of the modification is in line with the existing code.(Please be kind, it is my first pull request!)