Optional support for GDI+ on Windows (emacs-28)

Optional support for GDI+ on Windows (emacs-28)

[Juan José García-Ripoll]

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

Here is a more canonical patch for the Windows port:

When emacs is configured with --with-gdiplus and it is built on Mingw,
--with-jpeg, --with-png, --with-tiff and --with-gif are disabled and
those image formats are implemented using Windows' GDI+ library.

-- 
Juan José García Ripoll
http://juanjose.garciaripoll.com
http://quinfog.hbar.es


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gdiplus.diff --]
[-- Type: text/x-patch, Size: 21235 bytes --]

diff --git a/configure.ac b/configure.ac
index a4daf1414d..0808bd86b9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -433,6 +433,7 @@ AC_DEFUN
 OPTION_DEFAULT_ON([cairo],[don't compile with Cairo drawing])
 OPTION_DEFAULT_ON([xml2],[don't compile with XML parsing support])
 OPTION_DEFAULT_OFF([imagemagick],[compile with ImageMagick image support])
+OPTION_DEFAULT_ON([gdiplus], [disable use of GDI+ on Windows for JPEG/TIFF/GIFF/PNG])
 OPTION_DEFAULT_ON([json], [don't compile with native JSON support])
 
 OPTION_DEFAULT_ON([xft],[don't use XFT for anti aliased fonts])
@@ -2132,6 +2133,7 @@ AC_DEFUN
 NTLIB=
 CM_OBJ="cm.o"
 XARGS_LIMIT=
+HAVE_GDIPLUS=no
 if test "${HAVE_W32}" = "yes"; then
   AC_DEFINE(HAVE_NTGUI, 1, [Define to use native MS Windows GUI.])
   if test "$with_toolkit_scroll_bars" = "no"; then
@@ -2160,8 +2162,14 @@ AC_DEFUN
     # the rc file), not a linker script.
     W32_RES_LINK="-Wl,emacs.res"
   else
-    W32_OBJ="$W32_OBJ w32.o w32console.o w32heap.o w32inevt.o w32proc.o"
-    W32_LIBS="$W32_LIBS -lwinmm -lusp10 -lgdi32 -lcomdlg32"
+    if test "${with_gdiplus}" = yes; then
+      AC_DEFINE(HAVE_GDIPLUS, 1, [Define to use MS Windows GDI+ for images.])
+      HAVE_GDIPLUS=yes
+      W32_GDIPLUS="w32image.o"
+      W32_GDILIBS="-lgdiplus -lshlwapi"
+    fi
+    W32_OBJ="$W32_OBJ w32.o w32console.o w32heap.o w32inevt.o w32proc.o $W32_GDIPLUS"
+    W32_LIBS="$W32_LIBS -lwinmm -lusp10 $W32_GDILIBS -lgdi32 -lcomdlg32"
     W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32"
     W32_RES_LINK="\$(EMACSRES)"
     CLIENTRES="emacsclient.res"
@@ -3572,8 +3580,8 @@ AC_DEFUN
 ### Use -ljpeg if available, unless '--with-jpeg=no'.
 HAVE_JPEG=no
 LIBJPEG=
-if test "${NS_IMPL_COCOA}" = yes; then
-  : # Cocoa provides its own jpeg support, so do nothing.
+if test "${NS_IMPL_COCOA}" = yes || test "${HAVE_GDIPLUS}" = "yes"; then
+  : # Cocoa and Windows' GDI+ provide their own jpeg support, so do nothing.
 elif test "${HAVE_X11}" = "yes" || test "${HAVE_W32}" = "yes"; then
   if test "${with_jpeg}" != "no"; then
     AC_CACHE_CHECK([for jpeglib 6b or later],
@@ -3723,8 +3731,8 @@ AC_DEFUN
 HAVE_PNG=no
 LIBPNG=
 PNG_CFLAGS=
-if test "${NS_IMPL_COCOA}" = yes; then
-  : # Cocoa provides its own png support, so do nothing.
+if test "${NS_IMPL_COCOA}" = yes || test "${HAVE_GDIPLUS}" = "yes"; then
+  : # Cocoa and Windows' GDI+ provide their own png support, so do nothing.
 elif test "${with_png}" != no; then
   # mingw32 loads the library dynamically.
   if test "$opsys" = mingw32; then
@@ -3796,7 +3804,9 @@ AC_DEFUN
 ### mingw32 doesn't use -ltiff, since it loads the library dynamically.
 HAVE_TIFF=no
 LIBTIFF=
-if test "${opsys}" = "mingw32"; then
+if test "${HAVE_GDIPLUS}" = "yes"; then
+  : # Windows' GDI+ supports TIFF
+elif test "${opsys}" = "mingw32"; then
   if test "${with_tiff}" != "no"; then
     AC_CHECK_HEADER(tiffio.h, HAVE_TIFF=yes, HAVE_TIFF=no)
   fi
@@ -3824,7 +3834,9 @@ AC_DEFUN
 ### mingw32 doesn't use -lgif/-lungif, since it loads the library dynamically.
 HAVE_GIF=no
 LIBGIF=
-if test "${opsys}" = "mingw32"; then
+if test "${HAVE_GDIPLUS}" = "yes"; then
+  : # Windows' GDI+ supports TIFF
+elif test "${opsys}" = "mingw32"; then
   if test "${with_gif}" != "no"; then
     AC_CHECK_HEADER(gif_lib.h, HAVE_GIF=yes, HAVE_GIF=no)
   fi
@@ -5707,6 +5719,7 @@ AC_DEFUN
   Does Emacs use a png library?                           ${HAVE_PNG} $LIBPNG
   Does Emacs use -lrsvg-2?                                ${HAVE_RSVG}
   Does Emacs use cairo?                                   ${HAVE_CAIRO}
+  Does Emacs use GDI+?                                    ${HAVE_GDIPLUS}
   Does Emacs use -llcms2?                                 ${HAVE_LCMS2}
   Does Emacs use imagemagick?                             ${HAVE_IMAGEMAGICK}
   Does Emacs support sound?                               ${HAVE_SOUND}
diff --git a/etc/NEWS b/etc/NEWS
index 870d39f7ee..6fc327665d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -297,6 +297,12 @@ such as "2020-01-15T16:12:21-08:00".
 'module-file-suffix' now has the value ".dylib" on macOS, but the
 ".so" suffix is supported as well.
 
++++
+** Emacs can now use Microsoft Windows GDI+ library to load bitmap images in
+JPEG, PNG, GIF and TIFF formats.  This support is enabled with --with-gdiplus,
+which automatically disables the use of third party libraries for those
+formats.
+
 \f
 ----------------------------------------------------------------------
 This file is part of GNU Emacs.
diff --git a/src/Makefile.in b/src/Makefile.in
index 552dd2e50a..0dc613352b 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -280,10 +280,12 @@ GNU_OBJC_CFLAGS=
 ## w32fns.o w32menu.c w32reg.o fringe.o fontset.o w32font.o w32term.o
 ## w32xfns.o w32select.o image.o w32uniscribe.o w32cygwinx.o if HAVE_W32,
 ## w32cygwinx.o if CYGWIN but not HAVE_W32, else empty.
+## w32image.o if we use GDI+
 W32_OBJ=@W32_OBJ@
 ## -lkernel32 -luser32 -lusp10 -lgdi32 -lole32 -lcomdlg32 -lcomctl32
 ## -lwinspool if HAVE_W32,
 ## -lkernel32 if CYGWIN but not HAVE_W32, else empty.
+## -lshlwapi if we use GDI+
 W32_LIBS=@W32_LIBS@
 
 ## emacs.res if HAVE_W32
@@ -435,7 +437,7 @@ SOME_MACHINE_OBJECTS =
   fontset.o dbusbind.o cygw32.o \
   nsterm.o nsfns.o nsmenu.o nsselect.o nsimage.o nsfont.o macfont.o \
   w32.o w32console.o w32cygwinx.o w32fns.o w32heap.o w32inevt.o w32notify.o \
-  w32menu.o w32proc.o w32reg.o w32select.o w32term.o w32xfns.o \
+  w32menu.o w32proc.o w32reg.o w32select.o w32term.o w32image.o w32xfns.o \
   w16select.o widget.o xfont.o ftfont.o xftfont.o gtkutil.o \
   xsettings.o xgselect.o termcap.o hbfont.o
 
diff --git a/src/image.c b/src/image.c
index 65d59254f0..b0af991af7 100644
--- a/src/image.c
+++ b/src/image.c
@@ -18,6 +18,12 @@ Copyright (C) 1989, 1992-2020 Free Software Foundation, Inc.
 along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 
 #include <config.h>
+#ifdef HAVE_GDIPLUS
+#undef HAVE_JPEG
+#undef HAVE_PNG
+#undef HAVE_GIF
+#undef HAVE_TIFF
+#endif
 
 #include <fcntl.h>
 #include <unistd.h>
@@ -6235,7 +6241,7 @@ pbm_load (struct frame *f, struct image *img)
 				 PNG
  ***********************************************************************/
 
-#if defined (HAVE_PNG) || defined (HAVE_NS)
+#if defined (HAVE_PNG) || defined (HAVE_NS) || defined (HAVE_GDIPLUS)
 
 /* Indices of image specification fields in png_format, below.  */
 
@@ -6289,7 +6295,7 @@ png_image_p (Lisp_Object object)
 #endif /* HAVE_PNG || HAVE_NS */
 
 
-#if defined HAVE_PNG && !defined HAVE_NS
+#if defined HAVE_PNG && !defined HAVE_NS && !defined HAVE_GDIPLUS
 
 # ifdef WINDOWSNT
 /* PNG library details.  */
@@ -6889,8 +6895,19 @@ png_load (struct frame *f, struct image *img)
                         image_spec_value (img->spec, QCdata, NULL));
 }
 
+#elif defined HAVE_GDIPLUS
 
-#endif /* HAVE_NS */
+static bool
+png_load (struct frame *f, struct image *img)
+{
+  return w32_load_image (f, img,
+                         image_spec_value (img->spec, QCfile, NULL),
+                         image_spec_value (img->spec, QCdata, NULL));
+}
+
+#define init_png_functions w32_gdiplus_startup
+
+#endif /* HAVE_GDIPLUS */
 
 
 \f
@@ -6898,7 +6915,7 @@ png_load (struct frame *f, struct image *img)
 				 JPEG
  ***********************************************************************/
 
-#if defined (HAVE_JPEG) || defined (HAVE_NS)
+#if defined (HAVE_JPEG) || defined (HAVE_NS) || defined (HAVE_GDIPLUS)
 
 /* Indices of image specification fields in gs_format, below.  */
 
@@ -6950,7 +6967,7 @@ jpeg_image_p (Lisp_Object object)
   return fmt[JPEG_FILE].count + fmt[JPEG_DATA].count == 1;
 }
 
-#endif /* HAVE_JPEG || HAVE_NS */
+#endif /* HAVE_JPEG || HAVE_NS || HAVE_GDIPLUS */
 
 #ifdef HAVE_JPEG
 
@@ -7464,6 +7481,19 @@ jpeg_load (struct frame *f, struct image *img)
 }
 #endif  /* HAVE_NS */
 
+#ifdef HAVE_GDIPLUS
+static bool
+jpeg_load (struct frame *f, struct image *img)
+{
+  return w32_load_image (f, img,
+                         image_spec_value (img->spec, QCfile, NULL),
+                         image_spec_value (img->spec, QCdata, NULL));
+}
+
+#define init_jpeg_functions w32_gdiplus_startup
+#endif  /* HAVE_GDIPLUS */
+
+
 #endif /* !HAVE_JPEG */
 
 
@@ -7472,7 +7502,7 @@ jpeg_load (struct frame *f, struct image *img)
 				 TIFF
  ***********************************************************************/
 
-#if defined (HAVE_TIFF) || defined (HAVE_NS)
+#if defined (HAVE_TIFF) || defined (HAVE_NS) || defined (HAVE_GDIPLUS)
 
 /* Indices of image specification fields in tiff_format, below.  */
 
@@ -7525,7 +7555,7 @@ tiff_image_p (Lisp_Object object)
   return fmt[TIFF_FILE].count + fmt[TIFF_DATA].count == 1;
 }
 
-#endif /* HAVE_TIFF || HAVE_NS */
+#endif /* HAVE_TIFF || HAVE_NS || HAVE_GDIPLUS */
 
 #ifdef HAVE_TIFF
 
@@ -7903,6 +7933,18 @@ tiff_load (struct frame *f, struct image *img)
                         image_spec_value (img->spec, QCdata, NULL));
 }
 
+#elif defined HAVE_GDIPLUS
+
+static bool
+tiff_load (struct frame *f, struct image *img)
+{
+  return w32_load_image (f, img,
+                         image_spec_value (img->spec, QCfile, NULL),
+                         image_spec_value (img->spec, QCdata, NULL));
+}
+
+#define init_tiff_functions w32_gdiplus_startup
+
 #endif
 
 
@@ -7911,7 +7953,7 @@ tiff_load (struct frame *f, struct image *img)
 				 GIF
  ***********************************************************************/
 
-#if defined (HAVE_GIF) || defined (HAVE_NS)
+#if defined (HAVE_GIF) || defined (HAVE_NS) || defined (HAVE_GDIPLUS)
 
 /* Indices of image specification fields in gif_format, below.  */
 
@@ -7973,7 +8015,7 @@ gif_image_p (Lisp_Object object)
   return fmt[GIF_FILE].count + fmt[GIF_DATA].count == 1;
 }
 
-#endif /* HAVE_GIF */
+#endif /* HAVE_GIF || HAVE_NS || HAVE_GDIPLUS */
 
 #ifdef HAVE_GIF
 
@@ -8502,6 +8544,17 @@ gif_load (struct frame *f, struct image *img)
 }
 #endif /* HAVE_NS */
 
