Interest in nt_load_image?

Interest in nt_load_image?

[Juan José García-Ripoll]

Having a look at the sources, Emacs on OSX already supports native
loading of images via ns_load_image().

I believe something similar can be done for Windows using GDI+, removing
the dependency on libpng, libjpeg and libtiff. Would that be ok or
interesting? Has anyone tried it before?

Cheers,

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

Re: Interest in nt_load_image?

[Juan José García-Ripoll]

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

Juan José García-Ripoll <juanjose.garciaripoll@gmail.com> writes:
> I believe something similar can be done for Windows using GDI+, removing
> the dependency on libpng, libjpeg and libtiff. Would that be ok or
> interesting? Has anyone tried it before?

The capture attached shows it works with jpeg :-)

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

[-- Attachment #2: jpeg-on-emacs-with-gdi+.png --]
[-- Type: image/png, Size: 53241 bytes --]

Re: Interest in nt_load_image?

[Eli Zaretskii]

> From: Juan José García-Ripoll
>  <juanjose.garciaripoll@gmail.com>
> Date: Sun, 29 Mar 2020 21:33:52 +0200
> 
> Having a look at the sources, Emacs on OSX already supports native
> loading of images via ns_load_image().
> 
> I believe something similar can be done for Windows using GDI+, removing
> the dependency on libpng, libjpeg and libtiff. Would that be ok or
> interesting? Has anyone tried it before?

Please point to the relevant APIs, to make the discussion more
practical.

Thanks.

Re: Interest in nt_load_image?

[Juan José García-Ripoll]

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

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Juan José García-Ripoll
>>  <juanjose.garciaripoll@gmail.com>
>> I believe something similar can be done for Windows using GDI+, removing
>> the dependency on libpng, libjpeg and libtiff. Would that be ok or
>> interesting? Has anyone tried it before?
>
> Please point to the relevant APIs, to make the discussion more
> practical.

GDI+ is an evolution of GDI that supports arbitrary plug-ins for image formats,
both bitmaps and vector type. It is a bit more modern than GDI, from what I
get, but, just as GDI, it is not the modern standard for Windows 2D
displays. Indeed, it is old enough that it is also supported by Windows XP.

https://docs.microsoft.com/en-us/windows/win32/gdiplus/-gdiplus-gdi-start

GDI+ has a flat C interface that allows loading images, querying properties,
displaying them and converting them to older GDI formats.

https://docs.microsoft.com/en-us/windows/win32/gdiplus/-gdiplus-flatapi-flat

This interface is included with the Mingw64/32 headers.

I propose to use this last feature. I attach a patch that works with Emacs 28
(and probably also Emacs 27). The way it works:

- At configuration time, it works just as the NextStep (NS) system, disabling
  the use of libpng, libjpeg, libtiff and libgif when the build system is
  Mingw.

- In images such as PNG, GIF or TIFF, it currently does not use a bitmask for
  display. Instead, it relies on GDI+'s convertion to HBITMAP, which allows
  alpha blending with any background color of choice.

- In the C code, it replaces the load_jpeg, load_gif, etc, with a generic
  w32_load_image() function in src/w32image.c. This function is heavily
  inspired by ns_load_image() in src/nsimage.c.

The patch is not intrusive at all, I believe. It does not aim to replace
Emacs's engine for displaying images and fonts on Windows. Instead, it just
uses GDI+ for the conversion.

The only thing that is missing is a place to call GdipShutdown(). I do not know
how to add an exit handler for Emacs' C core.

I have tested that the patch works with some stock JPEG, GIF, PNG and TIFF
images, including multipage and transparency.

I have also verified that it is possible to convert all *.xpm icons to *.png
format and thus eliminate the need to include libXpm-noX.dll. Plus, the size of
the icons is reduced by 50%

Cheers,

-- 
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: 18400 bytes --]

diff --git a/configure.ac b/configure.ac
index a4daf14..4384bc9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2132,6 +2132,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,9 +2161,11 @@ 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"
-    W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32"
+    AC_DEFINE(HAVE_GDIPLUS, 1, [Define to use MS Windows GDI+ for images.])
+    HAVE_GDIPLUS=yes
+    W32_OBJ="$W32_OBJ w32.o w32console.o w32heap.o w32inevt.o w32proc.o w32image.o"
+    W32_LIBS="$W32_LIBS -lwinmm -lusp10 -lgdiplus -lgdi32 -lcomdlg32"
+    W32_LIBS="$W32_LIBS -lmpr -lwinspool -lshlwapi -lole32 -lcomctl32"
     W32_RES_LINK="\$(EMACSRES)"
     CLIENTRES="emacsclient.res"
     CLIENTW="emacsclientw\$(EXEEXT)"
@@ -3572,8 +3575,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 +3726,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 +3799,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 +3829,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
diff --git a/src/Makefile.in b/src/Makefile.in
index 552dd2e..0dc6133 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 65d5925..7cdd85c 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_NTGUI)
 
 /* 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_NTGUI
 
 # 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 init_w32_image_load_functions
+
+#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,26 @@ 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));
+}
+
+static bool
+init_w32_image_load_functions (void)
+{
+  fprintf(stderr, "Functions initialized\n");
+  return 1;
+}
+
+#define init_jpeg_functions init_w32_image_load_functions
+#endif  /* HAVE_GDIPLUS */
+
+
 #endif /* !HAVE_JPEG */
 
 
@@ -7472,7 +7509,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 +7562,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 +7940,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 init_w32_image_load_functions
+
 #endif
 
 
@@ -7911,7 +7960,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_NTGUI)
 
 /* Indices of image specification fields in gif_format, below.  */
 
@@ -7973,7 +8022,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_NTGUI */
 
 #ifdef HAVE_GIF
 
