Added support for explicitly setting the dpi(xres,yres) on TIFF images. #828
Conversation
YvesBos
changed the title
test/unit/io.js
Outdated
| .toFile(fixtures.outputTiff, (err, info) => { | ||
| if (err) throw err; | ||
| assert.strictEqual('tiff', info.format); | ||
| assert.strictEqual(image.options.tiffXres, res); |
There was a problem hiding this comment.
Perhaps the returned info Object should include xres and yres properties? Or maybe the metadata() operation could provide this? I'd prefer to avoid testing the "private" options internals of a sharp instance.
src/pipeline.h
Outdated
| @@ -182,6 +184,8 @@ struct PipelineBaton { | |||
| tiffCompression(VIPS_FOREIGN_TIFF_COMPRESSION_JPEG), | |||
| tiffPredictor(VIPS_FOREIGN_TIFF_PREDICTOR_NONE), | |||
| tiffSquash(false), | |||
| tiffXres(10), | |||
There was a problem hiding this comment.
It looks like libvips uses a default of 1.0 - see https://github.com/jcupitt/libvips/blob/d1ca808769339a8856be69b281b72d674a9c0f75/libvips/foreign/
There was a problem hiding this comment.
I assume you mean 10.0 right? I'll update it.
https://github.com/jcupitt/libvips/blob/d1ca808769339a8856be69b281b72d674a9c0f75/libvips/foreign/tiffsave.c#L137
There was a problem hiding this comment.
Sorry, the URL was truncated, see https://github.com/jcupitt/libvips/blob/d1ca808769339a8856be69b281b72d674a9c0f75/libvips/foreign/tiffsave.c#L305-L306 for the default of 1.
src/pipeline.cc
Outdated
| @@ -809,6 +809,9 @@ class PipelineWorker : public Nan::AsyncWorker { | |||
| ->set("squash", baton->tiffSquash) | |||
| ->set("compression", baton->tiffCompression) | |||
| ->set("predictor", baton->tiffPredictor))); | |||
| ->set("predictor", baton->tiffPredictor) | |||
There was a problem hiding this comment.
I also added xres, and yres when writing to buffer. However, these values do not get saved when calling the ToBuffer method on a tiff image.
e.g. when I write a test like this:
it('TIFF setting xres and yres on buffer', function (done) {
const res = 1000.0; // inputTiff has a dpi of 300 (res*2.54)
const image = sharp(fixtures.inputTiff)
.tiff({
xres: (res),
yres: (res)
})
.toBuffer(function (err, data, info) {
if (err) throw err;
sharp(data).metadata(function (err, metadata) {
if (err) throw err;
assert.strictEqual(metadata.density, res*2.54); // convert to dpi
done();
});
});
});
And then write it to disk afterwards or read the metadata, the density is still the same as the original(300dpi).
@lovell do you have any idea why?
There was a problem hiding this comment.
The position of the brackets are fixed by the way. But the dpi on the buffer is still not working.
|
Thanks for the updates, looking good.
Based on the code in thie PR, it sounds like a possible bug in libvips or libtiff itself. I should have a chance to investigate this further at the weekend if you haven't been able to work it out in the meantime. |
|
I had a quick look at this branch and added your proposed |
|
A reinstall of sharp did the trick, pretty weird as I was also on libvips v8.5.5. |
|
Fantastic, thank you very much for this Yves. |
In reference to #823 .
This PR exposes the xres and yres properties from libvips.
http://jcupitt.github.io/libvips/API/current/VipsForeignSave.html#vips-tiffsave