+#ifdef HAVE_NTGUI
+static bool
+gif_load (struct frame *f, struct image *img)
+{
+  return w32_load_image (f, img,
+                         image_spec_value (img->spec, QCfile, NULL),
+                         image_spec_value (img->spec, QCdata, NULL));
+}
+#define init_gif_functions w32_gdiplus_startup
+#endif /* HAVE_NTGUI */
+
 #endif /* HAVE_GIF */
 
 
@@ -10164,19 +10217,19 @@ initialize_image_type (struct image_type const *type)
  { SYMBOL_INDEX (Qsvg), svg_image_p, svg_load, image_clear_image,
    IMAGE_TYPE_INIT (init_svg_functions) },
 #endif
-#if defined HAVE_PNG || defined HAVE_NS
+#if defined HAVE_PNG || defined HAVE_NS || defined HAVE_GDIPLUS
  { SYMBOL_INDEX (Qpng), png_image_p, png_load, image_clear_image,
    IMAGE_TYPE_INIT (init_png_functions) },
 #endif
-#if defined HAVE_GIF || defined HAVE_NS
+#if defined HAVE_GIF || defined HAVE_NS || defined HAVE_GDIPLUS
  { SYMBOL_INDEX (Qgif), gif_image_p, gif_load, gif_clear_image,
    IMAGE_TYPE_INIT (init_gif_functions) },
 #endif
