Make combinations of :width/:max-height image specs work reliably

* doc/lispref/display.texi (ImageMagick Images): Document
:width/:max-height combinations (etc) (bug #25583).

* src/image.c (compute_image_size): Handle :width/:max-height
(etc) combinations consistently (by letting "max" win and
preserve ratio).

* test/manual/image-size-tests.el (image-size-tests): Add
tests for :width/:max-height (etc) combinations.

4 files changed, 72 insertions, 54 deletions

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 005d31af05a..98940cbc996 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5305,6 +5305,17 @@ and if @code{:height} is set it will have precedence over
 wish.  @code{:max-width} and @code{:max-height} will always preserve
 the aspect ratio.
 
+If both @code{:width} and @code{:max-height} has been set (but
+@code{:height} has not been set), then @code{:max-height} will have
+precedence.  The same is the case for the opposite combination: The
+``max'' keyword has precedence.  That is, if you have a 200x100 image
+and specify that @code{:width} should be 400 and @code{:max-height}
+should be 150, you'll end up with an image that is 300x150: Preserving
+the aspect ratio and not exceeding the ``max'' setting.  This
+combination of parameters is a useful way of saying ``display this
+image as large as possible, but no larger than the available display
+area''.
+
 @item :scale @var{scale}
 This should be a number, where values higher than 1 means to increase
 the size, and lower means to decrease the size.  For instance, a value
diff --git a/etc/NEWS b/etc/NEWS
index 0ab49587d79..edb71118efd 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -763,6 +763,14 @@ Ido mode is active.
 in question).
 
 +++
+*** It's now possible to specify aspect-ratio preserving combinations
+of :width/:max-height and :height/:max-width keywords.  In either
+case, the "max" keywords win.  (Previously some combinations would,
+depending on the aspect ratio of the image, just be ignored and in
+other instances this would lead to the aspect ratio not being
+preserved.)
+
++++
 *** Images inserted with 'insert-image' and related functions get a
 keymap put into the text properties (or overlays) that span the
 image.  This keymap binds keystrokes for manipulating size and
