bug#25583: 26.0.50; :width/:max-width and vice versa in images

bug#25583: 26.0.50; :width/:max-width and vice versa in images

[Lars Ingebrigtsen]

As far as I can tell, it isn't documented what should happen if you have
both :width and :max-height set in image specification, or vice versa.

Currently :width wins in this situation, but I think that's probably
just a coincidence.  (I mean, I implemented this, and I can't remember
considering that case...)

Here's the use case: I want to display images that are mostly square,
but can sometimes be rectangular, and I want them to be displayed in max
width if possible, even if they are smaller than that width originally,
but not exceeding a certain height.

So I thought ":width 400 :max-height 500" should do the trick, but
apparently compute_image_size just ignores :max-height in this case.

I think :max-height should "win" here... (That is, the width will end up
smaller than 400 if making it 400 wide will make height exceed 500.)

I'll implement this sometimes soon unless somebody objects or I think of
a reason why not...


In GNU Emacs 26.0.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.14.5)
 of 2017-01-30 built on stories
Repository revision: ab96c8509736a7ed622916ad2749ff356e520d02
Windowing system distributor 'The X.Org Foundation', version 11.0.11604000
System Description:	Debian GNU/Linux 8.6 (jessie)

Recent messages:
Mark saved where search started
Grep finished (matches found)
Mark saved where search started
Annotating...
Redisplaying annotation...done (Spanned from 4337.9 to 29.9 days old)
Annotating... done
Mark set
Annotating...
Redisplaying annotation...done (Spanned from 4337.9 to 29.9 days old)
Annotating... done

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LC_ALL: C
  value of $LANG: en_US.UTF-8
  locale-coding-system: nil

Major mode: Texinfo

Minor modes in effect:
  shell-dirtrack-mode: t
  diff-auto-refine-mode: t
  global-whitespace-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  auto-fill-function: do-auto-fill

Load-path shadows:
/home/larsi/src/cddb.el/expect hides /home/larsi/lisp/expect
/home/larsi/src/cddb.el/captitle hides /home/larsi/lisp/captitle
/home/larsi/src/clock.el/clock hides /home/larsi/lisp/clock
~/pgnus/contrib/vcard hides /home/larsi/lisp/vcard
/home/larsi/src/pvr.el/pvr hides /home/larsi/lisp/pvr
~/lisp/zenirc-2.112/src/zenirc-example hides /home/larsi/lisp/zenirc-example
~/pgnus/contrib/compface hides /home/larsi/src/emacs/trunk/lisp/image/compface

Features:
(shadow ecomplete emacsbug sendmail log-view pcvs-util vc-annotate vc
vc-dispatcher texinfo shell pcomplete thingatpt grep compile comint ring
misearch multi-isearch cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs vc-git diff-mode map pp
flow-fill eww copyright vc-cvs gnus-html help-fns radix-tree url-queue
url-cache sort gnus-cite smiley ansi-color shr-color color mm-archive
gnus-async gnus-dup qp gnus-ml gmane spam-gmane dns mm-url disp-table
gnus-fun gnus-mdrtn gnus-topic pop3 nndoc nnmbox utf-7 nnml nnfolder
network-stream starttls nnir spam-report spam spam-stat gnus-uu yenc
gnus-delay gnus-draft gnus-agent gnus-srvr gnus-score score-mode
nnvirtual nntp gnus-cache gnus-msg gnus-art mm-uu mml2015 mm-view
mml-smime smime dig gnus-sum nndraft nnmh gnus-group gnus-undo
gnus-start gnus-cloud nnimap utf7 netrc nnoo parse-time gnus-spec
gnus-win nnmail gnus-int gnus-range mail-source message format-spec
rfc822 mml mml-sec epa epg mailabbrev gmm-utils mailheader gnus nnheader
gnus-util rmail rmail-loaddefs mail-utils whitespace movie mkv shr svg
imdb dom pvr debug debbugs-gnu easy-mmode derived debbugs soap-client
mm-decode mm-bodies mm-encode url-http tls gnutls url-auth mail-parse
rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr url-gw nsm puny
url url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf url-util mailcap warnings rng-xsd rng-dt rng-util xsd-regexp
xml ido flyspell ispell benchmark w3m browse-url doc-view subr-x dired
dired-loaddefs image-mode timezone w3m-hist w3m-fb w3m-ems wid-edit
w3m-ccl ccl w3m-favicon w3m-image w3m-proc w3m-util add-log mail-extr
jka-compr cl finder-inf package epg-config url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache url-vars seq byte-opt gv bytecomp byte-compile cl-extra
help-mode easymenu cconv cl-loaddefs pcase cl-lib time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript case-table epa-hook jka-cmpr-hook help
simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs button
faces cus-face macroexp files text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind inotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 822387 78630)
 (symbols 48 173923 1)
 (miscs 40 177 574)
 (strings 32 241877 19050)
 (string-bytes 1 13817599)
 (vectors 16 38683)
 (vector-slots 8 954704 24999)
 (floats 8 6994 296)
 (intervals 56 22928 15416)
 (buffers 976 50)
 (heap 1024 113146 198091))

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#25583: 26.0.50; :width/:max-width and vice versa in images

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Mon, 30 Jan 2017 22:42:44 +0100
> 
> As far as I can tell, it isn't documented what should happen if you have
> both :width and :max-height set in image specification, or vice versa.