-#if defined HAVE_TIFF || defined HAVE_NS
+#if defined HAVE_TIFF || defined HAVE_NS || defined HAVE_GDIPLUS
  { SYMBOL_INDEX (Qtiff), tiff_image_p, tiff_load, image_clear_image,
    IMAGE_TYPE_INIT (init_tiff_functions) },
 #endif
-#if defined HAVE_JPEG || defined HAVE_NS
+#if defined HAVE_JPEG || defined HAVE_NS || defined HAVE_GDIPLUS
  { SYMBOL_INDEX (Qjpeg), jpeg_image_p, jpeg_load, image_clear_image,
    IMAGE_TYPE_INIT (init_jpeg_functions) },
 #endif
@@ -10315,22 +10368,22 @@ syms_of_image (void)
   add_image_type (Qxpm);
 #endif
 
-#if defined (HAVE_JPEG) || defined (HAVE_NS)
+#if defined (HAVE_JPEG) || defined (HAVE_NS) || defined (HAVE_GDIPLUS)
   DEFSYM (Qjpeg, "jpeg");
   add_image_type (Qjpeg);
 #endif
 
-#if defined (HAVE_TIFF) || defined (HAVE_NS)
+#if defined (HAVE_TIFF) || defined (HAVE_NS) || defined (HAVE_GDIPLUS)
   DEFSYM (Qtiff, "tiff");
   add_image_type (Qtiff);
 #endif
 
-#if defined (HAVE_GIF) || defined (HAVE_NS)
+#if defined (HAVE_GIF) || defined (HAVE_NS) || defined (HAVE_GDIPLUS)
   DEFSYM (Qgif, "gif");
   add_image_type (Qgif);
 #endif
 
-#if defined (HAVE_PNG) || defined (HAVE_NS)
+#if defined (HAVE_PNG) || defined (HAVE_NS) || defined(HAVE_GDIPLUS)
   DEFSYM (Qpng, "png");
   add_image_type (Qpng);
 #endif
diff --git a/src/w32.c b/src/w32.c
index 698e10e234..1d2a52b6df 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -10225,6 +10225,10 @@ term_ntproc (int ignored)
   term_winsock ();
 
   term_w32select ();
+
+#ifdef HAVE_GDIPLUS
+  w32_gdiplus_shutdown ();
+#endif
 }
 
 void
