libvips icon indicating copy to clipboard operation
libvips copied to clipboard

vips_thumbnail: Prefer width over height or vice versa

Open kleisauke opened this issue 8 years ago • 3 comments
trafficstars

I like to see a (optional) prefer option in vips_thumbnail which can be set to a new enum (let's call it VIPS_PREFER_WIDTH or VIPS_PREFER_HEIGHT for example).

Use case scenario: To prevent reduction or enlargement in the horizontal axis, I'm setting the target resize width to VIPS_MAX_COORD (which is currently 10000000), when a user only have specified a fixed height. I'm doing vice versa for the target resize height (so setting it to VIPS_MAX_COORD when a fixed width is specified). This feels somewhat hackish.

Example which currently fails due to reduction in the vertical axis (the dimensions of the input JPEG-file are 1600x622): https://gist.github.com/kleisauke/8e090b4f83679cd536cd3bc7aebe60a2

thumbnail width = 499
expected width = 500
thumbnail height = 194
expected height = 194

This is fixable if we change the script to this: https://gist.github.com/kleisauke/a7df1817fa7ed8f01a627a7eccd342c4

thumbnail width = 500
expected width = 500
thumbnail height = 194
expected height = 194

When a prefer option is in vips_thumbnail we can do: https://gist.github.com/kleisauke/1dc424dfdf1b2b65192be4189a4c6364

kleisauke avatar Aug 10 '17 13:08 kleisauke

It would be nice if omitting one dimension would automatically resize just by the other dimension, rather than the dimension default being a square. That would be ideal because it would be more useful, as it's easy to specify square dimensions explicitly, though I know it would bring backwards incompatibility.

In the image_processing Ruby library I'm using this code to calculate the other dimension:

      def infer_dimensions((width, height), (current_width, current_height))
        raise Error, "either width or height must be specified" unless width || height

        case
        when width.nil?
          ratio  = Rational(height, current_height)
          width  = (current_width * ratio).ceil
        when height.nil?
          ratio  = Rational(width, current_width)
          height = (current_height * ratio).ceil
        end

        [width, height]
      end

janko avatar Mar 16 '18 00:03 janko

The official way to handle this is to default the unspecified dimension to infinity, so I think I'd write it as something like:

      def infer_dimensions((width, height), (current_width, current_height))
        raise Error, "either width or height must be specified" unless width || height

        # or what libvips uses as +inf anyway
        infinity = 10000000

        width = width.nil? ? infinity : width
        height = height.nil? ? infinity : height

        [width, height]
      end

Perhaps the VIPS_MAX_COORD constant should be defined by ruby-vips to stop this magic number spreading.

jcupitt avatar Mar 16 '18 08:03 jcupitt

@jcupitt That's perfectly reasonable, thank you!

janko avatar Mar 16 '18 09:03 janko