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

tiffsave xres/yres arguments don't match docstring #1421

Closed
f--f opened this issue Sep 4, 2019 · 4 comments
Closed

tiffsave xres/yres arguments don't match docstring #1421

f--f opened this issue Sep 4, 2019 · 4 comments

Comments

@f--f
Copy link

f--f commented Sep 4, 2019

I noticed that the docstring for tiffsave describes the optional xres and yres arguments like this:

xres (float): Horizontal resolution in pixels/mm
yres (float): Vertical resolution in pixels/mm

But running

pyvips.Image.grey(100, 100).tiffsave("grey.tif", xres=100, yres=100)

and then checking the result with tiffinfo outputs Resolution: 100, 100 pixels/cm (xres and yres in pixels/cm, and not pixels/mm - this is the case even if resunit is specified as inches).

I suspect the confusion is that libvips stores xres/yres metadata in pixels/mm, which does get converted properly if it is specified in tiffsave.

The relevant section of existing code for handling resolution units seems to be here:

/* Default xres/yres to the values from the image.
*/
if( !vips_object_argument_isset( object, "xres" ) )
tiff->xres = save->ready->Xres * 10.0;
if( !vips_object_argument_isset( object, "yres" ) )
tiff->yres = save->ready->Yres * 10.0;
/* resunit param overrides resunit metadata.
*/
if( !vips_object_argument_isset( object, "resunit" ) &&
vips_image_get_typeof( save->ready,
VIPS_META_RESOLUTION_UNIT ) &&
!vips_image_get_string( save->ready,
VIPS_META_RESOLUTION_UNIT, &p ) &&
vips_isprefix( "in", p ) )
tiff->resunit = VIPS_FOREIGN_TIFF_RESUNIT_INCH;
if( tiff->resunit == VIPS_FOREIGN_TIFF_RESUNIT_INCH ) {
tiff->xres *= 2.54;
tiff->yres *= 2.54;
}

(Using vips 8.8.0)

@jcupitt
Copy link
Member

jcupitt commented Sep 4, 2019

Ooop, you're right, it's all messed up. I'll have a look. Thanks!

jcupitt added a commit that referenced this issue Sep 4, 2019
and should have been in pixels/mm

thanks f--f

see #1421
@jcupitt
Copy link
Member

jcupitt commented Sep 4, 2019

I think it's working now. There was a *10 missing from one side of an if :-( Thanks again for reporting this, it'll be in 8.8.2.

$ vips copy k2.jpg x.tif[strip]
$ vips copy x.tif y.tif[xres=100]
$ tiffinfo y.tif
TIFF Directory at offset 0x87f008 (8908808)
  Image Width: 1450 Image Length: 2048
  Resolution: 1000, 28.3465 pixels/cm
  Bits/Sample: 8
  Sample Format: unsigned integer
  Compression Scheme: None
  Photometric Interpretation: RGB color
  Orientation: row 0 top, col 0 lhs
  Samples/Pixel: 3
  Rows/Strip: 128
  Planar Configuration: single image plane
$ vips copy x.tif y.tif[xres=100,resunit=inch]
$ tiffinfo y.tif
TIFF Directory at offset 0x87f008 (8908808)
  Image Width: 1450 Image Length: 2048
  Resolution: 2540, 72 pixels/inch
  Bits/Sample: 8
  Sample Format: unsigned integer
  Compression Scheme: None
  Photometric Interpretation: RGB color
  Orientation: row 0 top, col 0 lhs
  Samples/Pixel: 3
  Rows/Strip: 128
  Planar Configuration: single image plane

@f--f
Copy link
Author

f--f commented Sep 4, 2019

😁 Looks good, thanks!

@f--f f--f closed this as completed Sep 4, 2019
@jcupitt
Copy link
Member

jcupitt commented Sep 5, 2019

I added some tests too:

f80c7a1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants