upgrade to newer miniz from mainstream?

[sezero]

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.

[sezero]

OK, will try messing with it tomorrow.

[icculus]

Someone else appears to have taken over maintaining miniz from Rich Geldreich, and relicensed it to be MIT instead of public domain:

richgel999/miniz@224d207

So we don't have to add MIT to our license, we'd need to either delete the zip64 code (we only need zlib decompression I assume, not any zip support), or use a version before they were added.

[icculus]

(To be clear, Rich probably added the zip64 code when personally working on vogl at Valve, not whomever the new person is.)

[sezero]

OK, I'll cancel this one.

There are a lot of warnings from miniz.h (-Wmisleading-indentation mostly, IIRC), will fix them later.

[sezero]

OK, I silenced the majority of the warnings. Only the following are left:

1. strict-aliasing warnings I get from gcc4.9:

In file included from IMG_png.c:699:0:
miniz.h: In function 'tdefl_find_match':
miniz.h:2360:3: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
   mz_uint16 c01 = TDEFL_READ_UNALIGNED_WORD(&d->m_dict[pos + match_len - 1]), s01 = TDEFL_READ_UNALIGNED_WORD(s);
   ^
miniz.h:2372:7: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
       TDEFL_PROBE;
       ^
miniz.h:2373:7: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
       TDEFL_PROBE;
       ^
miniz.h:2374:7: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
       TDEFL_PROBE;
       ^
miniz.h:2389:7: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
       c01 = TDEFL_READ_UNALIGNED_WORD(&d->m_dict[pos + match_len - 1]);
       ^
miniz.h: In function 'tdefl_compress_fast':
miniz.h:2464:7: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
       if (((cur_match_dist = (mz_uint16)(lookahead_pos - probe_pos)) <= dict_size) && ((*(const mz_uint32 *)(d->m_dict + (probe_pos &= TDEFL_LZ_DICT_SIZE_MASK)) & 0xFFFFFF) == first_trigram))
       ^

I can silence them by disabling unaligned stores, like the following. OK?

diff --git a/IMG_png.c b/IMG_png.c
index b5c6082..6ab0aa7 100644
--- a/IMG_png.c
+++ b/IMG_png.c
@@ -695,6 +695,7 @@ static int IMG_SavePNG_RW_libpng(SDL_Surface *surface, SDL_RWops *dst, int freed
 #else
 #define MINIZ_LITTLE_ENDIAN 0
 #endif
+#define MINIZ_USE_UNALIGNED_LOADS_AND_STORES 0
 #define MINIZ_SDL_NOUNUSED
 #include "miniz.h"
 
diff --git a/miniz.h b/miniz.h
index 76f51cb..b213c4e 100644
--- a/miniz.h
+++ b/miniz.h
@@ -230,10 +230,12 @@
 #endif
 #endif /**/
 
+#ifndef MINIZ_USE_UNALIGNED_LOADS_AND_STORES
 #if MINIZ_X86_OR_X64_CPU
 // Set MINIZ_USE_UNALIGNED_LOADS_AND_STORES to 1 on CPU's that permit efficient integer loads and stores from unaligned addresses.
 #define MINIZ_USE_UNALIGNED_LOADS_AND_STORES 1
 #endif
+#endif /**/
 
 #if defined(_M_X64) || defined(_WIN64) || defined(__MINGW64__) || defined(_LP64) || defined(__LP64__) || defined(__ia64__) || defined(__x86_64__)
 // Set MINIZ_HAS_64BIT_REGISTERS to 1 if operations on 64-bit integers are reasonably fast (and don't involve compiler generated calls to helper functions).

2. unused-function warnings from stb_image.h:

In file included from IMG_stb.c:55:0:
stb_image.h:1231:18: warning: 'stbi_load_from_memory' defined but not used [-Wunused-function]
 STBIDEF stbi_uc *stbi_load_from_memory(stbi_uc const *buffer, int len, int *x, int *y, int *comp, int req_comp)
                  ^
stb_image.h:1217:18: warning: 'stbi_load_16_from_memory' defined but not used [-Wunused-function]
 STBIDEF stbi_us *stbi_load_16_from_memory(stbi_uc const *buffer, int len, int *x, int *y, int *channels_in_file, int desired_channels)
                  ^
stb_image.h:1224:18: warning: 'stbi_load_16_from_callbacks' defined but not used [-Wunused-function]
 STBIDEF stbi_us *stbi_load_16_from_callbacks(stbi_io_callbacks const *clbk, void *user, int *x, int *y, int *channels_in_file, int desired_channels)
                  ^
stb_image.h:1361:18: warning: 'stbi_is_hdr_from_callbacks' defined but not used [-Wunused-function]
 STBIDEF int      stbi_is_hdr_from_callbacks(stbi_io_callbacks const *clbk, void *user)
                  ^
stb_image.h:1319:13: warning: 'stbi_is_hdr_from_memory' defined but not used [-Wunused-function]
 STBIDEF int stbi_is_hdr_from_memory(stbi_uc const *buffer, int len)
             ^
stb_image.h:7220:13: warning: 'stbi_info_from_memory' defined but not used [-Wunused-function]
 STBIDEF int stbi_info_from_memory(stbi_uc const *buffer, int len, int *x, int *y, int *comp)
             ^
stb_image.h:7227:13: warning: 'stbi_info_from_callbacks' defined but not used [-Wunused-function]
 STBIDEF int stbi_info_from_callbacks(stbi_io_callbacks const *c, void *user, int *x, int *y, int *comp)
             ^
stb_image.h:7234:13: warning: 'stbi_is_16_bit_from_memory' defined but not used [-Wunused-function]
 STBIDEF int stbi_is_16_bit_from_memory(stbi_uc const *buffer, int len)
             ^
stb_image.h:7241:13: warning: 'stbi_is_16_bit_from_callbacks' defined but not used [-Wunused-function]
 STBIDEF int stbi_is_16_bit_from_callbacks(stbi_io_callbacks const *c, void *user)
             ^
stb_image.h:4687:14: warning: 'stbi_set_unpremultiply_on_load' defined but not used [-Wunused-function]
 STBIDEF void stbi_set_unpremultiply_on_load(int flag_true_if_should_unpremultiply)
              ^
stb_image.h:4692:14: warning: 'stbi_convert_iphone_png_to_rgb' defined but not used [-Wunused-function]
 STBIDEF void stbi_convert_iphone_png_to_rgb(int flag_true_if_should_convert)
              ^
stb_image.h:976:14: warning: 'stbi_set_flip_vertically_on_load' defined but not used [-Wunused-function]
 STBIDEF void stbi_set_flip_vertically_on_load(int flag_true_if_should_flip)
              ^
stb_image.h:4205:15: warning: 'stbi_zlib_decode_malloc' defined but not used [-Wunused-function]
 STBIDEF char *stbi_zlib_decode_malloc(char const *buffer, int len, int *outlen)
               ^
stb_image.h:4226:13: warning: 'stbi_zlib_decode_buffer' defined but not used [-Wunused-function]
 STBIDEF int stbi_zlib_decode_buffer(char *obuffer, int olen, char const *ibuffer, int ilen)
             ^
stb_image.h:4237:15: warning: 'stbi_zlib_decode_noheader_malloc' defined but not used [-Wunused-function]
 STBIDEF char *stbi_zlib_decode_noheader_malloc(char const *buffer, int len, int *outlen)
               ^
stb_image.h:4253:13: warning: 'stbi_zlib_decode_noheader_buffer' defined but not used [-Wunused-function]
 STBIDEF int stbi_zlib_decode_noheader_buffer(char *obuffer, int olen, const char *ibuffer, int ilen)
             ^
stb_image.h:1082:13: warning: 'stbi__vertical_flip_slices' defined but not used [-Wunused-function]
 static void stbi__vertical_flip_slices(void *image, int w, int h, int z, int bytes_per_pixel)
             ^

A quick solution would be marking all of those with SDL_UNUSED to pacify compiler.
Or, I can #if 0 them out somehow.

@slouken, @icculus: Waiting for your decisions.

[slouken]

For files that we're likely to pull changes from upstream I'd be inclined to minimize changes to make it easy to accept updates in the future. Even for the misleading whitespace changes you made, I'd prefer to silence the warnings using a pragma outside the header or at the top of the header rather than change the affected code sites. If we think fixing the warnings is important, I'd rather submit an upstream PR and cherry-pick that when it's accepted.

@icculus, what are your thoughts?

[icculus]

I think you're right in general, but I'm going to ask the miniz guy if he'll remove the license change if I write the zip64 code from scratch, because this is a dumb thing to have to workaround in the first place.

[sezero]

Why do we even need the zip64 code in the first place??

I think my first instinct about cancelling the upgrade to new miniz was right: We don't
need many parts of it. Therefore, fixing the warnings here in the old version should
be OK. (And there is nothing to submit to mainstream miniz: No such warnings there.)

[sezero]

I pushed the MINIZ_USE_UNALIGNED_LOADS_AND_STORES patch for miniz,
because it's what the mainstream does by default recently, anyway.

As for stb_image: Waiting your decisions. (BTW: our stb_image is behind mainstream,
but whatever.)

[icculus]

Why do we even need the zip64 code in the first place??

We don't; but if that's why they changed the license, it would be nice to get it back to public domain so we don't have to maintain a fork.