diff --git a/src/w32image.c b/src/w32image.c
new file mode 100644
index 0000000000..e8469b9c94
--- /dev/null
+++ b/src/w32image.c
@@ -0,0 +1,275 @@
+/* Implementation of GUI terminal on the Microsoft Windows API.
+
+Copyright (C) 1989, 1993-2020 Free Software Foundation, Inc.
+
+This file is part of GNU Emacs.
+
+GNU Emacs is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or (at
+your option) any later version.
+
+GNU Emacs is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
+
+#include <config.h>
+#include "lisp.h"
+#include "dispextern.h"
+#define COBJMACROS
+#include <objidl.h>
+#include <wtypes.h>
+#include <gdiplus.h>
+#include <shlwapi.h>
+#include "w32term.h"
+#include "frame.h"
+#include "coding.h"
+
+
+#if 0
+/*
+ * The following code is not really needed, but it provides an illustration on
+ * how to probe the list of available codecs.
+ */
+
+static void
+print_wchar(FILE *stream, WCHAR *string)
+{
+  for ( ; *string; ++string) {
+    fputc (*string, stream);
+  }
+}
+
+static void
+w32_list_decoders (void)
+{
+  UINT ndecoders, size;
+  ImageCodecInfo *pCodec;
+  int i;
+
+  GdipGetImageDecodersSize(&ndecoders, &size);
+  pCodec = (ImageCodecInfo *)malloc(size);
+
+  GdipGetImageDecoders(ndecoders, size, pCodec);
+  for (i = 0; i < ndecoders; ++i)
+    {
+      fprintf(stderr, "\nCodec name: "); print_wchar (stderr, pCodec[i].CodecName);
+      fprintf(stderr, "\n    format: "); print_wchar (stderr, pCodec[i].FormatDescription);
+      fprintf(stderr, "\n       ext: "); print_wchar (stderr, pCodec[i].FilenameExtension);
+      fprintf(stderr, "\n      mime: "); print_wchar (stderr, pCodec[i].MimeType);
+    }
+  fprintf(stderr, "\n");
+
+  free(pCodec);
+}
+#endif /* Unused */
+
+static int gdip_initialized = 0;
+static ULONG_PTR token;
+static GdiplusStartupInput input;
+static GdiplusStartupOutput output;
+
+bool
+w32_gdiplus_startup (void)
+{
+  GpStatus status;
+
+  if (gdip_initialized < 0)
+      return 0;
+  else if (gdip_initialized)
+      return 1;
+  else
+    {
+      input.GdiplusVersion = 1;
+      input.DebugEventCallback = NULL;
+      input.SuppressBackgroundThread = FALSE;
+      input.SuppressExternalCodecs = FALSE;
+
+      status = GdiplusStartup(&token, &input, &output);
+      if (status == Ok)
+        {
+          gdip_initialized = 1;
+          return 1;
+        }
+      else
+        {
+          gdip_initialized = -1;
+          return 0;
+        }
+    }
+}
+
+void
+w32_gdiplus_shutdown (void)
+{
+  GdiplusShutdown(token);
+}
+
+
+static double
+w32_frame_delay (GpBitmap *pBitmap, int frame)
+{
+  UINT size;
+  PropertyItem *propertyItem;
+  double delay = 0.0;
+
+  /* Assume that the image has a property item of type PropertyItemEquipMake.
+     Get the size of that property item. */
+  GdipGetPropertyItemSize(pBitmap, PropertyTagFrameDelay, &size);
+
+  /* Allocate a buffer to receive the property item.  */
+  propertyItem = (PropertyItem*)malloc(size);
+  if (propertyItem != NULL)
+    {
+      /* Get the property item.  */
+      GdipGetPropertyItem(pBitmap, PropertyTagFrameDelay, size, propertyItem);
+      delay = ((double)propertyItem[frame].length) / 100;
+      if (delay == 0)
+        {
+          /* In GIF files, unfortunately, delay is only specified for the first
+             frame.  */
+          delay = ((double)propertyItem[0].length) / 100;
+        }
+      free(propertyItem);
+    }
+  return delay;
+}
+
+static UINT
+w32_select_active_frame(GpBitmap *pBitmap, int frame, int *nframes, double *delay)
+{
+  UINT count, frameCount;
+  GUID pDimensionIDs[1];
+  GpStatus status = Ok;
+
+  status = GdipImageGetFrameDimensionsCount(pBitmap, &count);
+  frameCount = *nframes = 0;
+  *delay = 0.0;
+  if (count)
+    {
+      status = GdipImageGetFrameDimensionsList (pBitmap, pDimensionIDs, 1);
+      status = GdipImageGetFrameCount (pBitmap, &pDimensionIDs[0], &frameCount);
+      if ((status == Ok) && (frameCount > 1))
+        {
+          if (frame < 0 || frame >= frameCount)
+            {
+              status = GenericError;
+            }
+          else
+            {
+              status = GdipImageSelectActiveFrame (pBitmap, &pDimensionIDs[0], frame);
+              *delay = w32_frame_delay (pBitmap, frame);
+              *nframes = frameCount;
+            }
+        }
+    }
+  return status;
+}
+
+static ARGB
+w32_image_bg_color(struct frame *f, struct image *img)
+{
+  /* png_color_16 *image_bg; */
+  Lisp_Object specified_bg
+    = Fplist_get (XCDR (img->spec), QCbackground);
+  Emacs_Color color;
+
+  /* If the user specified a color, try to use it; if not, use the
+     current frame background, ignoring any default background
+     color set by the image.  */
+  if (STRINGP (specified_bg)
+      ? FRAME_TERMINAL (f)->defined_color_hook (f,
+                                                SSDATA (specified_bg),
+                                                &color,
+                                                false,
+                                                false)
+      : (FRAME_TERMINAL (f)->query_frame_background_color (f, &color),
+         true))
+    /* The user specified `:background', use that.  */
+    {
+      DWORD red = (((DWORD) color.red) & 0xff00) << 8;
+      DWORD green = ((DWORD) color.green) & 0xff00;
+      DWORD blue = ((DWORD) color.blue) >> 8;
+      return red | green | blue;
+    }
+  return ((DWORD) 0xff000000);
+}
+
+int
+w32_load_image (struct frame *f, struct image *img,
+                Lisp_Object spec_file, Lisp_Object spec_data)
+{
+  Emacs_Pixmap pixmap;
+  GpStatus status = GenericError;
+  GpBitmap *pBitmap;
+  wchar_t filename[MAX_PATH];
+  ARGB bg_color;
+  Lisp_Object lisp_index, metadata;
+  unsigned int index, nframes;
+  double delay;
+
+  eassert (valid_image_p (img->spec));
+
+  /* This function only gets called if init_w32_gdiplus () was invoked. We have
+     a valid token and GDI+ is active.  */
+  if (STRINGP (spec_file))
+    {
+      filename_to_utf16 (SSDATA (spec_file) , filename);
+      status = GdipCreateBitmapFromFile (filename, &pBitmap);
+    }
+  else if (STRINGP (spec_data))
+    {
+      IStream *pStream = SHCreateMemStream ((BYTE *) SSDATA (spec_data),
+                                            SBYTES (spec_data));
+      if (pStream != NULL)
+        {
+          status = GdipCreateBitmapFromStream (pStream, &pBitmap);
+          IStream_Release (pStream);
+        }
+    }
+
+  metadata = Qnil;
+  if (status == Ok)
+    {
+      /* In multiframe pictures, select the first one */
+      lisp_index = Fplist_get (XCDR (img->spec), QCindex);
+      index = FIXNUMP (lisp_index) ? XFIXNAT (lisp_index) : 0;
+      status = w32_select_active_frame (pBitmap, index, &nframes, &delay);
+      if ((status == Ok))
+        {
+          if (nframes > 1)
+            metadata = Fcons (Qcount, Fcons (make_fixnum (nframes), metadata));
+          if (delay)
+            metadata = Fcons (Qdelay, Fcons (make_float (delay), metadata));
+        }
+    }
+
+  if (status == Ok)
+    {
+      bg_color = w32_image_bg_color(f, img);
+      status = GdipCreateHBITMAPFromBitmap (pBitmap, &pixmap, bg_color);
+      if (status == Ok)
+        {
+          UINT width, height;
+          GdipGetImageWidth (pBitmap, &width);
+          GdipGetImageHeight (pBitmap, &height);
+          img->width = width;
+          img->height = height;
+          img->pixmap = pixmap;
+          img->lisp_data = metadata;
+        }
+
+      GdipDisposeImage (pBitmap);
+    }
+
+  if (status != Ok)
+    {
+      add_to_log ("Unable to load image %s", img->spec);
+      return 0;
+    }
+  return 1;
+}
diff --git a/src/w32term.h b/src/w32term.h
index f8a8a727e8..365d752767 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -75,7 +75,10 @@ #define CP_DEFAULT 1004
 extern void w32_regenerate_palette (struct frame *f);
 extern void w32_fullscreen_rect (HWND hwnd, int fsmode, RECT normal,
                                  RECT *rect);
-
+extern int w32_load_image (struct frame *f, struct image *img,
+                           Lisp_Object spec_file, Lisp_Object spec_data);
+extern bool w32_gdiplus_startup (void);
+extern void w32_gdiplus_shutdown (void);
 \f
 /* For each display (currently only one on w32), we have a structure that
    records information about it.  */

Re: Optional support for GDI+ on Windows (emacs-28)

[Eli Zaretskii]

> From: Juan José García-Ripoll
>  <juanjose.garciaripoll@gmail.com>
> Date: Mon, 30 Mar 2020 21:26:52 +0200
> 
> Here is a more canonical patch for the Windows port:

Thanks.  Was this version supposed to take care of the review
comments to the previous one?  If so, perhaps you sent the wrong
patch, because it looks like all the issues are still there in this
version.

What am I missing?

Re: Optional support for GDI+ on Windows (emacs-28)

[Juan José García-Ripoll]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Juan José García-Ripoll
>>  <juanjose.garciaripoll@gmail.com>
>> Date: Mon, 30 Mar 2020 21:26:52 +0200
>> 
>> Here is a more canonical patch for the Windows port:
>
> Thanks.  Was this version supposed to take care of the review
> comments to the previous one?  If so, perhaps you sent the wrong
> patch, because it looks like all the issues are still there in this
> version.

Regarding your comments:

* I do not know what is supposed to replace the code regarding search
  for terminal background colors. If this code is wrong, it is also
  wrong in the code that uses the PNG library (src/image.c).
      if (STRINGP (specified_bg)
	  ? FRAME_TERMINAL (f)->defined_color_hook (f,
                                                    SSDATA (specified_bg),
                                                    &color,
                                                    false,
                                                    false)
	  : (FRAME_TERMINAL (f)->query_frame_background_color (f, &color),
             true))

* Regarding the use of WCHAR in filenames, because this component only
  works when GDI+ is linked in, w32_unicode_filenames is always 1 and
  there is no need for additional translations.

* Regarding the use of pointers to string data, once more, I am using
  the same code that is used in the image.c file to extract the image
  data from the user's input. If that code is wrong, then it is so in
  the PNG driver
        /* Read from memory.  */
      tbr.bytes = SDATA (specified_data);
      tbr.len = SBYTES (specified_data);
      tbr.index = 0;
  in the JPEG driver
      jpeg_memory_src (&mgr->cinfo, SDATA (specified_data),
		     SBYTES (specified_data));
  in the GIF driver, etc, etc.

* Regarding stylistic conventions, all have been fixed.

* Regarding the use of HAVE_NTGUI and unconditional removal of
  PNG/JPEG/etc, it has been replaced with a flag HAVE_GDIPLUS which is
  optionally selected at configuration time with --with-gdiplus, which
  defaults to NO.

* All declarations have been changed to ANSI C99

* The static variable has to be initialized to 0 because it explicitely
  describes a condition (library has not been initiated) that is
  meaningful.

* I replaced the use of float with double. You might want to do the same
  with nsimage.c if that is a problem.

* All fprintf() are removed except for a piece of code that is currently
  commented out, but which is needed to have a look at the installed decoders.

So, what exactly is missing?

-- 
Juan José García Ripoll
http://juanjose.garciaripoll.com
http://quinfog.hbar.es

Re: Optional support for GDI+ on Windows (emacs-28)

[Eli Zaretskii]

> From: Juan José García-Ripoll
>  <juanjose.garciaripoll@gmail.com>
> Date: Tue, 31 Mar 2020 17:35:30 +0200
> 
> > Thanks.  Was this version supposed to take care of the review
> > comments to the previous one?  If so, perhaps you sent the wrong
> > patch, because it looks like all the issues are still there in this
> > version.
> 
> Regarding your comments:
>  
> * I do not know what is supposed to replace the code regarding search
>   for terminal background colors. If this code is wrong, it is also
>   wrong in the code that uses the PNG library (src/image.c).
>       if (STRINGP (specified_bg)
> 	  ? FRAME_TERMINAL (f)->defined_color_hook (f,

This resolves to w32_defined_color.

>                                                     SSDATA (specified_bg),
>                                                     &color,
>                                                     false,
>                                                     false)
> 	  : (FRAME_TERMINAL (f)->query_frame_background_color (f, &color),
>              true))

And this resolves to w32_query_frame_background_color.  See w32term.c
around line 7250.  Like I said, image.c is a largely
platform-independent code, whereas w32image.c isn't, so calling
w32-specific functions directly is OK.  But this is not an important
comment, so if you want commonality with image.c, I won't object.
It's just slightly sub-optimal, because each call needs to dereference
a function pointer.

> * Regarding the use of WCHAR in filenames, because this component only
>   works when GDI+ is linked in, w32_unicode_filenames is always 1 and
>   there is no need for additional translations.

w32_unicode_filenames is exposed to Lisp and can be set to nil at
runtime.  Only its default value is guaranteed to be non-nil on modern
Windows platforms.  My question was what to do about that, since in
general we aren't supposed to use Unicode file names when that
variable is nil.

Btw, I just now spotted a more serious problem with w32_load_image: it
should encode the file name before it calls filename_to_utf16.  (The
same problem exists in ns_load_image, AFAICT.)  You will see that
png_load_body, for example, encodes the file name it receives from the
caller, before using it in libpng calls.  In general, we cannot pass
the internal Emacs representation of file-name strings to system APIs.

