bug#62637: 29.0.60; Issues when displaying images

bug#62637: 29.0.60; Issues when displaying images

[Abdul-Lateef Haji-Ali via Bug reports for GNU Emacs, the Swiss army knife of text editors]

It seems that `mm-inline-image` switched to using `insert-image` instead of `put-image` which resulted in some regressions in notmuch that I noticed between emacs 29 and emacs 28.2. However, I tracked down the discrepancy to the following issues in displaying images in emacs-29. 

First the following code (assuming some image in /tmp/tmp.png), executed in "emacs -Q" on either emacs 28.2 or emacs 29:

(let (content-begin
      content-end)
  (goto-char (point-max))
  (insert "\n")
  (setq content-begin (point))
  (insert-image (create-image "/tmp/tmp.png"))
  (setq content-end (point))

  ;; I expect the following line to indent the image (or not).
  ;; instead the image is removed completely
  ;;(indent-rigidly content-begin content-end 1)

  ;; This line should hide the image, but it doesn't
  ;;(overlay-put (make-overlay content-begin content-end) 'invisible t)
  )

Uncommenting each of the two lines of code shows the two issues (explained in comments). These issues appear in both emacs 29 and 28.2 (though they might be specific to MacOS).

Similar code to this example is executed in notmuch when displaying message, but using `mm-inline-image` instead of insert-image:

(let* ((buffer (find-file-noselect "/tmp/tmp.png" t t))
       (handle (list
                buffer '("image/jpeg" (charset))
                nil nil nil nil
                `(image :type jpeg
                        :data ,(with-current-buffer buffer
                                 (buffer-string))
                        :scale 1
                        :transform-smoothing t)
                nil))
       content-begin
       content-end)
  (goto-char (point-max))
  (insert "\n")
  (setq content-begin (point))
  (require 'mm-view)
  (mm-inline-image handle)
  (setq content-end (point))

  ;; I expect the following line to indent the image (or not).
  ;; instead the image is removed completely
  ;;(indent-rigidly content-begin content-end 1)

  ;; This line should hide the image, but it doesn't
  ;;(overlay-put (make-overlay content-begin content-end) 'invisible t)
  )

In this case, in Emacs 29, the behaviour is similar to the "wrong" behaviour of the first example. In Emacs 28.2 on the other hand, the behaviour is what I would expect in both cases (though the undo stack gets messed up and I cannot undo inserting the image. Indenting also doesn't work, but at least the image is not removed)

As a last test, in case it is helpful, I tried the following in emacs 29 using `put-image` instead of `insert-image` (`mm-inline-image` uses in Emacs 28.2 uses `put-image` while in Emacs 29 it uses 'insert-image'), and noticed an equally puzzling behaviour:

(let (content-begin
      content-end)
  (goto-char (point-max))
  (insert "\n")

  (setq content-begin (point))
  (put-image (create-image "/tmp/tmp.png") (point-marker))  
  (setq content-end (point))

  ;; I expect the following line to indent the image (or not).
  ;; instead the image is removed completely
  ;;(indent-rigidly content-begin content-end 1)

  ;; This line should hide the image, but it doesn't
  ;;(overlay-put (make-overlay content-begin content-end) 'invisible t)
)

In this case, `indent-rigidly` does not remove the image, but hiding the overlay does not work.

Best regards,
-- Al

----------------------------------------------------------------------------
In GNU Emacs 29.0.60 (build 1, aarch64-apple-darwin22.3.0, NS
 appkit-2299.40 Version 13.2.1 (Build 22D68)) of 2023-03-22 built on
 HW-R9XXWKPJ4D
Windowing system distributor 'Apple', version 10.3.2299
System Description:  macOS 13.2.1

Configured using:
 'configure --disable-dependency-tracking --disable-silent-rules
 --enable-locallisppath=/opt/homebrew/share/emacs/site-lisp
 --infodir=/opt/homebrew/Cellar/emacs-plus@29/29.0.60/share/info/emacs
 --prefix=/opt/homebrew/Cellar/emacs-plus@29/29.0.60 --with-xml2
 --with-gnutls --with-native-compilation --without-compress-install
 --without-dbus --with-imagemagick --with-modules --with-rsvg --with-ns
 --disable-ns-self-contained 'CFLAGS=-Os -w -pipe
 -mmacosx-version-min=13
 -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk
 -DFD_SETSIZE=10000 -DDARWIN_UNLIMITED_SELECT'
 'CPPFLAGS=-I/opt/homebrew/opt/zlib/include
 -I/opt/homebrew/opt/jpeg/include -I/opt/homebrew/opt/libomp/include
 -I/opt/homebrew/opt/icu4c/include
 -I/opt/homebrew/opt/openssl@1.1/include -isystem/opt/homebrew/include
 -F/opt/homebrew/Frameworks
 -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk'
 'LDFLAGS=-L/opt/homebrew/opt/zlib/lib -L/opt/homebrew/opt/jpeg/lib
 -L/opt/homebrew/opt/libomp/lib -L/opt/homebrew/opt/icu4c/lib
 -L/opt/homebrew/opt/openssl@1.1/lib -L/opt/homebrew/lib
 -F/opt/homebrew/Frameworks -Wl,-headerpad_max_install_names
 -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk''

Configured features:
ACL GIF GLIB GMP GNUTLS IMAGEMAGICK JPEG JSON LCMS2 LIBXML2 MODULES
NATIVE_COMP NOTIFY KQUEUE NS PDUMPER PNG RSVG SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XIM ZLIB

Important settings:
  value of $LANG: en_GB.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
comp comp-cstr warnings icons subr-x rx cl-seq cl-macs gv cl-extra
help-mode bytecomp byte-compile cl-lib sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils rmc iso-transl tooltip cconv
eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/ns-win ns-win ucs-normalize mule-util
term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
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 emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads kqueue cocoa ns lcms2 multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 77309 5714)

________________________________
Heriot-Watt University was founded in 1821 and is a registered Scottish charity (SC000278).
 (symbols 48 7039 0)
 (strings 32 18694 2727)
 (string-bytes 1 577061)
 (vectors 16 16292)
 (vector-slots 8 324373 10025)
 (floats 8 27 46)
 (intervals 56 297 0)
 (buffers 984 10))

bug#62637: 29.0.60; Issues when displaying images

[Eli Zaretskii]

> Date: Sun, 02 Apr 2023 14:31:12 +0100
> From:  Abdul-Lateef Haji-Ali via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> 
> It seems that `mm-inline-image` switched to using `insert-image` instead of `put-image` which resulted in some regressions in notmuch that I noticed between emacs 29 and emacs 28.2. However, I tracked down the discrepancy to the following issues in displaying images in emacs-29. 

Maybe I'm missing something (you've lumped together several code
fragments without too many explanations), but I see no discrepancies
here, only unjustified expectations.

> First the following code (assuming some image in /tmp/tmp.png), executed in "emacs -Q" on either emacs 28.2 or emacs 29:
> 
> (let (content-begin
>       content-end)
>   (goto-char (point-max))
>   (insert "\n")
>   (setq content-begin (point))
>   (insert-image (create-image "/tmp/tmp.png"))
>   (setq content-end (point))
> 
>   ;; I expect the following line to indent the image (or not).
>   ;; instead the image is removed completely
>   ;;(indent-rigidly content-begin content-end 1)

indent-rigidly removes the existing whitespace and then inserts new
whitespace as needed.  And insert-image by default puts the 'display'
property on a space character.  Does this explain what you see?

>   ;; This line should hide the image, but it doesn't
>   ;;(overlay-put (make-overlay content-begin content-end) 'invisible t)
>   )

Due to the way the Emacs display engine is implemented, 'display'
properties (which is how images are implemented in Emacs) are
processed before invisible properties.  And a 'display' property that
specifies an image is a "replacing" property: the image is displayed
_instead_ of the buffer text which has this property, and the buffer
text itself is skipped.  What this means in this case is that the
overlay with the invisible property is completely ignored by the
display code, because the buffer text on which you put that overlay is
skipped when the 'display' property is processed.

> Similar code to this example is executed in notmuch when displaying message, but using `mm-inline-image` instead of insert-image:

I don't understand all the fine details of what mm-inline-image does,
but its current implementation again uses a space for the text on
which it puts the 'display' property that is displayed as the image.

> As a last test, in case it is helpful, I tried the following in emacs 29 using `put-image` instead of `insert-image` (`mm-inline-image` uses in Emacs 28.2 uses `put-image` while in Emacs 29 it uses 'insert-image'), and noticed an equally puzzling behaviour:
> 
> (let (content-begin
>       content-end)
>   (goto-char (point-max))
>   (insert "\n")
> 
>   (setq content-begin (point))
>   (put-image (create-image "/tmp/tmp.png") (point-marker))  
>   (setq content-end (point))
> 
>   ;; I expect the following line to indent the image (or not).
>   ;; instead the image is removed completely
>   ;;(indent-rigidly content-begin content-end 1)
> 
>   ;; This line should hide the image, but it doesn't
>   ;;(overlay-put (make-overlay content-begin content-end) 'invisible t)
> )
> 
> In this case, `indent-rigidly` does not remove the image, but hiding the overlay does not work.

For the overlays part, see the above: put-image cannot change that
basic fact.  If you are saying that notmuch was able to hide images by
using overlays, you will need to tell more and show some code which
did that in some prior version of Emacs.

As for indentation: put-image uses a different default for the text on
which it puts the image 'display' property.  That explains why
re-indentation didn't remove the image in Emacs 28.  Perhaps we should
modify mm-inline-image to use a non-whitespace text on which to put
the image?  Can you try that?  Lars, any reason you explicitly used
whitespace as the STRING argument of insert-image that replaced
put-image in mm-inline-image?

bug#62637: 29.0.60; Issues when displaying images

[Eli Zaretskii]

> Cc: 62637@debbugs.gnu.org
> Date: Mon, 03 Apr 2023 16:35:02 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> As for indentation: put-image uses a different default for the text on
> which it puts the image 'display' property.  That explains why
> re-indentation didn't remove the image in Emacs 28.

Actually, put-image uses an overlay string with the 'display'
property, so it doesn't put the property on buffer text, and thus
re-indentation doesn't affect it.

bug#62637: 29.0.60; Issues when displaying images

[Al Haji-Ali]

Thanks Lars for explaining.

I understand the issue now regarding indentation. I think passing a non-empty string to `insert-image` in `mml-inline-image` (and maybe by default in `insert-image`?) would be better IMO.

Regarding the overlay visibility, the issue for me was the different behaviour of `put-image` and `insert-image` in different cases. Which can be seen in the following example:

(let (content-begin content-end
      (img (create-image "/tmp/tmp.png")))
  (goto-char (point-max))

  (insert "\ninsert-image:\n")
  (setq content-begin (point))
  (insert-image img "I")
  (insert "\n")
  (setq content-end (point))
  (overlay-put (make-overlay content-begin content-end) 'invisible t)

  (insert "\nput-image without new line:\n")
  (setq content-begin (point))
  (put-image img (point-marker))
  (setq content-end (point))
  (overlay-put (make-overlay content-begin content-end) 'invisible t)
  
  (insert "\nput-image with new line:\n")
  (setq content-begin (point))
  (put-image img (point-marker))
  (insert "\n")
  (setq content-end (point))
  (overlay-put (make-overlay content-begin content-end) 'invisible t))

This code tests overlay visibility for images with three cases:
- using insert-image: as you explained this does not work as expected.
- using put-image: Since put-images uses an overlay, I guess the same logic applies.
- put-image with a newline: Seems to work for some reason. This is what `mml-inline-image` was using before and why notmuch was working.

Sorry for the noise. I am not sure there's a bug in this case, only unjustified expectations as you put it.

Best regards,
-- Al

On 03/04/2023, Eli Zaretskii wrote:
>
>
>> Cc: 62637@debbugs.gnu.org
>> Date: Mon, 03 Apr 2023 16:35:02 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>>
>> As for indentation: put-image uses a different default for the text on
>> which it puts the image 'display' property.  That explains why
>> re-indentation didn't remove the image in Emacs 28.
>
> Actually, put-image uses an overlay string with the 'display'
> property, so it doesn't put the property on buffer text, and thus
> re-indentation doesn't affect it.

bug#62637: 29.0.60; Issues when displaying images

[Eli Zaretskii]

> From: Al Haji-Ali <abdo.haji.ali@gmail.com>
> Cc: larsi@gnus.org, 62637@debbugs.gnu.org
> Date: Mon, 03 Apr 2023 19:30:38 +0100
> 
> This code tests overlay visibility for images with three cases:
> - using insert-image: as you explained this does not work as expected.
> - using put-image: Since put-images uses an overlay, I guess the same logic applies.
> - put-image with a newline: Seems to work for some reason. This is what `mml-inline-image` was using before and why notmuch was working.

I think the invisible overlay will work if its span is different from
exactly the same text as the one on which image is defined.  I think
notmuch could simply copy the old definition of mml-inline-image, and
use that.

> Sorry for the noise. I am not sure there's a bug in this case, only unjustified expectations as you put it.

No sweat.

I'd still like to hear from Lars about the string he used in the
insert-image call: should we replace it with a non-whitespace string?

bug#62637: 29.0.60; Issues when displaying images

[Eli Zaretskii]

> Cc: 62637@debbugs.gnu.org
> Date: Mon, 03 Apr 2023 16:35:02 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> As for indentation: put-image uses a different default for the text on
> which it puts the image 'display' property.  That explains why
> re-indentation didn't remove the image in Emacs 28.  Perhaps we should
> modify mm-inline-image to use a non-whitespace text on which to put
> the image?  Can you try that?  Lars, any reason you explicitly used
> whitespace as the STRING argument of insert-image that replaced
> put-image in mm-inline-image?

No further comments, so I've now modified mm-inline-image to use "x"
as the string on which to put the image 'display' property.
Hopefully, this will allow notmuch and other applications to re-indent
images inserted using mm-inline-image.

And with that, I'm closing the bug.