Image decoding for many popular formats for Simple Directmedia Layer.

Adding --enable-jxl-shared does not make libjxl dynamically loaded since SONAME of libjxl has two digits separated by dot:

https://github.com/libjxl/libjxl/blob/101fa3f748578553b3fb25e5f78cee69d938356a/lib/CMakeLists.txt#L19-L24

and SDL_image discards any file not matching <libname>.so.<digit>+: https://github.com/libsdl-org/SDL_image/blob/3572a87e97bc7163eda30775880d3eeebd7c54c0/configure.ac#L135

Note: created PR in SDL to fix this: https://github.com/libsdl-org/SDL/pull/5901. If accepted I will create similar one for SDL_image.

don't merge yet

Known issues that need to be fixed before merging can occur:

• [ ] cmake cannot build with vendored dependencies: https://github.com/libsdl-org/SDL_image/pull/189#issuecomment-1121476928
• [x] on mingw with dynamic libraries + system libraries, the cmake script uses the import library instead of the dll
• [x] on Linux with dynamic libraries, the cmake script might select the shared library with the "longest version name". It should e.g. for libpng my system packages provides:

  /usr/lib64/libpng16.so -> libpng16.so.16.37.0
  /usr/lib64/libpng16.so.16 -> libpng16.so.16.37.0
  /usr/lib64/libpng16.so.16.37.0
  

  Because /usr/lib64/libpng16.so links to libpng16.so.16.37.0, it selects that one (and /usr/lib64/libpng16.16 is ignored).

• [ ] consider testing CMake on mingw, as demonstrated in https://github.com/libsdl-org/SDL_image/pull/189#issuecomment-1122173090

This pr improves (imho) the cmake build script by:

• adding options for enabling/disabling each loader
• allow switching between vendored and system libraries for jpeg/libpng/libwebp/libtiff/zlib
• make the file names of the created libraries/files in a prefix the same
• installs a SDL2_image-config.cmake file to enable cmake users to do find_package(SDL2_image CONFIG) and use a SDL2::SDL2_image target.
• for compatibility, also create a SDL2_image.pc file
• I think this provides a better solution then #187 to add PIC => I have added a BUILD_PIC option. (#187 was caused by libjpeg building a static library without PIC, which cannot be used inside a shared library)
• adds support for dynamically loading libraries (instead of linking to them at build time)

See https://github.com/libsdl-org/SDL_image/pull/189#issuecomment-1121476928 for current issues.

When building using system dependencies, the install prefix looks like:

prefix
├── include
│   └── SDL2
│       └── SDL_image.h
├── lib64
│   ├── cmake
│   │   └── SDL2_image
│   │       ├── SDL2_image-config.cmake
│   │       ├── SDL2_image-targets.cmake
│   │       └── SDL2_image-targets-noconfig.cmake
│   ├── libSDL2_image-2.0.so -> libSDL2_image-2.0.so.0
│   ├── libSDL2_image-2.0.so.0 -> libSDL2_image-2.0.so.0.6.0
│   ├── libSDL2_image-2.0.so.0.6.0
│   ├── libSDL2_image.so -> libSDL2_image-2.0.so
│   └── pkgconfig
│       └── SDL2_image.pc
└── share
    └── licenses
        └── SDL2_image
            └── LICENSE.txt

9 directories, 10 files

When building+installing all dependencies, the install prefix looks like:

