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

[George Huebner <george@feyor.sh>]

[-- Attachment #1: Type: text/plain, Size: 3373 bytes --]

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Elfeed: render after display --]
[-- Type: text/x-patch, Size: 1144 bytes --]

From 8f911a56beaf52f0df18537d0f9abae8373d7133 Mon Sep 17 00:00:00 2001
From: George Huebner <george@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


[-- Attachment #3: Type: text/plain, Size: 1004 bytes --]


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@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.