GNU bug report logs - #71913
29.1; shr: shr-resize-image does not behave as expected

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 71913 in the body.
You can then email your comments to 71913 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: George Huebner <george <at> feyor.sh>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.1; shr: shr-resize-image does not behave as expected
Date: Wed, 03 Jul 2024 01:37:52 -0500

I noticed an issue with =shr-max-image-proportion= wherein images 
would be correctly resized on initial load, but would then appear 
much larger.
I suspected this was an issue with caching, and sure enough, 
either disabling the cache with ~(setq shr-ignore-cache t)~ or 
manually deleting images from the cache results in images 
displaying correctly.
This unanswered 
[[https://www.reddit.com/r/emacs/comments/esa3x5/image_scaling_in_elfeed_entry_window/][Reddit 
post]] seems to corroborate this behaviour.
It looks like it might be because of the codepath taken in 
~shr-tag-img~ wherein a cache miss results in the creation of a 
placeholder image; I had some trouble with edebug though, so 
that's just an educated guess.

Here's a minimal reproducible example (credit to Sacha Chua):
#+begin_src elisp :eval no
;; run this (image is very large), run again many times without 
  killing *test* (image is correct size), kill *test* and run 
  again (image is large again)
;; won't observe this behaviour if you disable cache
;; (setq shr-ignore-cache t)
(setq shr-max-image-proportion 0.5)
(with-current-buffer (get-buffer-create "*test*")
 (erase-buffer)
 (insert "<img 
 src=\"https://upload.wikimedia.org/wikipedia/commons/8/83/The_GNU_logo.png\">")
 (shr-insert-document (libxml-parse-html-region (point-min) 
 (point-max)))
 (display-buffer (current-buffer)))
#+end_src elisp

I also observed this behaviour on Emacs master (667ca66), but I 
daily drive 29.1.
In GNU Emacs 29.1 (build 1, aarch64-apple-darwin23.4.0, Carbon 
Version
170 AppKit 2487.5)
Windowing system distributor 'Apple Inc.', version 14.4.0
System Description:  macOS 14.4

Configured using:
'configure
--prefix=/nix/store/ismv7jzf3hcqziq5bpjfs54zd4qfjjn7-emacs-mac-macport-29.1
--disable-build-details --with-modules --without-gif 
--without-jpeg
--without-png --without-tiff --without-x --without-xpm
'--enable-mac-app=$$out/Applications' --with-gnutls --with-mac
--with-xml2 --without-ns --with-compress-install
--with-toolkit-scroll-bars --with-native-compilation
--without-imagemagick --with-mailutils --without-small-ja-dic
--with-tree-sitter --without-xinput2 --without-xwidgets 
--without-dbus
--without-selinux --with-xwidgets'

Message #8 received at 71913 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: George Huebner <george <at> feyor.sh>, 71913 <at> debbugs.gnu.org
Subject: Re: bug#71913: 29.1; shr: shr-resize-image does not behave as expected
Date: Wed, 3 Jul 2024 08:48:46 -0700

On 7/2/2024 11:37 PM, George Huebner wrote:
> I noticed an issue with =shr-max-image-proportion= wherein images would 
> be correctly resized on initial load, but would then appear much larger.
[snip]
> Here's a minimal reproducible example (credit to Sacha Chua):
> #+begin_src elisp :eval no
> ;; run this (image is very large), run again many times without   
> killing *test* (image is correct size), kill *test* and run   again 
> (image is large again)
> ;; won't observe this behaviour if you disable cache
> ;; (setq shr-ignore-cache t)
> (setq shr-max-image-proportion 0.5)
> (with-current-buffer (get-buffer-create "*test*")
>   (erase-buffer)
>   (insert "<img 
>   src=\"https://upload.wikimedia.org/wikipedia/commons/8/83/The_GNU_logo.png\">")
>   (shr-insert-document (libxml-parse-html-region (point-min)  (point-max)))
>   (display-buffer (current-buffer)))
> #+end_src elisp

The bug here is really in your steps to reproduce, which I suppose is 
roughly what Elfeed is doing too (though I haven't looked at that code 
to be totally sure).

SHR scales images with respect to the size of the buffer's window. 
However, by rendering the HTML document with SHR *before* displaying the 
buffer in a window, it's impossible for Emacs to do that: there's no 
such window yet! If you swap the 'display-buffer' and 
'shr-insert-document' lines though, all should work properly.