prefix
├── include
│   └── SDL2
│       └── SDL_image.h
├── lib64
│   ├── cmake
│   │   └── SDL2_image
│   │       ├── SDL2_image-config.cmake
│   │       ├── SDL2_image-targets.cmake
│   │       └── SDL2_image-targets-noconfig.cmake
│   ├── libjpeg.so
│   ├── libpng16.so -> libpng16.so.16
│   ├── libpng16.so.16 -> libpng16.so.16.37.0
│   ├── libpng16.so.16.37.0
│   ├── libSDL2_image-2.0.so -> libSDL2_image-2.0.so.0
│   ├── libSDL2_image-2.0.so.0 -> libSDL2_image-2.0.so.0.6.0
│   ├── libSDL2_image-2.0.so.0.6.0
│   ├── libSDL2_image.so -> libSDL2_image-2.0.so
│   ├── libtiff.so -> libtiff.so.5
│   ├── libtiff.so.5 -> libtiff.so.5.6.0
│   ├── libtiff.so.5.6.0
│   ├── libz.so -> libz.so.1
│   ├── libz.so.1 -> libz.so.1.2.11
│   ├── libz.so.1.2.11
│   └── pkgconfig
│       └── SDL2_image.pc
└── share
    └── licenses
        └── SDL2_image
            └── LICENSE.txt

9 directories, 20 files

The prefix for a shared mingw build:

prefix
├── bin
│   └── SDL2_image.dll
├── include
│   └── SDL2
│       └── SDL_image.h
├── lib
│   ├── cmake
│   │   └── SDL2_image
│   │       ├── SDL2_image-config.cmake
│   │       ├── SDL2_image-targets.cmake
│   │       └── SDL2_image-targets-noconfig.cmake
│   ├── libSDL2_image.dll.a
│   └── pkgconfig
│       └── SDL2_image.pc
└── share
    └── licenses
        └── SDL2_image
            └── LICENSE.txt

10 directories, 8 files

Tested on [email protected] and [email protected]

Some feedback would be appreciated.

TODO:

• [ ] Compatibility with Cmake's FindSDL_image? Or it that one only looking for 1.x?

The reasoning behind this extra github workflow dependency is enforcing compatibility with the minimum required cmake version we declared at the top of the cmake build script.

It would be nice to re-add the define to compile out the PNG support:

eg:

diff --git a/Android.mk b/Android.mk
index c6d83db..7e9ab1c 100644
--- a/Android.mk
+++ b/Android.mk
@@ -127,6 +136,11 @@ ifeq ($(SUPPORT_PNG),true)
     LOCAL_CFLAGS += -DLOAD_PNG
     LOCAL_STATIC_LIBRARIES += png
     LOCAL_LDLIBS += -lz
+ifeq ($(SUPPORT_SAVE_PNG),true)
+    LOCAL_CFLAGS += -DSDL_IMAGE_SAVE_PNG=1
+else
+    LOCAL_CFLAGS += -DSDL_IMAGE_SAVE_PNG=0
+endif
 endif
 
 ifeq ($(SUPPORT_WEBP),true)
diff --git a/IMG_png.c b/IMG_png.c
index 462fe2f..61b5621 100644
--- a/IMG_png.c
+++ b/IMG_png.c
@@ -23,9 +23,8 @@
 
 #include "SDL_image.h"
 
-/* We'll have PNG save support by default */
-#ifndef SAVE_PNG
-#define SAVE_PNG    1
+#if !defined(SDL_IMAGE_SAVE_PNG)
+#  define SDL_IMAGE_SAVE_PNG 1
 #endif
 
 #if defined(USE_STBIMAGE)