@@ -8502,6 +8551,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 init_w32_image_load_functions
+#endif /* HAVE_NTGUI */
+
 #endif /* HAVE_GIF */
 
 
@@ -10164,19 +10224,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 +10375,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/w32image.c b/src/w32image.c
new file mode 100644
index 0000000..9292879
--- /dev/null
+++ b/src/w32image.c
@@ -0,0 +1,240 @@
+/* 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"
+
+static int
+gdiplus_initialized_p()
+{
+  static int gdip_initialized = 0;
+  static ULONG_PTR token;
+  static GdiplusStartupInput input;
+  static GdiplusStartupOutput output;
+  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;
+        }
+    }
+  return 1;
+}
+
+static float
+w32_frame_delay(GpBitmap *pBitmap, int frame)
+{
+
+   UINT size;
+   PropertyItem *propertyItem;
+   float 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 = ((float)propertyItem[frame].length) / 100;
+       if (delay == 0)
+         {
+           /* In GIF files, unfortunately, delay is only specified for
+              the first frame */
+           delay = ((float)propertyItem[0].length) / 100;
+         }
+       // Free space
+       free(propertyItem);
+     }
+   return delay;
+}
+
+static UINT
+w32_select_active_frame(GpBitmap *pBitmap, int frame, int *nframes, float *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);
+      fprintf(stderr, "FrameCount: %d\n", (int)frameCount);
+      fprintf(stderr, "     index: %d\n", frame);
+      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;
+  float delay;
+
+  eassert (valid_image_p (img->spec));
+
+  if (!gdiplus_initialized_p ())
+    {
+      return 0;
+    }
+
+  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 f8a8a72..ed67957 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -75,7 +75,8 @@ #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);
 \f
 /* For each display (currently only one on w32), we have a structure that
    records information about it.  */

Re: Interest in nt_load_image?

[Eli Zaretskii]

> From: Juan José García-Ripoll
>  <juanjose.garciaripoll@gmail.com>
> Date: Mon, 30 Mar 2020 09:54:26 +0200
> 
> > Please point to the relevant APIs, to make the discussion more
> > practical.
> 
> GDI+ is an evolution of GDI that supports arbitrary plug-ins for image formats,
> both bitmaps and vector type. It is a bit more modern than GDI, from what I
> get, but, just as GDI, it is not the modern standard for Windows 2D
> displays. Indeed, it is old enough that it is also supported by Windows XP.
> 
> https://docs.microsoft.com/en-us/windows/win32/gdiplus/-gdiplus-gdi-start
> 
> GDI+ has a flat C interface that allows loading images, querying properties,
> displaying them and converting them to older GDI formats.
> 
> https://docs.microsoft.com/en-us/windows/win32/gdiplus/-gdiplus-flatapi-flat

Thanks.  Interesting, I didn't know GDI+ had a C API.

Did you verify using these APIs will allow us to keep using the code
which processes images at display time?  Will image scaling and
rotation still work as it does now?  What about masking?

> - At configuration time, it works just as the NextStep (NS) system, disabling
>   the use of libpng, libjpeg, libtiff and libgif when the build system is
>   Mingw.

AFAIU, we will also be able to support EXIF without ImageMagick.

> - In images such as PNG, GIF or TIFF, it currently does not use a bitmask for
>   display. Instead, it relies on GDI+'s convertion to HBITMAP, which allows
>   alpha blending with any background color of choice.

I don't know enough to understand what that means, but if this doesn't
lose functionality, it's OK.

> - In the C code, it replaces the load_jpeg, load_gif, etc, with a generic
>   w32_load_image() function in src/w32image.c. This function is heavily
>   inspired by ns_load_image() in src/nsimage.c.

There's a complication we must consider in this regard, see below.

> The only thing that is missing is a place to call GdipShutdown(). I do not know
> how to add an exit handler for Emacs' C core.

I think you want to do it in w32.c:term_ntproc.

> I have also verified that it is possible to convert all *.xpm icons to *.png
> format and thus eliminate the need to include libXpm-noX.dll. Plus, the size of
> the icons is reduced by 50%

We aren't going to get rid of XPM icons in the distribution any time
soon (because of other platforms), so I don't see an urgent need to do
this.

> --- 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

We cannot do this at compile time, because we still try supporting
ancient Windows versions where GDI+ is not available.  Moreover, I
think it's good to let users have the ability to decide dynamically
which image display capability they want.  It certainly makes sense to
do that initially, while the GDI+ way is being tested in all kinds of
real-life use cases.

So all the compile-time tests will have to be rewritten as run-time
tests, and we should provide variables to force Emacs use/not use
GDI+, perhaps at image format granularity, and expose those variables
to Lisp, so users could control that.

A few other minor comments about the code below:

> +#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 init_w32_image_load_functions

And this stuff will have to learn to coexist with the current code
which loads PNG etc. images using the respective libraries.

> +static int
> +gdiplus_initialized_p()

Style: we use ANSI C99 declarations, so please say

  static int
  gdiplus_initialized_p (void)

(Btw, it looks like this function should return a 'bool', not 'int'.

Also, please leave a space between the end of the function/macro name
and the opening parentheses.

> +  static int gdip_initialized = 0;

No need to initialize a static variable to a zero value.

> +  if (gdip_initialized < 0)
> +    {
> +      return 0;
> +    }
> +  else if (gdip_initialized)
> +    {
> +      return 1;
> +    }

Style: we don't use braces when a block has only one line.

> +      status = GdiplusStartup(&token, &input, &output);
                               ^^
Style: space missing there.

> +static float
> +w32_frame_delay(GpBitmap *pBitmap, int frame)

A nit: why 'float' and not 'double'?  'float' causes a minor
inefficiency, so unless there's a good reason, I'd prefer 'double'.

> +   // Assume that the image has a property item of type PropertyItemEquipMake.
> +   // Get the size of that property item.

Please use C-style comments /* like this */, not C++-style comments.

> +      fprintf(stderr, "FrameCount: %d\n", (int)frameCount);
> +      fprintf(stderr, "     index: %d\n", frame);

Are these left-overs from the debugging stage?

> +static ARGB
> +w32_image_bg_color(struct frame *f, struct image *img)
> +{
> +  /* png_color_16 *image_bg; */
        ^^^^^^^^^^^^^^^^^^^^^^^
And this?

> +  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))