> * Regarding the use of pointers to string data, once more, I am using
>   the same code that is used in the image.c file to extract the image
>   data from the user's input. If that code is wrong, then it is so in
>   the PNG driver
>         /* Read from memory.  */
>       tbr.bytes = SDATA (specified_data);
>       tbr.len = SBYTES (specified_data);
>       tbr.index = 0;
>   in the JPEG driver
>       jpeg_memory_src (&mgr->cinfo, SDATA (specified_data),
> 		     SBYTES (specified_data));
>   in the GIF driver, etc, etc.

You are missing my point, but it isn't worth arguing about it, so
let's drop this part.

> * Regarding stylistic conventions, all have been fixed.

Not all: some spaces are still missing between the function name and
the opening parenthesis.  One example:

      status = GdiplusStartup(&token, &input, &output);
                            ^^

Also, some comments still don't leave 2 spaces after the final
period.  Example:

   /* Assume that the image has a property item of type PropertyItemEquipMake.
      Get the size of that property item. */
                                        ^^

And there are still one-line blocks in braces.  Example:

          if (frame < 0 || frame >= frameCount)
            {
              status = GenericError;
            }
          else

> * Regarding the use of HAVE_NTGUI and unconditional removal of
>   PNG/JPEG/etc, it has been replaced with a flag HAVE_GDIPLUS which is
>   optionally selected at configuration time with --with-gdiplus, which
>   defaults to NO.

Like I said: this must not be a compile-time condition, we should
decide whether GDI+ is supported at runtime, and we should provide
variables to control whether GDI+ is used for each supported image
format.  HAVE_GDIPLUS should guard code which uses GDI+, but it should
NOT decide whether that code is actually used.

Btw, the same issue is with SHCreateMemStream -- it should be called
through a function pointer that gets populated at runtime after
loading shlwapi.dll, and for the same reason: the function is not
available on all the versions we want to support.

> * The static variable has to be initialized to 0 because it explicitely
>   describes a condition (library has not been initiated) that is
>   meaningful.

Static variables are automatically initialized to zero, you don't need
to do that explicitly (this is a minor nit).

> So, what exactly is missing?

The main issue remains the compile-time decision whether to use GDI+
in your patch, as opposed to run-time decision, and the related
variables, that I'd like to see.  I don't know who told you something
that could be interpreted in the other direction; if that was
something I wrote, please accept my sincere apologies -- I never meant
anything to that effect.

Thanks.

Re: Optional support for GDI+ on Windows (emacs-28)

[Alan Third]

On Tue, Mar 31, 2020 at 07:47:38PM +0300, Eli Zaretskii wrote:
> 
> Like I said: this must not be a compile-time condition, we should
> decide whether GDI+ is supported at runtime, and we should provide
> variables to control whether GDI+ is used for each supported image
> format.  HAVE_GDIPLUS should guard code which uses GDI+, but it should
> NOT decide whether that code is actually used.

May I request that however the above is implemented, it can be
extended to the NS port’s use of NSImage vs libpng, etc. as well?

-- 
Alan Third

Re: Optional support for GDI+ on Windows (emacs-28)

[Eli Zaretskii]

> Date: Tue, 31 Mar 2020 18:57:52 +0200 (CEST)
> From: Alan Third <alan@idiocy.org>
> Cc: Juan José García-Ripoll <juanjose.garciaripoll@gmail.com>,
> 	emacs-devel@gnu.org
> 
> On Tue, Mar 31, 2020 at 07:47:38PM +0300, Eli Zaretskii wrote:
> > 
> > Like I said: this must not be a compile-time condition, we should
> > decide whether GDI+ is supported at runtime, and we should provide
> > variables to control whether GDI+ is used for each supported image
> > format.  HAVE_GDIPLUS should guard code which uses GDI+, but it should
> > NOT decide whether that code is actually used.
> 
> May I request that however the above is implemented, it can be
> extended to the NS port’s use of NSImage vs libpng, etc. as well?

Can you elaborate?  I don't think I understand the request, probably
because I know very little about how NS deals with images in Emacs.

Thanks.

Re: Optional support for GDI+ on Windows (emacs-28)

[Alan Third]

On Tue, Mar 31, 2020 at 08:41:29PM +0300, Eli Zaretskii wrote:
> > Date: Tue, 31 Mar 2020 18:57:52 +0200 (CEST)
> > From: Alan Third <alan@idiocy.org>
> > Cc: Juan José García-Ripoll <juanjose.garciaripoll@gmail.com>,
> > 	emacs-devel@gnu.org
> > 
> > On Tue, Mar 31, 2020 at 07:47:38PM +0300, Eli Zaretskii wrote:
> > > 
> > > Like I said: this must not be a compile-time condition, we should
> > > decide whether GDI+ is supported at runtime, and we should provide
> > > variables to control whether GDI+ is used for each supported image
> > > format.  HAVE_GDIPLUS should guard code which uses GDI+, but it should
> > > NOT decide whether that code is actually used.
> > 
> > May I request that however the above is implemented, it can be
> > extended to the NS port’s use of NSImage vs libpng, etc. as well?
> 
> Can you elaborate?  I don't think I understand the request, probably
> because I know very little about how NS deals with images in Emacs.

Simply that since the proposed patch works largely the same as the NS
port, if possible leave any implementation generic. That is, rather
than create a variable called w32-png-use-gdi-plus, perhaps name it
something like image–png-use-native.

If it’s not possible then that’s fine.
-- 
Alan Third

Re: Optional support for GDI+ on Windows (emacs-28)

[Stefan Monnier]

>> Can you elaborate?  I don't think I understand the request, probably
>> because I know very little about how NS deals with images in Emacs.
> Simply that since the proposed patch works largely the same as the NS
> port, if possible leave any implementation generic.

I agree that it would be good to keep the two codes as similar as possible.

> That is, rather than create a variable called w32-png-use-gdi-plus,
> perhaps name it something like image–png-use-native.

YAGNI


        Stefan

Re: Optional support for GDI+ on Windows (emacs-28)

[Eli Zaretskii]