Message #11 received at 71913 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: George Huebner <george <at> feyor.sh>
Cc: 71913 <at> debbugs.gnu.org
Subject: Re: bug#71913: 29.1; shr: shr-resize-image does not behave as expected
Date: Fri, 5 Jul 2024 11:11:29 -0700

(Cc'ing the bug list to follow up there too.)

On Wed, Jul 3, 2024 at 1:26 PM George Huebner <george <at> feyor.sh> wrote:
>
> You're quite right... I understood ~(not (get-buffer-window
> (current-buffer) t))~ (in shr-rescale-image) was preventing
> rescaling in undisplayed buffers, but in retrospect this is the
> responsibility of the caller, as you say.
>
> Apologies for the fruitless bug report, this thread can be closed.

On the contrary, I think it makes sense to do *something* here. At
minimum, Elfeed should probably get a fix, since I've noticed this
issue there too (but never bothered to figure out why). Now that you
found minimal steps to reproduce, hopefully we can fix Elfeed.

It might also be good to at least document this limitation, or even
enhance 'shr-max-image-proportion' to support absolute pixel sizes,
which would be immune to this issue.

Message #14 received at 71913 <at> debbugs.gnu.org (full text, mbox):

From: George Huebner <george <at> feyor.sh>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 71913 <at> debbugs.gnu.org
Subject: Re: bug#71913: 29.1; shr: shr-resize-image does not behave as expected
Date: Wed, 10 Jul 2024 14:02:33 -0500

[Message part 1 (text/plain, inline)]

Aha: after patching Elfeed to render after buffer display (which 
seems to fix the display of raster images), I actually did find 
behaviour that I believe should be addressed in shr, not in 
Elfeed; let me know what you think.