Do we really need to go through the hook mechanism here?  The frame
type is known in advance, right?

> +  if (STRINGP (spec_file))
> +    {
> +      filename_to_utf16 (SSDATA (spec_file) , filename);
> +      status = GdipCreateBitmapFromFile (filename, &pBitmap);

What to do here if w32-unicode-filenames is nil?

> +  else if (STRINGP (spec_data))
> +    {
> +      IStream *pStream = SHCreateMemStream ((BYTE *) SSDATA (spec_data),
> +                                            SBYTES (spec_data));

Are we sure spec_data is a unibyte string here?  Do we need an
assertion or maybe a test and a conversion?

> +      /* In multiframe pictures, select the first one */

Style: comments should end with a period and 2 spaces before */.

Re: Interest in nt_load_image?

[Juan José García-Ripoll]

Hi,

I reply to the more technical comments below. What I sent was not meant
for production: it was the result of four hours of work and digging
through Windows manuals. I was surprised it worked so well with so
little code.

Eli Zaretskii <eliz@gnu.org> writes:
> Did you verify using these APIs will allow us to keep using the code
> which processes images at display time?  Will image scaling and
> rotation still work as it does now?  What about masking?

Right now all Windows code works towards creating HBITMAP images. This
code only extends that functionality, using GDI+ as an intermediary to
finally produce an HBITMAP.

The only difference is in the masking of images. The current method does
not override the masking of images. However, just like the NextStep /
OSX port, it skips building a bitmask from alpha channels.

To be more precise, the code in image.c scans the image and creates a
separate bitmask if the alpha channel contains either 0x00 or 0xFF. If
there are intermediate values of alpha (from 0x01 to 0xFE) it relies on
alpha blending from the respective library. I do not understand
why. Maybe efficency?

I am right now trying to see whether this would affect some context,
such as icons in toolbars. However, as I said, the OSX port
(ns_load_image) does not use this dual use and instead relies on the OS
for blending.

>> - At configuration time, it works just as the NextStep (NS) system,
>> disabling the use of libpng, libjpeg, libtiff and libgif when the
>> build system is Mingw.
>
> AFAIU, we will also be able to support EXIF without ImageMagick.

The Windows codec takes care of the image orientation automatically, I
believe; but I have no image in mind I can test with. However if you
want any other more sophisticated handling, such as color spaces, one
might need to use a more modern interface (Windows Image Component).

>> I have also verified that it is possible to convert all *.xpm icons
>> to *.png format and thus eliminate the need to include
>> libXpm-noX.dll. Plus, the size of the icons is reduced by 50%
>
> We aren't going to get rid of XPM icons in the distribution any time
> soon (because of other platforms), so I don't see an urgent need to do
> this.

No. I did not mean to replace the xpm icons in the sources, but instead
distribute emacs without depending on libxpm-nox, at least on Windows.

> We cannot do this at compile time, because we still try supporting
> ancient Windows versions where GDI+ is not available.  Moreover, I
> think it's good to let users have the ability to decide dynamically
> which image display capability they want.  It certainly makes sense to
> do that initially, while the GDI+ way is being tested in all kinds of
> real-life use cases.

Understood. I was just submitting a proof-of-concept demonstration. I
will integrate the automated configuration in configure.ac later.

However, regarding "ancient" windows machines, GDI+ was introduced in
Windows XP and Windows 2000
(https://docs.microsoft.com/en-us/windows/win32/gdiplus/-gdiplus-gdi-start). It
hardly gets older than this with working machines, does it? You would
have to go back to Windows 98 or NT 4.0, whose support ended 17 years ago.

>> +static float
>> +w32_frame_delay(GpBitmap *pBitmap, int frame)
>
> A nit: why 'float' and not 'double'?  'float' causes a minor
> inefficiency, so unless there's a good reason, I'd prefer 'double'.

The use of float comes from ns_load_image.c I guess nobody has audited
that either.

>> +  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))
>
> Do we really need to go through the hook mechanism here?  The frame
> type is known in advance, right?

This code is not testing for frame type, but whether the background was
specified as an argument for image creation (uses the given value), or
not (it then uses ->query_frame_background). I just copied this form
image.c, which is where the determination of the PNG background color
takes place when libpng is used.

The good thing about this is that using Windows native components must
be faster than going through third-party libraries and this would allow
distributing a *-no-deps.zip with support for most standard bitmap
formats.

The bummer is that there is no simple replacement for rsvg yet, and this
library by itself pulls in libpng, libjpeg, libgif, libtiff, cairo, ...

Cheers,

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

Re: Interest in nt_load_image?

[Juan José García-Ripoll]

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

Juan José García-Ripoll <juanjose.garciaripoll@gmail.com> writes:
> I am right now trying to see whether this would affect some context,
> such as icons in toolbars. However, as I said, the OSX port
> (ns_load_image) does not use this dual use and instead relies on the OS
> for blending.

At least replacing the icons with *.png versions does not seem to create
any artifacts, as seen in the attached screen capture.

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

[-- Attachment #2: emacs-png-icons-gdi+.png --]
[-- Type: image/png, Size: 7005 bytes --]

Re: Interest in nt_load_image?

[Eli Zaretskii]

> From: Juan José García-Ripoll
>  <juanjose.garciaripoll@gmail.com>
> Date: Mon, 30 Mar 2020 16:36:05 +0200
> 
> >> I have also verified that it is possible to convert all *.xpm icons
> >> to *.png format and thus eliminate the need to include
> >> libXpm-noX.dll. Plus, the size of the icons is reduced by 50%
> >
> > We aren't going to get rid of XPM icons in the distribution any time
> > soon (because of other platforms), so I don't see an urgent need to do
> > this.
> 
> No. I did not mean to replace the xpm icons in the sources, but instead
> distribute emacs without depending on libxpm-nox, at least on Windows.

At 84KB, I don't see how the XPM DLL could be a problem for anyone.
More importantly, tool-bar.el explicitly loads XPM icons.

> However, regarding "ancient" windows machines, GDI+ was introduced in
> Windows XP and Windows 2000
> (https://docs.microsoft.com/en-us/windows/win32/gdiplus/-gdiplus-gdi-start). It
> hardly gets older than this with working machines, does it? You would
> have to go back to Windows 98 or NT 4.0, whose support ended 17 years ago.

I know.  Believe it or not, we do still support those old version (or
at least try not to break their support knowingly).  You will see that
in many places in the w32-specific code, for example we load
unicows.dll and call MultiByteToWideChar through a function pointer.

> >> +  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))
> >
> > Do we really need to go through the hook mechanism here?  The frame
> > type is known in advance, right?
> 
> This code is not testing for frame type, but whether the background was
> specified as an argument for image creation (uses the given value), or
> not (it then uses ->query_frame_background). I just copied this form
> image.c, which is where the determination of the PNG background color
> takes place when libpng is used.

image.c is mostly platform-independent code, but this isn't: the
result of

   FRAME_TERMINAL (f)->defined_color_hook

and its ilk, i.e. what functions will be called, is known in advance.
Why not call those functions directly?

> The good thing about this is that using Windows native components must
> be faster than going through third-party libraries and this would allow
> distributing a *-no-deps.zip with support for most standard bitmap
> formats.
> 
> The bummer is that there is no simple replacement for rsvg yet, and this
> library by itself pulls in libpng, libjpeg, libgif, libtiff, cairo, ...

I'm not really bothered by additional dependencies or by the disk
space they take.  My main motivation is to give users more options and
features to use, because I'm quite sure the results will not be
identical, so sometimes one method will be preferable, and sometimes
the other.  This makes Emacs more future-proof, which on a platform
such as MS-Windows is no small feat.

Re: Interest in nt_load_image?

[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 16:36:05 +0200
>> 
>> >> I have also verified that it is possible to convert all *.xpm icons
>> >> to *.png format and thus eliminate the need to include
>> >> libXpm-noX.dll. Plus, the size of the icons is reduced by 50%
>> >
>> > We aren't going to get rid of XPM icons in the distribution any time
>> > soon (because of other platforms), so I don't see an urgent need to do
>> > this.
>> 
>> No. I did not mean to replace the xpm icons in the sources, but instead
>> distribute emacs without depending on libxpm-nox, at least on Windows.
>
> At 84KB, I don't see how the XPM DLL could be a problem for anyone.
> More importantly, tool-bar.el explicitly loads XPM icons.

Not really. tool-bar.el uses find-image which can be trivially extended
to also seach for PNG as last resort. This is what I showed in my
previous message with a screenshot.

Also I am just trying to address Phillip's complaint about keeping track
of the dependencies. I agree that having to manually download and copy
libxpm-nox.dll is just one more complication that, in the Windows case,
is not needed. This would also make the *-no-deps.zip file only
dependent on libgmp.dll and nothing else.

diff --git a/lisp/tool-bar.el b/lisp/tool-bar.el
index 7df1e28..d0aef3d 100644
--- a/lisp/tool-bar.el
+++ b/lisp/tool-bar.el
@@ -150,16 +150,20 @@ tool-bar--image-expression
 	 (xpm-spec (list :type 'xpm :file (concat icon ".xpm")))
 	 (xpm-lo-spec (list :type 'xpm :file
 			    (concat "low-color/" icon ".xpm")))
+	 (png-lo-spec (list :type 'png :file
+			    (concat "low-color/" icon ".png")))
 	 (pbm-spec (append (list :type 'pbm :file
                                  (concat icon ".pbm")) colors))
 	 (xbm-spec (append (list :type 'xbm :file
-                                 (concat icon ".xbm")) colors)))
+                                 (concat icon ".xbm")) colors))
+	 (png-spec (append (list :type 'png :file
+                                 (concat icon ".png")) colors)))
     `(find-image (cond ((not (display-color-p))
 			',(list pbm-spec xbm-spec xpm-lo-spec xpm-spec))
 		       ((< (display-color-cells) 256)
-			',(list xpm-lo-spec xpm-spec pbm-spec xbm-spec))
+			',(list xpm-lo-spec png-lo-spec xpm-spec png-spec pbm-spec xbm-spec))
 		       (t
-			',(list xpm-spec pbm-spec xbm-spec))))))
+			',(list xpm-spec png-spec pbm-spec xbm-spec))))))


>> However, regarding "ancient" windows machines, GDI+ was introduced in
>> Windows XP and Windows 2000
>> (https://docs.microsoft.com/en-us/windows/win32/gdiplus/-gdiplus-gdi-start). It
>> hardly gets older than this with working machines, does it? You would have
>> to go back to Windows 98 or NT 4.0, whose support ended 17 years ago.
>
> I know.  Believe it or not, we do still support those old version (or at
> least try not to break their support knowingly).  You will see that in many
> places in the w32-specific code, for example we load unicows.dll and call
> MultiByteToWideChar through a function pointer.

I believe this executable cannot be built in older versions, as msys does not
even support Windows XP. It pobably also does not run there. What would you use
to build it? Are there any regression tests for those platforms?

>> >> +  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))
>> >
>> > Do we really need to go through the hook mechanism here?  The frame
>> > type is known in advance, right?
>> 
>> This code is not testing for frame type, but whether the background was
>> specified as an argument for image creation (uses the given value), or not
>> (it then uses ->query_frame_background). I just copied this form image.c,
>> which is where the determination of the PNG background color takes place
>> when libpng is used.
>
> image.c is mostly platform-independent code, but this isn't: the result of
>
>    FRAME_TERMINAL (f)->defined_color_hook
>
> and its ilk, i.e. what functions will be called, is known in advance.  Why
> not call those functions directly?

You assume too much of me. I do not know what is going on there under the
hoods. And your question also applies to the code in image.c that uses this
exact piece of code, and which is in the non-platform-dependent part (HAVE_PNG
section).

> I'm not really bothered by additional dependencies or by the disk space they
> take.  My main motivation is to give users more options and features to use,
> because I'm quite sure the results will not be identical, so sometimes one
> method will be preferable, and sometimes the other.  This makes Emacs more
> future-proof, which on a platform such as MS-Windows is no small feat.

My motivation on this particular contribution is not space, but removing
external dependencies and making the code more robust. Building the Windows
port around GDI+ makes it also more future proof: if GDI+ support is dropped in
the near future (which I doubt, given it is also supported by Direct2D), it can
be almost trivially replaced with Windows Image Component, which is the more
modern interface.

Cheers,

P.S.: Just a side question, is LCMS2 actually used anywhere in Emacs?

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

Re: Interest in nt_load_image?

[Eli Zaretskii]

> From: Juan José García-Ripoll
>  <juanjose.garciaripoll@gmail.com>
> Date: Mon, 30 Mar 2020 18:15:47 +0200
> 
> > At 84KB, I don't see how the XPM DLL could be a problem for anyone.
> > More importantly, tool-bar.el explicitly loads XPM icons.
> 
> Not really. tool-bar.el uses find-image which can be trivially extended
> to also seach for PNG as last resort. This is what I showed in my
> previous message with a screenshot.

Why bother?

> > I know.  Believe it or not, we do still support those old version (or at
> > least try not to break their support knowingly).  You will see that in many
> > places in the w32-specific code, for example we load unicows.dll and call
> > MultiByteToWideChar through a function pointer.
> 
> I believe this executable cannot be built in older versions, as msys does not
> even support Windows XP.

It cannot be built on those old systems, but we hope it's able to run
on them.

> It pobably also does not run there. What would you use
> to build it? Are there any regression tests for those platforms?

Like I said: we try not to knowingly break those systems, that's all.
Wed cannot test on those systems without someone who has access to
them helping us.

> P.S.: Just a side question, is LCMS2 actually used anywhere in Emacs?

Yes, see src/lcms.c.

Re: Interest in nt_load_image?

[Stefan Monnier]

> It cannot be built on those old systems, but we hope it's able to run
> on them.

FWIW, the last time I remember discussions here where someone was
actually running Emacs on Windows-98 was more than 10 years ago.

Since then, I've seen mentions of Windows-98 support but only in the
context of "blind" efforts to try and make sure it can still work on
Windows-98.

Maybe we should try and use this Windows-98-incompatible feature and see
if someone hollers, as a way to test whether we do still have users
running such systems.


        Stefan

Re: Interest in nt_load_image?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Juan José García-Ripoll
>  <juanjose.garciaripoll@gmail.com>,
>   emacs-devel@gnu.org
> Date: Mon, 30 Mar 2020 13:16:13 -0400
> 
> Since then, I've seen mentions of Windows-98 support but only in the
> context of "blind" efforts to try and make sure it can still work on
> Windows-98.

That's exactly what we are trying to do, and cannot (and shouldn't)
try doing more.

> Maybe we should try and use this Windows-98-incompatible feature and see
> if someone hollers, as a way to test whether we do still have users
> running such systems.

I don't see the point.  We have enough real issues to worry about,
let's not waste precious resources on trying to figure out whether the
9X code can be dropped.  One day it will die of natural causes, no
need to invest even a minimal effort in that.

Re: Interest in nt_load_image?

[Stefan Monnier]

> I don't see the point.  We have enough real issues to worry about,
> let's not waste precious resources on trying to figure out whether the
> 9X code can be dropped.

I don't see what resources you're talking about.

From where I stand, you're suggesting to "waste precious resources"
trying to make the code hypothetically work in Windows-98, whereas I'm
suggesting we don't do that and let other people come whining if they
suffer the consequences.

> One day it will die of natural causes, no need to invest even
> a minimal effort in that.

I'm not sure how that would happen: if noone tries to run current Emacs
on Windows-98 any more, then we'll never discover that it doesn't run
there any more.
So in this context, what would be a scenario where it would die?


        Stefan

Re: Interest in nt_load_image?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: juanjose.garciaripoll@gmail.com,  emacs-devel@gnu.org
> Date: Mon, 30 Mar 2020 14:41:25 -0400
> 
> > I don't see the point.  We have enough real issues to worry about,
> > let's not waste precious resources on trying to figure out whether the
> > 9X code can be dropped.
> 
> I don't see what resources you're talking about.

Resources needed to try to figure out whether we need the
compatibility code, devise probes, waiting for someone to holler and
remembering to check if someone did...

> >From where I stand, you're suggesting to "waste precious resources"
> trying to make the code hypothetically work in Windows-98, whereas I'm
> suggesting we don't do that and let other people come whining if they
> suffer the consequences.

There are no consequences.  The paradigm of loading a shared library
dynamically and testing whether doing that succeeded is so boilerplate
in the w32 code that it never takes more than a simple copy/pasta to
add one more.

> > One day it will die of natural causes, no need to invest even
> > a minimal effort in that.
> 
> I'm not sure how that would happen

It will happen when 32-bit builds will die.  2038 at the latest.

Re: Interest in nt_load_image?

[Stefan Monnier]

>> I don't see what resources you're talking about.
> Resources needed to try to figure out whether we need the
> compatibility code, devise probes, waiting for someone to holler and
> remembering to check if someone did...

I'm not suggesting we do any of those things.

I suggest we write the code under the assumption that GDI+ is available
and that's it.  "waiting for someone to holler" is already taken care of
by debbugs, so it doesn't take any resources.  As for remembering to
check, there's no need to do that either: if noone hollers, then we
do nothing.

>> From where I stand, you're suggesting to "waste precious resources"
>> trying to make the code hypothetically work in Windows-98, whereas I'm
>> suggesting we don't do that and let other people come whining if they
>> suffer the consequences.
> There are no consequences.  The paradigm of loading a shared library
> dynamically and testing whether doing that succeeded is so boilerplate
> in the w32 code that it never takes more than a simple copy/pasta to
> add one more.

But what do we do if the loading fails?  If we simply fail to display
the image, then that's fine by me.  If OTOH we go through the trouble of
falling back on some other code which otherwise wouldn't be needed in
the build (and would hence never be tested, BTW), then it sounds like
"waste precious resources".

>> > One day it will die of natural causes, no need to invest even
>> > a minimal effort in that.
>> I'm not sure how that would happen
> It will happen when 32-bit builds will die.  2038 at the latest.

Are we sure sure 32bit Windows installations will have all disappeared
by 2038 ?  ;-)  


        Stefan "who finds 18 more years of support for Windows-98 rather
                excessive"

Re: Interest in nt_load_image?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: juanjose.garciaripoll@gmail.com,  emacs-devel@gnu.org
> Date: Mon, 30 Mar 2020 15:39:11 -0400
> 
> I suggest we write the code under the assumption that GDI+ is available
> and that's it.

I'd like to have a Lisp knob to select whether GDI+ or the various
image libraries are used for image handling, so the for systems
without GDI+ comes at almost no cost: just determine at startup what
is/are the value(s) of those variable(s).

> > There are no consequences.  The paradigm of loading a shared library
> > dynamically and testing whether doing that succeeded is so boilerplate
> > in the w32 code that it never takes more than a simple copy/pasta to
> > add one more.
> 
> But what do we do if the loading fails?  If we simply fail to display
> the image, then that's fine by me.  If OTOH we go through the trouble of
> falling back on some other code which otherwise wouldn't be needed in
> the build (and would hence never be tested, BTW), then it sounds like
> "waste precious resources".

If the library cannot be loaded, or doesn't have all the required
functions, the corresponding feature becomes unavailable, and we will
generally display an error message.  This is clearly visible in the
code: that's what those FOO-available-p functions are there for.  In
some rare cases, where we already have a fallback, we use it instead.
But those cases are a small minority.

> >> > One day it will die of natural causes, no need to invest even
> >> > a minimal effort in that.
> >> I'm not sure how that would happen
> > It will happen when 32-bit builds will die.  2038 at the latest.
> 
> Are we sure sure 32bit Windows installations will have all disappeared
> by 2038 ?  ;-)  

Yes, I'm sure.  In any case, Emacs will not be able to run on them, as
we use time functions all over the place.

Re: Interest in nt_load_image?

[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 18:15:47 +0200
>> 
>> > At 84KB, I don't see how the XPM DLL could be a problem for anyone.
>> > More importantly, tool-bar.el explicitly loads XPM icons.
>> 
>> Not really. tool-bar.el uses find-image which can be trivially extended
>> to also seach for PNG as last resort. This is what I showed in my
>> previous message with a screenshot.
>
> Why bother?

Why not? Why not allow the toolbar icons to be PNG's? Using them in
Windows removes one more dependency and there is the opportunity to have
nicer icons, just like the ones Aquamacs uses.

In any case, support for Xpm seems to be in a somewhat fluid state. The
NS port is regularly built without libxpm, as shown in the
#ifdef's. That configuration is also the one for Cairo builds. So there
is a trend there, right? Good thing is I see I can use that code to make
a build with GDI+ that support Xpm v.3 but does not require libxpm-nox
(i.e. --with-gdiplus --without-xpm would activate it).

>> P.S.: Just a side question, is LCMS2 actually used anywhere in Emacs?
>
> Yes, see src/lcms.c.

Sorry, apologies once more for the ambiguity: I meant _used_ not exposed
as a library. lcms.c seems only to provide functions to compute color
distances, but the color management does not seem to be used anywhere by
Emacs.

Cheers,

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

Re: Interest in nt_load_image?

[Eli Zaretskii]

> From: Juan José García-Ripoll
>  <juanjose.garciaripoll@gmail.com>
> Date: Mon, 30 Mar 2020 20:06:27 +0200
> 
> In any case, support for Xpm seems to be in a somewhat fluid state. The
> NS port is regularly built without libxpm, as shown in the
> #ifdef's. That configuration is also the one for Cairo builds. So there
> is a trend there, right? Good thing is I see I can use that code to make
> a build with GDI+ that support Xpm v.3 but does not require libxpm-nox
> (i.e. --with-gdiplus --without-xpm would activate it).

When we decide to move away from XPM on Posix platforms, that would be
a good time to revisit this issue for MS-Windows.  Until then, I see
no reason to be bothered by a small library that never caused any
trouble.

> >> P.S.: Just a side question, is LCMS2 actually used anywhere in Emacs?
> >
> > Yes, see src/lcms.c.
> 
> Sorry, apologies once more for the ambiguity: I meant _used_ not exposed
> as a library. lcms.c seems only to provide functions to compute color
> distances, but the color management does not seem to be used anywhere by
> Emacs.

I don't see it used anywhere in Emacs.  Maybe some third-party
packages do, though.

Re: Interest in nt_load_image?

[Eli Zaretskii]

> Date: Mon, 30 Mar 2020 21:47:38 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > >> P.S.: Just a side question, is LCMS2 actually used anywhere in Emacs?
> > >
> > > Yes, see src/lcms.c.
> > 
> > Sorry, apologies once more for the ambiguity: I meant _used_ not exposed
> > as a library. lcms.c seems only to provide functions to compute color
> > distances, but the color management does not seem to be used anywhere by
> > Emacs.
> 
> I don't see it used anywhere in Emacs.  Maybe some third-party
> packages do, though.

Actually, I take that back: those functions can be used as the 4th
argument to color-distance.

Re: Interest in nt_load_image?

[Juanma Barranquero]

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

On Mon, Mar 30, 2020 at 6:17 PM Juan José García-Ripoll <
juanjose.garciaripoll@gmail.com> wrote:

> if GDI+ support is dropped in
> the near future (which I doubt, given it is also supported by Direct2D),
it can
> be almost trivially replaced with Windows Image Component, which is the
more
> modern interface.

Is there any advantage in using Windows Imaging Component?

I mean, if as Eli suggests you're going to dynamically check for GDI+
support and revert back to the current API in Windows 9X, it is feasible to
check for Windows Imaging Component when Emacs runs on modern Windows and
use it if available? Does it offer any advantage?

Just curious.

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

Re: Interest in nt_load_image?

[Juan José García-Ripoll]

Hi Juanma,

let me clarify the issues because I think you (and probably others) have
misunderstood what I propose.

I have build GDI+ as a built-in back-end with which to compile Emacs. An
Emacs built with GDI+ requires UTF-16 support and thus will not run on
Windows 98, but it will run on Windows XP on later. GDI+ support is
optional because it can be selected at build time, not because it can be
chosen at compilation time.

Juanma Barranquero <lekktu@gmail.com> writes:
> it is feasible to check for Windows Imaging Component when Emacs runs
> on modern Windows and use it if available?

Not. That is not how it is intended to work, just like . But that is a non-issue.

>> if GDI+ support is dropped in the near future (which I doubt, given
>> it is also supported by Direct2D), it can be almost trivially
>> replaced with Windows Image Component, which is the more
>> modern interface.
>
> Is there any advantage in using Windows Imaging Component?

WIC and Direct2D are Windows' now standard display components, providing
support for accelerated GPU-based processing. WIC can act as a canvas
for rendering other components and would allow for a more up-to-date
capability in terms of image processing.

> I mean, if as Eli suggests you're going to dynamically check for GDI+
> support and revert back to the current API in Windows 9X,...

Neither of us said this. It is a compilation parameter.

Cheers,

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

Re: Interest in nt_load_image?

[Juanma Barranquero]

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

Hi, Juan José

On Tue, Mar 31, 2020 at 10:03 AM Juan José García-Ripoll <
juanjose.garciaripoll@gmail.com> wrote:

> I have build GDI+ as a built-in back-end with which to compile Emacs. An
> Emacs built with GDI+ requires UTF-16 support and thus will not run on
> Windows 98, but it will run on Windows XP on later. GDI+ support is
> optional because it can be selected at build time, not because it can be
> chosen at compilation time.

Assuming you mean "not because it can be chosen at run time", ok, but then
the
(not really) "official" builds of Emacs for Windows will either not use
GDI+, or
it'll be necessary to provide GDI+ and non-GDI+ builds. We still want to
support
Windows 9X.

> WIC and Direct2D are Windows' now standard display components, providing
> support for accelerated GPU-based processing. WIC can act as a canvas
> for rendering other components and would allow for a more up-to-date
> capability in terms of image processing.

Well, then it would be perhaps a worthy goal for (GDI+ builds of) Emacs to
detect
WIC at run-time and use it instead.

> > I mean, if as Eli suggests you're going to dynamically check for GDI+
> > support and revert back to the current API in Windows 9X,...
>
> Neither of us said this. It is a compilation parameter.

Eli said:

> We cannot do this at compile time, because we still try supporting
> ancient Windows versions where GDI+ is not available.  Moreover, I
> think it's good to let users have the ability to decide dynamically
> which image display capability they want.  It certainly makes sense to
> do that initially, while the GDI+ way is being tested in all kinds of
> real-life use cases.
> So all the compile-time tests will have to be rewritten as run-time
> tests, and we should provide variables to force Emacs use/not use
> GDI+, perhaps at image format granularity, and expose those variables
> to Lisp, so users could control that.

Did I misunderstand him?

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

Re: Interest in nt_load_image?

[Stefan Monnier]

> We still want to support Windows 9X.

Do we?  Do you know (of) someone using a recentish Emacs on Windows 9X?

>> We cannot do this at compile time, because we still try supporting
>> ancient Windows versions where GDI+ is not available.  Moreover, I
>> think it's good to let users have the ability to decide dynamically
>> which image display capability they want.  It certainly makes sense to
>> do that initially, while the GDI+ way is being tested in all kinds of
>> real-life use cases.
>> So all the compile-time tests will have to be rewritten as run-time
>> tests, and we should provide variables to force Emacs use/not use
>> GDI+, perhaps at image format granularity, and expose those variables
>> to Lisp, so users could control that.
>
> Did I misunderstand him?

Yes, he described the extra efforts that we should spend to try and
provide support for those hypothetical Windows 9X users out there.
I think that is very likely to be a waste, so I think we should first do
the easy thing and *if/when* someone comes complaining that it doesn't
work on Windows 9X then we can revisit this decision.


        Stefan

Re: Interest in nt_load_image?

[Juanma Barranquero]

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

On Tue, Mar 31, 2020 at 4:51 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:
>
> > We still want to support Windows 9X.
>
> Do we?  Do you know (of) someone using a recentish Emacs on Windows 9X?

No, I don't, but I had people running Emacs on Windows 9X just a couple of
years ago or so.

Anyway, the argument (not mine, Richard's and Eli's, I think) has been that
there are people, mostly in third world countries, with no access to modern
hardware or the possibility to upgrade to newer Windows. The
counter-argument is that they can run older Emacs, of course. Perhaps. I
don't really want to argue one way or another, to me whether to support
Windows 9X or not is decided by whoever bears the burden of maintainership.

> > Did I misunderstand him?
>
> Yes, he described the extra efforts that we should spend to try and
> provide support for those hypothetical Windows 9X users out there.

Yes, I did misunderstand him, or yes he said what I think and you disagree?
(I'm assuming the latter.)

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

Re: Interest in nt_load_image?

[Stefan Monnier]

>> > Did I misunderstand him?
>> Yes, he described the extra efforts that we should spend to try and
>> provide support for those hypothetical Windows 9X users out there.
> Yes, I did misunderstand him, or yes he said what I think and you disagree?
> (I'm assuming the latter.)

You're assuming right, thanks ;-)


        Stefan

Re: Interest in nt_load_image?

[Juan José García-Ripoll]

Juanma Barranquero <lekktu@gmail.com> writes:
>> We cannot do this at compile time, because we still try supporting
>> ancient Windows versions where GDI+ is not available. 
>
> Did I misunderstand him?

On a different email I was instructed to add this as a compile time
option, which is what my patch does. Sorry if I did not follow all
threads properly.

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

Re: Interest in nt_load_image?

[Juanma Barranquero]

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

On Tue, Mar 31, 2020 at 5:14 PM Juan José García-Ripoll <
juanjose.garciaripoll@gmail.com> wrote:

> On a different email I was instructed to add this as a compile time
> option, which is what my patch does. Sorry if I did not follow all
> threads properly.

My quote is from this very thread. But of course it is easy to accidentally
skip things when threads get too long or some issue is simultaneously
discussed in several places at once.

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

Re: Interest in nt_load_image?

[Eli Zaretskii]

> From: Juan José García-Ripoll
>  <juanjose.garciaripoll@gmail.com>
> Date: Tue, 31 Mar 2020 10:01:46 +0200
> 
> I have build GDI+ as a built-in back-end with which to compile Emacs. An
> Emacs built with GDI+ requires UTF-16 support and thus will not run on
> Windows 98, but it will run on Windows XP on later. GDI+ support is
> optional because it can be selected at build time, not because it can be
> chosen at compilation time.

The problem with such selection of features at build time is that the
resulting binary will flatly refuse to start on systems where the
optional functionality (in this case, GDI+) is not available, even if
there's no call at runtime to any of the optional functions.  Windows
refuses to run a program which has a function in its import table that
cannot be found in the shared libraries against which the program was
linked, or if the program was linked against a shared library that is
not available on the target system.  And on Windows we frequently see
that people are using binaries someone else compiled, so we cannot
rely on the end-user to build Emacs according to what will be
available on the end-user's system.

That is why we never link Emacs on Windows against any optional
libraries, but instead load them at runtime when the corresponding
feature is first requested.  The features whose DLL couldn't be
loaded, or where the DLL doesn't have the required functions -- those
features will not become available, but at least Emacs will start and
allow you to use it for other jobs.

In the case of GDI+, I'm quite sure we will want to have runtime
control on whether it is used even if available, at least initially,
until it becomes the default (if it ever does).  This is good for
maintenance (debugging problems people report), if nothing else.  I
could even envision users who'd prefer libpng etc. for some other
reason, whether technical or otherwise.

Therefore, catering to systems without GDI+ is very simple: the
respective variable(s) should be set up at startup to indicate that
GDI+ shouldn't be used.

Re: Interest in nt_load_image?

[Stefan Monnier]

>>  #include <config.h>
>> +#ifdef HAVE_GDIPLUS
>> +#undef HAVE_JPEG
>> +#undef HAVE_PNG
>> +#undef HAVE_GIF
>> +#undef HAVE_TIFF
>> +#endif
>
> We cannot do this at compile time, because we still try supporting
> ancient Windows versions where GDI+ is not available.

Which versions would be affected, in practice?
IIUC GDI+ was introduced with Windows XP and is available for Windows
98 (of course, I don't have any first hand knowledge of that), so maybe
it's not a real problem.


        Stefan

Re: Interest in nt_load_image?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Juan José García-Ripoll
>  <juanjose.garciaripoll@gmail.com>,
>   emacs-devel@gnu.org
> Date: Mon, 30 Mar 2020 11:14:13 -0400
> 
> > We cannot do this at compile time, because we still try supporting
> > ancient Windows versions where GDI+ is not available.
> 
> Which versions would be affected, in practice?
> IIUC GDI+ was introduced with Windows XP and is available for Windows
> 98

GDI+ is unavailable on Windows 9X, AFAIK.

Re: Interest in nt_load_image?

[Stefan Monnier]

> GDI+ is unavailable on Windows 9X, AFAIK.

https://www.wysiwygwebbuilder.com/gdiplus.html seems to disagree.

Of course, I have no reason to trust this site and the link they give is
dead, so take it with a large grain of salt (but not too large:
I wouldn't want to cause any discomfort).


        Stefan

Re: Interest in nt_load_image?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: juanjose.garciaripoll@gmail.com,  emacs-devel@gnu.org
> Date: Mon, 30 Mar 2020 13:10:32 -0400
> 
> > GDI+ is unavailable on Windows 9X, AFAIK.
> 
> https://www.wysiwygwebbuilder.com/gdiplus.html seems to disagree.
> 
> Of course, I have no reason to trust this site

I indeed don't trust it, since MS says it started from Windows 2000,
and the APIs there that load files expect UTF-16 encoded file names,
something that cannot work on Windows 9X without a lot of help.