all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Stefan Kangas <stefan@marxist.se>
Cc: 51296@debbugs.gnu.org
Subject: bug#51296: [PATCH] Add WebP format support
Date: Wed, 20 Oct 2021 19:35:17 +0300	[thread overview]
Message-ID: <83y26nac3e.fsf@gnu.org> (raw)
In-Reply-To: <CADwFkmnr=P9rvg5sTK1vjQWBwojpPhcgm7QQAvVQ5shs78mtjg@mail.gmail.com> (message from Stefan Kangas on Wed, 20 Oct 2021 08:22:44 -0700)

> From: Stefan Kangas <stefan@marxist.se>
> Date: Wed, 20 Oct 2021 08:22:44 -0700
> Cc: 51296@debbugs.gnu.org
> 
> > 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")






  reply	other threads:[~2021-10-20 16:35 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-19 23:27 bug#51296: [PATCH] Add WebP format support Stefan Kangas
2021-10-20  8:42 ` Lars Ingebrigtsen
2021-10-20 12:40   ` Stefan Kangas
2021-10-20 13:02 ` Stefan Kangas
2021-10-20 13:14   ` Eli Zaretskii
2021-10-20 15:22     ` Stefan Kangas
2021-10-20 16:35       ` Eli Zaretskii [this message]
2021-10-20 17:13         ` Eli Zaretskii
2021-10-20 17:41           ` Stefan Kangas
2021-10-20 18:19             ` Eli Zaretskii
2021-10-20 21:02         ` Stefan Kangas
2021-10-21  0:45           ` Stefan Kangas
2021-10-21  8:19             ` Eli Zaretskii
2021-10-21 18:36               ` Stefan Kangas
2021-10-21 18:39                 ` Eli Zaretskii
2021-10-21 21:19                   ` Stefan Kangas
2021-10-22  6:16                     ` Eli Zaretskii
2021-10-22  9:03                       ` Stefan Kangas
2021-10-22 12:06                         ` Eli Zaretskii
2021-10-22 12:47                           ` Eli Zaretskii
2021-10-22 14:27                             ` Stefan Kangas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83y26nac3e.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=51296@debbugs.gnu.org \
    --cc=stefan@marxist.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.