bug#57961: 29.0.50; [PATCH] image-dired thumbnail generation fails for PDFs on macOS

bug#57961: 29.0.50; [PATCH] image-dired thumbnail generation fails for PDFs on macOS

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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


I tried the new PDF thumbnail generation feature in image-dired and I
got the following error in the macOS console, and the thumbnails are
blank:

CoreGraphics PDF has logged an error. Set environment variable
"CG_PDF_VERBOSE" to learn more

The reason seems to be that the thumbnails are actually JPG files, but
they are created with PDF extension, so the image loading code in the NS
port sees the extension and tries to search for a %PDF marker in the
file, which always fails.

I've created this patch so that PDF thumbnails get the correct JPG or
PNG extension.  That fixes the problem on macOS, at least.

Any ideas if the logic is correct and makes sense?  Thanks.

In GNU Emacs 29.0.50 (build 3, aarch64-apple-darwin21.6.0, NS
 appkit-2113.60 Version 12.6 (Build 21G115)) of 2022-09-20 built on
 Daniels-MacBook-Pro.local
Repository revision: 1231a601ebe1fd9fe454c504dbeb9267440242e7
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2113
System Description:  macOS 12.6

Configured using:
 'configure CPPFLAGS=-I/opt/homebrew/opt/openjdk@11/include'

Configured features:
ACL DBUS GLIB GNUTLS JSON LCMS2 LIBXML2 MODULES NOTIFY KQUEUE NS PDUMPER
PNG RSVG SQLITE3 THREADS TOOLKIT_SCROLL_BARS WEBP XIM ZLIB

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

Major mode: Messages

Minor modes in effect:
  tooltip-mode: t
  global-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
  buffer-read-only: 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 rfc822
mml mml-sec password-cache epa derived epg rfc6068 epg-config gnus-util
text-property-search mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils time-date subr-x misearch multi-isearch
format-spec vc-git diff-mode easy-mmode vc-dispatcher image-file
image-converter dired-aux image-dired image-dired-tags
image-dired-external image-dired-util image-mode wallpaper xdg exif
cl-loaddefs cl-lib dired dired-loaddefs rmc iso-transl tooltip 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 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 dbusbind kqueue cocoa ns
lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 82066 6542)
 (symbols 48 6208 0)
 (strings 32 19408 2168)
 (string-bytes 1 585126)
 (vectors 16 11857)
 (vector-slots 8 170638 10214)
 (floats 8 39 25)
 (intervals 56 5237 164)
 (buffers 1000 12))


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-a-bug-where-PDF-thumbnails-were-stored-with-PDF-.patch --]
[-- Type: text/x-patch, Size: 2433 bytes --]

From 37583170eff4858b6bd288cc78e2ec967ee0073f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Tue, 20 Sep 2022 22:27:56 +0200
Subject: [PATCH] Fix a bug where PDF thumbnails were stored with PDF extension

* lisp/image/image-dired-util.el (image-dired-file-name-extension):
New function to compute the filename extension for a thumbnail.  This
new function ensures that the extension of a PDF thumbnail is either
JPG or PNG.
* lisp/image/image-dired-util.el (image-dired-thumb-name): Use the new
function.
---
 lisp/image/image-dired-util.el | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
index dcf0b22cfe..89513f9441 100644
--- a/lisp/image/image-dired-util.el
+++ b/lisp/image/image-dired-util.el
@@ -57,6 +57,19 @@ image-dired-dir
       (message "Thumbnail directory created: %s" image-dired-dir))
     image-dired-dir))
 
+(defun image-dired-file-name-extension (file)
+  "Return the filename extension for thumbnail FILE.
+Return the value of `file-name-extension', but for PDF files
+return PNG or JPG, depending on the thumbnail storage
+configuration."
+  (let ((extension (file-name-extension file)))
+    (cond ((string-equal extension "pdf")
+           (cond ((memq image-dired-thumbnail-storage
+                        image-dired--thumbnail-standard-sizes)
+                  "png")
+                 (t "jpg")))
+          (t extension))))
+
 (defun image-dired-thumb-name (file)
   "Return absolute file name for thumbnail FILE.
 Depending on the value of `image-dired-thumbnail-storage', the
@@ -91,13 +104,13 @@ image-dired-thumb-name
                    (file-name-as-directory (expand-file-name (image-dired-dir)))
                    (file-name-base f)
                    (if hash (concat "_" hash) "")
-                   (file-name-extension f))))
+                   (image-dired-file-name-extension f))))
         ((eq 'per-directory image-dired-thumbnail-storage)
          (let ((f (expand-file-name file)))
            (format "%s.image-dired/%s.thumb.%s"
                    (file-name-directory f)
                    (file-name-base f)
-                   (file-name-extension f))))))
+                   (image-dired-file-name-extension f))))))
 
 (defvar image-dired-thumbnail-buffer "*image-dired*"
   "Image-Dired's thumbnail buffer.")