If an image is a SVG, `shr-put-image` will not respect 
`shr-max-image-proportion`, because it will pass it directly to 
`create-image`, instead of resizing it:
#+begin_src elisp
;; in `shr-put-image`
((eq content-type 'image/svg+xml)
        (when (image-type-available-p 'svg)
  (create-image data 'svg t :ascent shr-image-ascent)))
(t
(ignore-errors (shr-rescale-image data content-type
                                          (plist-get flags 
                                          :width)
                                          (plist-get flags 
                                          :height)))))))
#+end_src

As I understand it, this decision was likely made because the 
dimensions of SVGs is weird; they aren't inherently required to 
have a size, but the SVG itself can define a viewbox with 
accompanying width and height (full disclosure: I have no idea 
what I'm talking about).

Using `shr-rescale-image` instead seems to do the trick (and 
respects SVGs that should display smaller than 
`shr-max-image-proportion`), but please do let me know if I'm 
ignoring an edge case or there's a better way to address this. 

#+begin_src elisp
(setq shr-put-image-function (lambda (spec alt &optional flags)
 (if (display-graphic-p)
     (let* ((size (cdr (assq 'size flags)))
	     (data (if (consp spec)
		       (car spec)
		     spec))
	     (content-type (and (consp spec)
				(cadr spec)))
	     (start (point))
	     (image (cond
		     ((eq size 'original)
		      (create-image data nil t :ascent 100
				    :format content-type))
            ;; BEGIN fix
		     ((eq content-type 'image/svg+xml)
                     (when (image-type-available-p 'svg)
		                ; (create-image data 'svg t :ascent 100)))
		                (shr-rescale-image data 'svg (plist-get 
		                flags :width) (plist-get flags :height))))
            ;; END fix
		     ((eq size 'full)
		      (ignore-errors
			(shr-rescale-image data content-type
                                          (plist-get flags 
                                          :width)
                                          (plist-get flags 
                                          :height))))
		     (t
		      (ignore-errors
			(shr-rescale-image data content-type
                                          (plist-get flags 
                                          :width)
                                          (plist-get flags 
                                          :height)))))))
       (when image
	  ;; When inserting big-ish pictures, put them at the
	  ;; beginning of the line.
	  (when (and (> (current-column) 0)
		     (> (car (image-size image t)) 400))
	    (insert "\n"))
         (let ((image-pos (point)))
	    (if (eq size 'original)
	        (insert-sliced-image image (or alt "*") nil 20 1)
	      (insert-image image (or alt "*")))
	    (put-text-property start (point) 'image-size size)
	    (when (and shr-image-animate
                      (cdr (image-multi-frame-p image)))
             (image-animate image nil 60 image-pos))))
	image)
   (insert (or alt "")))))
#+end_src elisp

[elfeed-shr.patch (text/x-patch, inline)]

From 8f911a56beaf52f0df18537d0f9abae8373d7133 Mon Sep 17 00:00:00 2001
From: George Huebner <george <at> feyor.sh>
Date: Wed, 10 Jul 2024 01:57:15 -0500
Subject: [PATCH] elfeed-show: render after switching to entry buffer

shr can't respect `shr-max-image-proportion` because it isn't rendered
in a window; call `elfeed-search-show-entry` first to fix this.

See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71913
---
 elfeed-show.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/elfeed-show.el b/elfeed-show.el
index 4915cae..e16b4bf 100644
--- a/elfeed-show.el
+++ b/elfeed-show.el
@@ -214,9 +214,9 @@ The result depends on the value of `elfeed-show-unique-buffers'."
   (let ((buff (get-buffer-create (elfeed-show--buffer-name entry))))
     (with-current-buffer buff
       (elfeed-show-mode)
-      (setq elfeed-show-entry entry)
-      (elfeed-show-refresh))
-    (funcall elfeed-show-entry-switch buff)))
+      (setq elfeed-show-entry entry))
+    (funcall elfeed-show-entry-switch buff)
+    (elfeed-show-refresh)))
 
 (defun elfeed-show-next ()
   "Show the next item in the elfeed-search buffer."
-- 
2.44.1

[Message part 3 (text/plain, inline)]

On Fri, Jul  5, 2024 at 13:11:29 EST Jim Porter wrote:

> (Cc'ing the bug list to follow up there too.)
>
> On Wed, Jul 3, 2024 at 1:26 PM George Huebner <george <at> feyor.sh> 
> wrote:
>>
>> You're quite right... I understood ~(not (get-buffer-window
>> (current-buffer) t))~ (in shr-rescale-image) was preventing
>> rescaling in undisplayed buffers, but in retrospect this is the
>> responsibility of the caller, as you say.
>>
>> Apologies for the fruitless bug report, this thread can be 
>> closed.
>
> On the contrary, I think it makes sense to do *something* 
> here. At
> minimum, Elfeed should probably get a fix, since I've noticed 
> this
> issue there too (but never bothered to figure out why). Now that 
> you
> found minimal steps to reproduce, hopefully we can fix Elfeed.
>
> It might also be good to at least document this limitation, or 
> even
> enhance 'shr-max-image-proportion' to support absolute pixel 
> sizes,
> which would be immune to this issue.

Message #17 received at 71913 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: George Huebner <george <at> feyor.sh>
Cc: 71913 <at> debbugs.gnu.org
Subject: Re: bug#71913: 29.1; shr: shr-resize-image does not behave as expected
Date: Thu, 11 Jul 2024 16:51:53 -0700

[Message part 1 (text/plain, inline)]

On 7/10/2024 12:02 PM, George Huebner wrote:
> As I understand it, this decision was likely made because the dimensions 
> of SVGs is weird; they aren't inherently required to have a size, but 
> the SVG itself can define a viewbox with accompanying width and height 
> (full disclosure: I have no idea what I'm talking about).

I think this was just a mistake. I tried getting rid of the special case 
for SVG and everything works just fine in builds with or without SVG 
support. (In builds without SVG support, you just get the empty string 
where an SVG would be, before and after this patch.)

[0001-Treat-SVG-images-like-other-image-types-in-shr-put-i.patch (text/plain, attachment)]

Message #20 received at 71913 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: george <at> feyor.sh, 71913 <at> debbugs.gnu.org
Subject: Re: bug#71913: 29.1; shr: shr-resize-image does not behave as expected
Date: Fri, 12 Jul 2024 08:58:44 +0300

> Cc: 71913 <at> debbugs.gnu.org
> Date: Thu, 11 Jul 2024 16:51:53 -0700
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 7/10/2024 12:02 PM, George Huebner wrote:
> > As I understand it, this decision was likely made because the dimensions 
> > of SVGs is weird; they aren't inherently required to have a size, but 
> > the SVG itself can define a viewbox with accompanying width and height 
> > (full disclosure: I have no idea what I'm talking about).
> 
> I think this was just a mistake. I tried getting rid of the special case 
> for SVG and everything works just fine in builds with or without SVG 
> support. (In builds without SVG support, you just get the empty string 
> where an SVG would be, before and after this patch.)

What version of librsvg did you test this with?  Please test also with
librsvg 2.40.x, as that was the last version which didn't need a Rust
compiler to build, and so some people (including yours truly) stay
with those old versions to this day.

Thanks.

Message #23 received at 71913 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: george <at> feyor.sh, 71913 <at> debbugs.gnu.org
Subject: Re: bug#71913: 29.1; shr: shr-resize-image does not behave as expected
Date: Fri, 12 Jul 2024 21:11:35 -0700

On 7/11/2024 10:58 PM, Eli Zaretskii wrote:
> What version of librsvg did you test this with?  Please test also with
> librsvg 2.40.x, as that was the last version which didn't need a Rust
> compiler to build, and so some people (including yours truly) stay
> with those old versions to this day.

I'm usually using 2.48.x since it's what my distro provides, but I've 
now tested against 2.40.21 (after a slightly bumpy road getting 
everything to build correctly) and the results are the same for both 
versions of librsvg. (Out of an abundance of caution, I've also verified 
with 'ldd' that the correct version of librsvg is in use during these 
tests.)

Message #26 received at 71913 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: george <at> feyor.sh, 71913 <at> debbugs.gnu.org
Subject: Re: bug#71913: 29.1; shr: shr-resize-image does not behave as expected
Date: Sat, 13 Jul 2024 09:20:39 +0300

> Date: Fri, 12 Jul 2024 21:11:35 -0700
> Cc: george <at> feyor.sh, 71913 <at> debbugs.gnu.org
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 7/11/2024 10:58 PM, Eli Zaretskii wrote:
> > What version of librsvg did you test this with?  Please test also with
> > librsvg 2.40.x, as that was the last version which didn't need a Rust
> > compiler to build, and so some people (including yours truly) stay
> > with those old versions to this day.
> 
> I'm usually using 2.48.x since it's what my distro provides, but I've 
> now tested against 2.40.21 (after a slightly bumpy road getting 
> everything to build correctly) and the results are the same for both 
> versions of librsvg. (Out of an abundance of caution, I've also verified 
> with 'ldd' that the correct version of librsvg is in use during these 
> tests.)

Thanks.  Then I have no objections for fixing this on the emacs-30
release branch.

Message #29 received at 71913 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: george <at> feyor.sh, 71913 <at> debbugs.gnu.org
Subject: Re: bug#71913: 29.1; shr: shr-resize-image does not behave as expected
Date: Sat, 13 Jul 2024 10:29:43 -0700

On 7/12/2024 11:20 PM, Eli Zaretskii wrote:
> Thanks.  Then I have no objections for fixing this on the emacs-30
> release branch.

Sounds good. I've merged my patch to the master branch though (as 
f38c42d1c7a), since the code change depends on the second round of 
patches from bug#71666, which are also only on the master branch.

I could probably make an Emacs 30 version of this patch if desired, but 
I think this particular issue is fairly small (and has been around for a 
long time). Maybe it's better to play it safe and leave it on the master 
branch only? I don't have a strong opinion here.

Message #32 received at 71913 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: george <at> feyor.sh, 71913 <at> debbugs.gnu.org
Subject: Re: bug#71913: 29.1; shr: shr-resize-image does not behave as expected
Date: Sat, 13 Jul 2024 20:39:50 +0300

> Date: Sat, 13 Jul 2024 10:29:43 -0700
> Cc: george <at> feyor.sh, 71913 <at> debbugs.gnu.org
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 7/12/2024 11:20 PM, Eli Zaretskii wrote:
> > Thanks.  Then I have no objections for fixing this on the emacs-30
> > release branch.
> 
> Sounds good. I've merged my patch to the master branch though (as 
> f38c42d1c7a), since the code change depends on the second round of 
> patches from bug#71666, which are also only on the master branch.
> 
> I could probably make an Emacs 30 version of this patch if desired, but 
> I think this particular issue is fairly small (and has been around for a 
> long time). Maybe it's better to play it safe and leave it on the master 
> branch only? I don't have a strong opinion here.

Leaving this on master is fine by me, thanks.

Message #37 received at 71913-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: george <at> feyor.sh, 71913-done <at> debbugs.gnu.org
Subject: Re: bug#71913: 29.1; shr: shr-resize-image does not behave as expected
Date: Sat, 13 Jul 2024 11:54:21 -0700

On 7/13/2024 10:39 AM, Eli Zaretskii wrote:
> Leaving this on master is fine by me, thanks.

Thanks. Closing this bug now, then.

George: we can track this elsewhere since it's not an Emacs bug per se, 
but did you want to submit your Elfeed patch to Elfeed? If you do, let 
me know and I'd be happy to chime in over there to support it getting 
merged.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.