I see this in the ELisp manual:

  ‘:width WIDTH, :height HEIGHT’
       The ‘:width’ and ‘:height’ keywords are used for scaling the image.
       If only one of them is specified, the other one will be calculated
       so as to preserve the aspect ratio.  If both are specified, aspect
       ratio may not be preserved.

  ‘:max-width MAX-WIDTH, :max-height MAX-HEIGHT’
       The ‘:max-width’ and ‘:max-height’ keywords are used for scaling if
       the size of the image of the image exceeds these values.  If
       ‘:width’ is set it will have precedence over ‘max-width’, and if
       ‘:height’ is set it will have precedence over ‘max-height’, but you
       can otherwise mix these keywords as you wish.  ‘:max-width’ and
       ‘:max-height’ will always preserve the aspect ratio.

So I think the behavior that should be expected is well documented.

> Here's the use case: I want to display images that are mostly square,
> but can sometimes be rectangular, and I want them to be displayed in max
> width if possible, even if they are smaller than that width originally,
> but not exceeding a certain height.
> 
> So I thought ":width 400 :max-height 500" should do the trick, but
> apparently compute_image_size just ignores :max-height in this case.
> 
> I think :max-height should "win" here... (That is, the width will end up
> smaller than 400 if making it 400 wide will make height exceed 500.)

Sorry, I don't understand how :max-height could (or should) affect the
width.  And where do you see in the code that :max-height is ignored
if :width is given?  My reading of the code is that that :max-height
is ignored only if :height is given.

bug#25583: 26.0.50; :width/:max-width and vice versa in images

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Date: Mon, 30 Jan 2017 22:42:44 +0100
>> 
>> As far as I can tell, it isn't documented what should happen if you have
>> both :width and :max-height set in image specification, or vice versa.
>
> I see this in the ELisp manual:
>
>   ‘:width WIDTH, :height HEIGHT’
>        The ‘:width’ and ‘:height’ keywords are used for scaling the image.
>        If only one of them is specified, the other one will be calculated
>        so as to preserve the aspect ratio.  If both are specified, aspect
>        ratio may not be preserved.
>
>   ‘:max-width MAX-WIDTH, :max-height MAX-HEIGHT’
>        The ‘:max-width’ and ‘:max-height’ keywords are used for scaling if
>        the size of the image of the image exceeds these values.  If
>        ‘:width’ is set it will have precedence over ‘max-width’, and if
>        ‘:height’ is set it will have precedence over ‘max-height’, but you
>        can otherwise mix these keywords as you wish.  ‘:max-width’ and
>        ‘:max-height’ will always preserve the aspect ratio.
>
> So I think the behavior that should be expected is well documented.

No, the case where :width and :max-height are both specified is not
documented.  Only :width and :max-width (and :height and :max-height).

>> Here's the use case: I want to display images that are mostly square,
>> but can sometimes be rectangular, and I want them to be displayed in max
>> width if possible, even if they are smaller than that width originally,
>> but not exceeding a certain height.
>> 
>> So I thought ":width 400 :max-height 500" should do the trick, but
>> apparently compute_image_size just ignores :max-height in this case.
>> 
>> I think :max-height should "win" here... (That is, the width will end up
>> smaller than 400 if making it 400 wide will make height exceed 500.)
>
> Sorry, I don't understand how :max-height could (or should) affect the
> width.

The aspect ratio is preserved in all these transforms, so changing (or
restricting) the width changes the height and vice versa.

> And where do you see in the code that :max-height is ignored
> if :width is given?  My reading of the code is that that :max-height
> is ignored only if :height is given.

You end up here:

  if (desired_height == -1)
    {
      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;
	}
    }

height is not larger than max_height here, so desired_height is not set.

  if (desired_width != -1 && desired_height == -1)
    /* w known, calculate h.  */
    desired_height = scale_image_size (desired_width, width, height);

And then this is done, and :max-height is ignored.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#25583: 26.0.50; :width/:max-width and vice versa in images

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> And then this is done, and :max-height is ignored.

I've now implemented this, and in doing so, simplified
compute_image_size quite a bit.  :-/  I'm glad I had written all those
image sizing tests in advance, otherwise I would be tempted to say there
must be something wrong somewhere, because the new code is less...
er...  convoluted.  I think.

But it even seems to work in practice, which is something of a miracle.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Semi-not-very interactive tests

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I'm glad I had written all those image sizing tests in advance

Currently, there is no way to have automatic tests that require a
(graphical) frame open.  I understand why you wouldn't want that in a
totally automatic test harness, but it would be really, really nice to
just be able to say

$ make not-very-manual-tests

and have Emacs pop up a frame, run the tests, and close the frame
again.  Running tests (like the image size tests) completely manually is
tedious.  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no