-- 
2.34.1

bug#57961: 29.0.50; [PATCH] image-dired thumbnail generation fails for PDFs on macOS

[Lars Ingebrigtsen]

Daniel Martín <mardani29@yahoo.es> writes:

> I've created this patch so that PDF thumbnails get the correct JPG or
> PNG extension.  That fixes the problem on macOS, at least.
>
> Any ideas if the logic is correct and makes sense?  Thanks.

I think the patch makes sense.  Perhaps Stefan has some comments; added
to the CCs.

bug#57961: 29.0.50; [PATCH] image-dired thumbnail generation fails for PDFs on macOS

[Stefan Kangas]

Daniel Martín via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> I've created this patch so that PDF thumbnails get the correct JPG or
> PNG extension.  That fixes the problem on macOS, at least.

Thanks for your testing and the patch!  Some comments below:

> diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
> index dcf0b22cfe..89513f9441 100644
> --- a/lisp/image/image-dired-util.el
> +++ b/lisp/image/image-dired-util.el
> @@ -57,6 +57,19 @@ image-dired-dir
>        (message "Thumbnail directory created: %s" image-dired-dir))
>      image-dired-dir))
>
> +(defun image-dired-file-name-extension (file)
> +  "Return the filename extension for thumbnail FILE.
> +Return the value of `file-name-extension', but for PDF files
> +return PNG or JPG, depending on the thumbnail storage
> +configuration."
> +  (let ((extension (file-name-extension file)))
> +    (cond ((string-equal extension "pdf")
> +           (cond ((memq image-dired-thumbnail-storage
> +                        image-dired--thumbnail-standard-sizes)
> +                  "png")
> +                 (t "jpg")))
> +          (t extension))))
> +

I don't think we need this when

    (memq image-dired-thumbnail-storage
          image-dired--thumbnail-standard-sizes)

because the files are then already saved to, e.g.:

    ~/.cache/thumbnails/normal/790f6914e3e396bf1b63f20769bd531d.png

>  (defun image-dired-thumb-name (file)
>    "Return absolute file name for thumbnail FILE.
>  Depending on the value of `image-dired-thumbnail-storage', the
> @@ -91,13 +104,13 @@ image-dired-thumb-name
>                     (file-name-as-directory (expand-file-name (image-dired-dir)))
>                     (file-name-base f)
>                     (if hash (concat "_" hash) "")
> -                   (file-name-extension f))))
> +                   (image-dired-file-name-extension f))))

Given that we already hard-code the use of JPEG, perhaps we should just
hard-code ".jpg" here too?

Of course, that will break when users start messing with
`image-dired-cmd-create-thumbnail-options' but on the other hand they
are then on their own in any case.

>          ((eq 'per-directory image-dired-thumbnail-storage)
>           (let ((f (expand-file-name file)))
>             (format "%s.image-dired/%s.thumb.%s"
>                     (file-name-directory f)
>                     (file-name-base f)
> -                   (file-name-extension f))))))
> +                   (image-dired-file-name-extension f))))))

Same here.

bug#57961: 29.0.50; [PATCH] image-dired thumbnail generation fails for PDFs on macOS

[Stefan Kangas]

BTW, I just pushed some tests to master (commit 661be73b5e), so I please
update them too.

Thanks in advance.

bug#57961: 29.0.50; [PATCH] image-dired thumbnail generation fails for PDFs on macOS

[Stefan Kangas]

close 57961 29.1
thanks

Stefan Kangas <stefankangas@gmail.com> writes:

> Given that we already hard-code the use of JPEG, perhaps we should just
> hard-code ".jpg" here too?

I took a closer look at this, and I decided that while forcing the file
extension to ".jpg", we might as well take the opportunity to simplify
the naming of thumbnails.

So I did that in commit 6cffaa3b6d, and I'm therefore closing this bug report.