diff --git a/src/image.c b/src/image.c
index 1426e309445..69a529e8c35 100644
--- a/src/image.c
+++ b/src/image.c
@@ -8086,83 +8086,76 @@ compute_image_size (size_t width, size_t height,
                     int *d_width, int *d_height)
 {
   Lisp_Object value;
-  int desired_width, desired_height;
+  int desired_width = -1, desired_height = -1, max_width = -1, max_height = -1;
   double scale = 1;
 
   value = image_spec_value (spec, QCscale, NULL);
   if (NUMBERP (value))
     scale = XFLOATINT (value);
 
+  value = image_spec_value (spec, QCmax_width, NULL);
+  if (NATNUMP (value))
+    max_width = min (XFASTINT (value), INT_MAX);
+
+  value = image_spec_value (spec, QCmax_height, NULL);
+  if (NATNUMP (value))
+    max_height = min (XFASTINT (value), INT_MAX);
+
   /* If width and/or height is set in the display spec assume we want
      to scale to those values.  If either h or w is unspecified, the
      unspecified should be calculated from the specified to preserve
      aspect ratio.  */
   value = image_spec_value (spec, QCwidth, NULL);
-  desired_width = NATNUMP (value) ?
-    min (XFASTINT (value) * scale, INT_MAX) : -1;
-  value = image_spec_value (spec, QCheight, NULL);
-  desired_height = NATNUMP (value) ?
-    min (XFASTINT (value) * scale, INT_MAX) : -1;
-
-  width = width * scale;
-  height = height * scale;
-
-  if (desired_width == -1)
+  if (NATNUMP (value))
     {
-      value = image_spec_value (spec, QCmax_width, NULL);
-      if (NATNUMP (value))
-        {
-          int max_width = min (XFASTINT (value), INT_MAX);
-          if (max_width < width)
-            {
-              /* The image is wider than :max-width. */
-              desired_width = max_width;
-              if (desired_height == -1)
-                {
-                  desired_height = scale_image_size (desired_width,
-                                                     width, height);
-                  value = image_spec_value (spec, QCmax_height, NULL);
-                  if (NATNUMP (value))
-                    {
-                      int max_height = min (XFASTINT (value), INT_MAX);
-                      if (max_height < desired_height)
-                        {
-                          desired_height = max_height;
-                          desired_width = scale_image_size (desired_height,
-                                                            height, width);
-                        }
-                    }
-                }
-            }
-        }
+      desired_width = min (XFASTINT (value) * scale, INT_MAX);
+      /* :width overrides :max-width. */
+      max_width = -1;
     }
 
-  if (desired_height == -1)
+  value = image_spec_value (spec, QCheight, NULL);
+  if (NATNUMP (value))
     {
-      value = image_spec_value (spec, QCmax_height, NULL);
-      if (NATNUMP (value))
-        {
-          int max_height = min (XFASTINT (value), INT_MAX);
-          if (max_height < height)
-            desired_height = max_height;
-        }
+      desired_height = min (XFASTINT (value) * scale, INT_MAX);
+      /* :height overrides :max-height. */
+      max_height = -1;
     }
 
+  /* If we have both width/height set explicitly, we skip past all the
+     aspect ratio-preserving computations below. */
+  if (desired_width != -1 && desired_height != -1)
+    goto out;
+
+  width = width * scale;
+  height = height * scale;
+
   if (desired_width != -1 && desired_height == -1)
-    /* w known, calculate h.  */
+    /* Width known, calculate height. */
     desired_height = scale_image_size (desired_width, width, height);
-
-  if (desired_width == -1 && desired_height != -1)
-    /* h known, calculate w.  */
+  else if (desired_width == -1 && desired_height != -1)
+    /* Height known, calculate width. */
     desired_width = scale_image_size (desired_height, height, width);
-
-  /* We have no width/height settings, so just apply the scale. */
-  if (desired_width == -1 && desired_height == -1)
+  else
     {
       desired_width = width;
       desired_height = height;
     }
 
+  if (max_width != -1 && desired_width > max_width)
+    {
+      /* The image is wider than :max-width. */
+      desired_width = max_width;
+      desired_height = scale_image_size (desired_width, width, height);
+    }
+
+  if (max_height != -1 && desired_height > max_height)
+    {
+      /* The image is higher than :max-height. */
+      desired_height = max_height;
+      desired_width = scale_image_size (desired_height, height, width);
+    }
+
+ out:
   *d_width = desired_width;
   *d_height = desired_height;
 }
diff --git a/test/manual/image-size-tests.el b/test/manual/image-size-tests.el
index 577c7658791..6721e348e12 100644
--- a/test/manual/image-size-tests.el
+++ b/test/manual/image-size-tests.el
@@ -25,8 +25,8 @@
 (defmacro im-should (image width height &rest props)
   `(let ((im (im-image ,image ,@props)))
      (unless (im-compare im ,width ,height)
-       (error "%s didn't succeed; size is %s"
-              ',props (image-size im t)))))
+       (error "%s %s didn't succeed; size is %s"
+              ',image ',props (image-size im t)))))
 
 (defun im-image (type &rest props)
   (let ((image-scaling-factor 1))
@@ -67,6 +67,9 @@
   ;; Both max-width/height.
   (im-should :w 100 50 :max-width 100 :max-height 75)
   (im-should :w 50 25 :max-width 100 :max-height 25)
+  ;; :width and :max-height (max-height wins).
+  (im-should :w 400 200 :width 400 :max-height 200)
+  (im-should :w 400 200 :width 500 :max-height 200)
 
   ;; Test the image that's taller than it is wide.
   (im-should :h 100 200)
@@ -87,6 +90,9 @@
   ;; Both max-width/height.
   (im-should :h 50 100 :max-width 75 :max-height 100)
   (im-should :h 25 50 :max-width 25 :max-height 100)
+  ;; :hieght and :max-width (max-width wins).
+  (im-should :h 200 400 :height 400 :max-width 200)
+  (im-should :h 200 400 :height 500 :max-width 200)
   )
 
 ;;; image-size-tests.el ends here