@@ -115,7 +116,7 @@ static struct {
     jmp_buf* (*png_set_longjmp_fn) (png_structrp, png_longjmp_ptr, size_t);
 #endif
 #endif
-#if SAVE_PNG
+#if SDL_IMAGE_SAVE_PNG
     png_structp (*png_create_write_struct) (png_const_charp user_png_ver, png_voidp error_ptr, png_error_ptr error_fn, png_error_ptr warn_fn);
     void (*png_destroy_write_struct) (png_structpp png_ptr_ptr, png_infopp info_ptr_ptr);
     void (*png_set_write_fn) (png_structrp png_ptr, png_voidp io_ptr, png_rw_ptr write_data_fn, png_flush_ptr output_flush_fn);
@@ -170,7 +173,7 @@ int IMG_InitPNG()
         FUNCTION_LOADER(png_set_longjmp_fn, jmp_buf* (*) (png_structrp, png_longjmp_ptr, size_t))
 #endif
 #endif
-#if SAVE_PNG
+#if SDL_IMAGE_SAVE_PNG
         FUNCTION_LOADER(png_create_write_struct, png_structp (*) (png_const_charp user_png_ver, png_voidp error_ptr, png_error_ptr error_fn, png_error_ptr warn_fn))
         FUNCTION_LOADER(png_destroy_write_struct, void (*) (png_structpp png_ptr_ptr, png_infopp info_ptr_ptr))
         FUNCTION_LOADER(png_set_write_fn, void (*) (png_structrp png_ptr, png_voidp io_ptr, png_rw_ptr write_data_fn, png_flush_ptr output_flush_fn))

@@ -540,7 +599,7 @@ SDL_Surface *IMG_LoadPNG_RW(SDL_RWops *src)
 
 #endif /* LOAD_PNG */
 
-#if SAVE_PNG
+#if SDL_IMAGE_SAVE_PNG
 
 #if SDL_BYTEORDER == SDL_LIL_ENDIAN
 static const Uint32 png_format = SDL_PIXELFORMAT_ABGR8888;
@@ -732,7 +791,7 @@ static int IMG_SavePNG_RW_miniz(SDL_Surface *surface, SDL_RWops *dst, int freeds
 }
 #endif /* LOAD_PNG_DYNAMIC || !WANT_LIBPNG */
 
-#endif /* SAVE_PNG */
+#endif /* SDL_IMAGE_SAVE_PNG */
 
 int IMG_SavePNG(SDL_Surface *surface, const char *file)
 {
@@ -746,7 +805,7 @@ int IMG_SavePNG(SDL_Surface *surface, const char *file)
 
 int IMG_SavePNG_RW(SDL_Surface *surface, SDL_RWops *dst, int freedst)
 {
-#if SAVE_PNG
+#if SDL_IMAGE_SAVE_PNG
 #ifdef USE_LIBPNG
     if ((IMG_Init(IMG_INIT_PNG) & IMG_INIT_PNG) != 0) {
         if (IMG_SavePNG_RW_libpng(surface, dst, freedst) == 0) {
@@ -763,5 +822,5 @@ int IMG_SavePNG_RW(SDL_Surface *surface, SDL_RWops *dst, int freedst)
 #else
     return IMG_SetError("SDL_image built without PNG save support");
 
-#endif /* SAVE_PNG */
+#endif /* SDL_IMAGE_SAVE_PNG */
 }

@madebr has made some upstream patches for libjxl and dependencies:

• libjxl: apply my patch from Wrap include directories in BUILD_INTERFACE libjxl/libjxl#1414 (or wait until/if upstream accepts it)

• libhighway: create a libsdl-org/libhighway fork + make sure my patch Wrap build system include directories with generator expression google/highway#687 is included

• brotli: create a libsdl-rog/brotli fork + apply the patch at Wrap interface include directories with BUILD_INTERFACE generator expression google/brotli#966 (or wait until/if upstream accepts it)

This issue is open to track the upstream pull requests. If they're not accepted, we can fork the projects and make the changes on libsdl-org repos.

Bug 1: Position independent code

Compiling SDL2_image as a shared library will not enable CMAKE_POSITION_INDEPENDENT_CODE, which is necessary for libjpeg as it is always compiled as a static library.

Before this fix, attempting to compile SDL2_image with BUILD_SHARED_LIBS=ON gives the following message at compile time:

/usr/bin/ld: external/jpeg-9d/libjpeg.a(jcapimin.c.o): relocation R_X86_64_PC32 against symbol `jpeg_natural_order' can not be used when making a shared object; recompile with -fPIC

I fixed this by enabling CMAKE_POSITION_INDEPENDENT_CODE (which is CMake's flag for -fPIC) for all builds, regardless of BUILD_SHARED_LIBS.

Bug 2: Compiling with system SDL

SDL2_image attempts to compile against SDL2::SDL2 all the time, although that target may not be valid if SDL was found with find_package(SDL2 REQUIRED). The variables SDL2_LIBRARIES and SDL2_LIBRARIES serve that purpose; I added a check for SDL2_FOUND (which is truthy if and only if find_package(SDL2 REQUIRED) was used successfully) and made it use those variables instead.

Note: I noticed SDL2_image also uses -DDLL_EXPORT for shared libraries on Windows; since I don't have a Windows machine nearby at the moment, I couldn't check whether it was related to the was SDL2 is compiled or not, and I didn't add it in this PR. Testing dynamic linking on Windows might be necessary before merging this PR.

• Use 'SDL2IMAGE_' prefix for all options
• Create SDL2_image::SDL2_image when building a shared library, create SDL2_image::SDL2_image-static when building a static library
• Aim is to use same CMake layout as SDL_ttf/SDL_mixer
• Create libSDL2_imaged.so symbolic link ('d' suffix) (when building debug build type)
• Add PrivateSdlFunctions.cmake script for common functionality between SDL_ttf/SDL_mixer
• Add FindXXX.cmake scripts for dependencies that don't provide cmake configuration files
• Bring jpeg/png saving support to cmake (#235)
• Update the git submodules
• Test CMake on mingw64 (using system dependencies there)
• Add MacOS CI
• Create symbolic link at build time + install it
• Only install SDL2_image.pc when building a shared SDL2_image (to avoid overwriting when later installing a static SDL2_image)
• Allow parallel installation of cmake support of a static and shared SDL2_image
• Add a single SDL2IMAGE_VENDORED option to use vendored libraries for everything, or none at all.
• Add Macos dylib versioning

I haven't added these scripts to EXTRA_DIST in Makefile.am yet, to allow some maturing + extra tests.

Need review on:

• ease of use: the cmake recipe is strict with its configuration options. When an option is configured to be enabled (-DSDL2IMAGE_xxx=ON), the cmake script will complain loudly instead of silently ignoring the option.
• vendored libraries: build these as shared or static libraries? Always building these as static libraries makes perhaps most sense.

Hi everyone,

Just testing out the new SDL2 releases with PySDL2 and noticed in the unit tests that WebP support has been removed from the official macOS release .framework. However, the changelog doesn't indicate that it was removed and the WebP .dll is still found in the "optional" folder for the Windows binaries. Was this a deliberate change or accidental?

Just wondering whether I should remove the unit test for WebP support on macOS or wait for a 2.6.1.

Also, sorry I didn't test this sooner before the official release! I've been busy with a new internship that eaten up a lot of my free time.

Best,

• Austin

Mainstream miniz received many updates: Do we want to upgrade?

Similar question for nanosvg, which we have two or three commits missing compared to mainstream.

This isn't mentioned in the header file and it returns a type that callers cannot understand without knowing internal implementation details, so I assume it was meant to be private to SDL_image.

With SDL_image 2.6.0, not all png types are supported, as greyscale images fail to load with stb_image.

Example images that fail: 16levelstriping.png: PNG image data, 512 x 512, 4-bit grayscale, non-interlaced 16levelstriping

capturedfuzz.png: PNG image data, 512 x 512, 8-bit grayscale, non-interlaced capturedfuzz

A workaround solution is to add --disable-stb-image to the configure flags in SDL_image so that full png support is used via libpng, although this doesn't seem to be set as default in some distributions package builds, which is problematic.

This is pretty minor, but I think it's worth reporting regardless: with the new STB PNG/JPG back end, reading in a JPEG image results in IMG_GetError() being populated with "Not a PNG" despite the image loading successfully (even with IMG_LoadJPEG_RW).

Not sure if it's a quirk with how stb_image identifies file formats or not, but a one-line fix would presumably be to just always call IMG_ClearError right before an image is successfully returned to avoid any confusion (e.g. encountering a different problem later that doesn't set an error, and getting a head-scratching "Not a PNG" when checking GetError for information). Would that approach work, or are there legitimate cases where you'd want an error set while still returning a valid image surface?

Thanks in advance!