> Date: Tue, 31 Mar 2020 21:36:06 +0200 (CEST)
> From: Alan Third <alan@idiocy.org>
> Cc: juanjose.garciaripoll@gmail.com, emacs-devel@gnu.org
> 
> > Can you elaborate?  I don't think I understand the request, probably
> > because I know very little about how NS deals with images in Emacs.
> 
> Simply that since the proposed patch works largely the same as the NS
> port, if possible leave any implementation generic. That is, rather
> than create a variable called w32-png-use-gdi-plus, perhaps name it
> something like image–png-use-native.

My idea is to define a new image type, and have a image_types[] entry
set up to call the native method.  Something like that, anyway.  Would
that be okay?

Re: Optional support for GDI+ on Windows (emacs-28)

[Alan Third]

On Wed, Apr 01, 2020 at 05:26:08AM +0300, Eli Zaretskii wrote:
> > Date: Tue, 31 Mar 2020 21:36:06 +0200 (CEST)
> > From: Alan Third <alan@idiocy.org>
> > Cc: juanjose.garciaripoll@gmail.com, emacs-devel@gnu.org
> > 
> > > Can you elaborate?  I don't think I understand the request, probably
> > > because I know very little about how NS deals with images in Emacs.
> > 
> > Simply that since the proposed patch works largely the same as the NS
> > port, if possible leave any implementation generic. That is, rather
> > than create a variable called w32-png-use-gdi-plus, perhaps name it
> > something like image–png-use-native.
> 
> My idea is to define a new image type, and have a image_types[] entry
> set up to call the native method.  Something like that, anyway.  Would
> that be okay?

Fine with me. I’d considered something similar for the NS port a while
back.

In actual fact what I’d wondered about was whether it would be
possible to delegate the image loading routine to a module, so it
would be possible to have, say, an imagemagick module, or a gd module,
or whatever and if someone wanted to use one they could just load it.
But I don’t know if modules would be suitable and I never investigated
it.

It would probably require significantly more upheaval than what we’re
talking about here.
-- 
Alan Third

Re: Optional support for GDI+ on Windows (emacs-28)

[Eli Zaretskii]

> Date: Wed, 1 Apr 2020 20:33:58 +0200 (CEST)
> From: Alan Third <alan@idiocy.org>
> Cc: juanjose.garciaripoll@gmail.com, emacs-devel@gnu.org
> 
> > My idea is to define a new image type, and have a image_types[] entry
> > set up to call the native method.  Something like that, anyway.  Would
> > that be okay?
> 
> Fine with me. I’d considered something similar for the NS port a while
> back.

Great, we will see how that plays off.

> In actual fact what I’d wondered about was whether it would be
> possible to delegate the image loading routine to a module, so it
> would be possible to have, say, an imagemagick module, or a gd module,
> or whatever and if someone wanted to use one they could just load it.

Some image types should still be built-in, since we display icons in
the UI.  Other than that, it could be an interesting idea, if it
works.

> But I don’t know if modules would be suitable and I never investigated
> it.

Neither did I.

Re: Optional support for GDI+ on Windows (emacs-28)

[Stefan Monnier]

> we should provide variables to control whether GDI+ is used for each
> supported image format.  HAVE_GDIPLUS should guard code which uses
> GDI+, but it should NOT decide whether that code is actually used.

Why bother?


        Stefan

Re: Optional support for GDI+ on Windows (emacs-28)

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Tue, 31 Mar 2020 13:27:19 -0400
> Cc: Juan José García-Ripoll
>  <juanjose.garciaripoll@gmail.com>, emacs-devel@gnu.org
> 
> > we should provide variables to control whether GDI+ is used for each
> > supported image format.  HAVE_GDIPLUS should guard code which uses
> > GDI+, but it should NOT decide whether that code is actually used.
> 
> Why bother?

Because I requested it.

Re: Optional support for GDI+ on Windows (emacs-28)

[Stefan Monnier]

>> > we should provide variables to control whether GDI+ is used for each
>> > supported image format.  HAVE_GDIPLUS should guard code which uses
>> > GDI+, but it should NOT decide whether that code is actually used.
>> Why bother?
> Because I requested it.

I understand this part, but really there's a good chance this is
wasted effort.  The NS port didn't have to jump through those hoops to
get its code accepted.


        Stefan

Re: Optional support for GDI+ on Windows (emacs-28)

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Tue, 31 Mar 2020 15:37:18 -0400
> Cc: juanjose.garciaripoll@gmail.com, emacs-devel@gnu.org
> 
> >> > we should provide variables to control whether GDI+ is used for each
> >> > supported image format.  HAVE_GDIPLUS should guard code which uses
> >> > GDI+, but it should NOT decide whether that code is actually used.
> >> Why bother?
> > Because I requested it.
> 
> I understand this part, but really there's a good chance this is
> wasted effort.  The NS port didn't have to jump through those hoops to
> get its code accepted.

I didn't request anything that could be qualified as jumping through
hoops.  This is our standard implementation paradigm for optional
features on MS-Windows, you can see it all over the place in w32*.c
files.  Convenience macros are available to make coding this
more-or-less copy-pasting from some other similar feature.

Re: Optional support for GDI+ on Windows (emacs-28)

[Stefan Monnier]

>> >> > we should provide variables to control whether GDI+ is used for each
>> >> > supported image format.  HAVE_GDIPLUS should guard code which uses
>> >> > GDI+, but it should NOT decide whether that code is actually used.
>> >> Why bother?
>> > Because I requested it.
>> 
>> I understand this part, but really there's a good chance this is
>> wasted effort.  The NS port didn't have to jump through those hoops to
>> get its code accepted.
>
> I didn't request anything that could be qualified as jumping through
> hoops.  This is our standard implementation paradigm for optional
> features on MS-Windows, you can see it all over the place in w32*.c
> files.  Convenience macros are available to make coding this
> more-or-less copy-pasting from some other similar feature.

I don't think "variables to control whether <foo> is used for each
supported <bar>" is comparable to what we do for other
"optional" features on MS-Windows.

Also, for "optional features on MS-Windows", the usual behavior if the
lib is not available at run-time is just not to provide the
corresponding functionality, rather than to fallback on some alternative
C implementation.

IOW, I think what would be comparable is: build with support for GDI+
images only (i.e. without support for images via libpng and friends),
and make sure Emacs does run even if GDI+ is not available (simply with
the limitation that it can't display those images, just like what we
have now if libpng is not available at run time).


        Stefan

