From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#51296: [PATCH] Add WebP format support Date: Wed, 20 Oct 2021 19:35:17 +0300 Message-ID: <83y26nac3e.fsf@gnu.org> References: <837de7bzy0.fsf@gnu.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="22720"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 51296@debbugs.gnu.org To: Stefan Kangas Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Oct 20 18:38:02 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mdEbJ-0005ek-Kf for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 20 Oct 2021 18:38:01 +0200 Original-Received: from localhost ([::1]:36136 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mdEbI-000193-6e for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 20 Oct 2021 12:38:00 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:51388) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mdEZO-0000jo-6H for bug-gnu-emacs@gnu.org; Wed, 20 Oct 2021 12:36:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:43774) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mdEZN-0002uX-T7 for bug-gnu-emacs@gnu.org; Wed, 20 Oct 2021 12:36:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mdEZN-0004mG-QP for bug-gnu-emacs@gnu.org; Wed, 20 Oct 2021 12:36:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 20 Oct 2021 16:36:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 51296 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 51296-submit@debbugs.gnu.org id=B51296.163474771518309 (code B ref 51296); Wed, 20 Oct 2021 16:36:01 +0000 Original-Received: (at 51296) by debbugs.gnu.org; 20 Oct 2021 16:35:15 +0000 Original-Received: from localhost ([127.0.0.1]:55320 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mdEYd-0004lE-DS for submit@debbugs.gnu.org; Wed, 20 Oct 2021 12:35:15 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:49266) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mdEYa-0004kx-Ie for 51296@debbugs.gnu.org; Wed, 20 Oct 2021 12:35:12 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:47182) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mdEYU-0001v1-MZ; Wed, 20 Oct 2021 12:35:06 -0400 Original-Received: from [87.69.77.57] (port=2049 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mdEYU-00027D-9Y; Wed, 20 Oct 2021 12:35:06 -0400 In-Reply-To: (message from Stefan Kangas on Wed, 20 Oct 2021 08:22:44 -0700) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:217691 Archived-At: > From: Stefan Kangas > 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")