Subject: bug#51296: [PATCH] Add WebP format support

From: Eli Zaretskii <[email-masked]>
Date: Wed, 20 Oct 2021 19:35:17 +0300

> From: Stefan Kangas <[email-masked]>
> Date: Wed, 20 Oct 2021 08:22:44 -0700
> Cc: [email-masked]
> 
> > You missed the fact that the MW-Windows build loads image libraries
> > on-demand, when/if the library is first required.  That affects the
> > way we support these in configure.ac, and it also needs an addition to
> > dynamic-library-alist in w32-win.el.
> >
> > It should be easy to add those nits (assuming the code works ;-)
> 
> I attempted to add the `dynamic-library-alist' part in the attached, and
> I also added a NEWS item.  I suppose someone who actually knows
> MS-Windows stuff and can test it should take a look.

Thanks.  If you install this when I'm awake, I can fix it in almost
real time.  For now, just a couple of comments based on reading the
patch:

> +HAVE_WEBP=no
> +if test "${HAVE_X11}" = "yes" || test "${HAVE_NS}" = "yes" || test "${opsys}" = "mingw32"; then
> +   if test "${with_webp}" != "no"; then
> +      WEBP_REQUIRED=0.6.0
> +      WEBP_MODULE="libwebp >= $WEBP_REQUIRED"
> +
> +      EMACS_CHECK_MODULES([WEBP], [$WEBP_MODULE])
> +      AC_SUBST(WEBP_CFLAGS)
> +      AC_SUBST(WEBP_LIBS)
> +
> +      if test $HAVE_WEBP = yes; then
> +        AC_DEFINE(HAVE_WEBP, 1, [Define to 1 if using libwebp.])
> +        CFLAGS="$CFLAGS $WEBP_CFLAGS"
> +      fi
> +   fi
> +fi

This is sub-optimal: it causes the Windows build to link with -lwebp,
which then makes the binary _require_ to have libwebp DLL when Emacs
starts.  (Unlike on Posix platforms, Windows binaries linked with a
shared library insist on finding it at startup, because the Windows
dynamic linking functionality needs that to resolve all the entry
points.)  This would require people who download the prebuilt binaries
to also have the library installed, or else Emacs will refuse to
start, even if the user has no use for WebP images.  So we instead do
this (this example is for libpng):

  # mingw32 loads the library dynamically.
  if test "$opsys" = mingw32; then
    AC_CHECK_HEADER([png.h], [HAVE_PNG=yes])
  elif test "${HAVE_X11}" = "yes" || test "${HAVE_W32}" = "yes" \
       || test "${HAVE_NS}" = "yes"; then
    EMACS_CHECK_MODULES([PNG], [libpng >= 1.0.0])
    if test $HAVE_PNG = yes; then
      LIBPNG=$PNG_LIBS

That is, for MS-Windows we only check for the header, because we need
that to compile the code which supports the image type.  But we don't
add -lFOO to the linker switches.  (If you wonder what is HAVE_W32
about, then it's for Cygwin builds that use native w32 GUI "toolkit"
instead of X; Cygwin builds are linked like on Unix, and don't load
the image libraries on demand.)

> --- a/lisp/term/w32-win.el
> +++ b/lisp/term/w32-win.el
> @@ -274,6 +274,7 @@ libgnutls-version
>  	     '(gif "libgif-6.dll" "giflib5.dll" "gif.dll")
>  	 '(gif "libgif-5.dll" "giflib4.dll" "libungif4.dll" "libungif.dll")))
>         '(svg "librsvg-2-2.dll")
> +       '(libwebp "libwebp.dll")

This requires some Internet search, because DLLs on Windows usually
have a numeric version indication in their names, as you see in the
other cases.  In this case, the name produced by current versions of
libwebp seems to be libwebp-7.dll, not just libwebp.dll.  But it's
probably a good idea to leave libwebp.dll as a fallback.  So this
should be

     '(libwebp "libwebp-7.dll" "libwebp.dll")