Re: Optional support for GDI+ on Windows (emacs-28)

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: juanjose.garciaripoll@gmail.com,  emacs-devel@gnu.org
> Date: Wed, 01 Apr 2020 10:58:29 -0400
> 
> > I didn't request anything that could be qualified as jumping through
> > hoops.  This is our standard implementation paradigm for optional
> > features on MS-Windows, you can see it all over the place in w32*.c
> > files.  Convenience macros are available to make coding this
> > more-or-less copy-pasting from some other similar feature.
> 
> I don't think "variables to control whether <foo> is used for each
> supported <bar>" is comparable to what we do for other
> "optional" features on MS-Windows.

Actually, it is comparable.  See w32-unicode-filenames, as one
example.  Another example is the font-backend frame parameter, which
controls what shaping engine will be loaded and used.

> Also, for "optional features on MS-Windows", the usual behavior if the
> lib is not available at run-time is just not to provide the
> corresponding functionality, rather than to fallback on some alternative
> C implementation.

In most cases, but not all of them.  See above.

> IOW, I think what would be comparable is: build with support for GDI+
> images only (i.e. without support for images via libpng and friends),
> and make sure Emacs does run even if GDI+ is not available (simply with
> the limitation that it can't display those images, just like what we
> have now if libpng is not available at run time).

I don't think it is correct for us to commit ourselves to GDI+ right
now.  We should first see if it is a 100% capable replacement, and
learn about its advantages and disadvantages.  We always do that with
new optional features: they start disabled.

In this case, we should also consider whether GDI+ will be supported
long enough before MS comes up with the next buzzword and decides to
deprecate GDI+ (something that happened with Uniscribe, for example).
This dependence on MS whims is one significant disadvantage of using
the native capabilities where ports from Posix are available as
replacements.

So, by and large, I don't think it's reasonable to rush to GDI+
without collecting experience first, and having those optional knobs
is necessary for that.

Re: Optional support for GDI+ on Windows (emacs-28)

[Stefan Monnier]

> In most cases, but not all of them.  See above.

My point still holds: what you're asking for is not the "standard
implementation paradigm for optional features on MS-Windows".

> I don't think it is correct for us to commit ourselves to GDI+ right
> now.  We should first see if it is a 100% capable replacement, and
> learn about its advantages and disadvantages.  We always do that with
> new optional features: they start disabled.

His patch doesn't commit to the new code, and it is disabled
by default (it requires `--with-gdiplus`), so it seems to satisfy this
constraints just fine.

> This dependence on MS whims is one significant disadvantage of using
> the native capabilities where ports from Posix are available as
> replacements.

Obviously, I look at this from a very different angle: I'm hoping we can
eventually throw away most of our image handling code and just rely on
native libraries instead (tho I don't know a thing about what
a corresponding native library would be for X11/Cairo).

> So, by and large, I don't think it's reasonable to rush to GDI+
> without collecting experience first, and having those optional knobs
> is necessary for that.

A compile-time option seems plenty to get that experience.
It's what we've had for Cairo.


        Stefan

Re: Optional support for GDI+ on Windows (emacs-28)

[Juanma Barranquero]

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

On Wed, Apr 1, 2020 at 8:27 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

 > A compile-time option seems plenty to get that experience.

I think most users of Emacs on Windows do not build their own Emacs.
So, if we want GDI+ support to be tested so we can trust it, we would
need to "distribute" (as we do know, non-officially) two different binaries.
Which means that a user that knows nothing about the current image
libraries *or* GDI+ has to decide beforehand and without enough
information; or alternatively we would need to "push" the GDI+ build as
preferable, which would be weird for a new feature that can potentially
cause problems.

[-- Attachment #2: Type: text/html, Size: 802 bytes --]

Re: Optional support for GDI+ on Windows (emacs-28)

[Stefan Monnier]

>  > A compile-time option seems plenty to get that experience.
> I think most users of Emacs on Windows do not build their own Emacs.
> So, if we want GDI+ support to be tested so we can trust it, we would
> need to "distribute" (as we do know, non-officially) two different binaries.

I'd expect we'd distribute the development version built from `master`
`--with-gdiplus` so we get feedback about it before we start the
pretest period of Emacs-28.


        Stefan

Re: Optional support for GDI+ on Windows (emacs-28)

[Juanma Barranquero]

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

On Wed, Apr 1, 2020 at 9:22 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> I'd expect we'd distribute the development version built from `master`
> `--with-gdiplus` so we get feedback about it before we start the
> pretest period of Emacs-28.

I'd be very surprised if that's testing enough to enable the feature for
Emacs 28, so we're back at distributing two bulds of the 28 release(s).

[-- Attachment #2: Type: text/html, Size: 549 bytes --]

Re: Optional support for GDI+ on Windows (emacs-28)

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: juanjose.garciaripoll@gmail.com,  emacs-devel@gnu.org
> Date: Wed, 01 Apr 2020 14:26:25 -0400
> 
> > In most cases, but not all of them.  See above.
> 
> My point still holds: what you're asking for is not the "standard
> implementation paradigm for optional features on MS-Windows".

Yes, it is.

And since we've got to the "he said, she said" part, let's drop this.

> A compile-time option seems plenty to get that experience.

Not on MS-Windows, it isn't.

Re: Optional support for GDI+ on Windows (emacs-28)

[Angelo Graziosi]

Juan José García-Ripoll wrote:
> Here is a more canonical patch for the Windows port:

> When emacs is configured with --with-gdiplus and it is built on 
> Mingw, --with-jpeg, --with-png, --with-tiff and --with-gif are 
> disabled and those image formats are implemented using Windows' GDI+ 
>  library.

I have built Emacs with this patch and it builds and works.. Maybe you can create a branch on which apply your ideas. Also the PNG icons looks  interesting.

Ciao,
 Angelo.

Re: Optional support for GDI+ on Windows (emacs-28)

[Juan José García-Ripoll]

Angelo Graziosi <angelo.g0@libero.it> writes:

> Juan José García-Ripoll wrote:
>> Here is a more canonical patch for the Windows port:
>
>> When emacs is configured with --with-gdiplus and it is built on 
>> Mingw, --with-jpeg, --with-png, --with-tiff and --with-gif are 
>> disabled and those image formats are implemented using Windows' GDI+ 
>>  library.
>
> I have built Emacs with this patch and it builds and works.. Maybe you
> can create a branch on which apply your ideas. Also the PNG icons
> looks interesting.

I have no permissions into the repository and I very much not trust
myself with it. I am developing all this locally in my computer.

Cheers,

-- 
Juan José García Ripoll
http://juanjose.garciaripoll.com
http://quinfog.hbar.es