Texture2D.SaveAsPng crashes with small sizes

[elisee]

When saving a very small PNG (2x2 size for instance), the number of bytes allocated for pngOut (https://github.com/flibitijibibo/FNA/blob/master/src/Graphics/Texture2D.cs#L453) might be 2 * 2 * 4 = 16 bytes but from my understanding, even the PNG header/footer are bigger than that, and just below we end up indexing into pngOut[33].

Maybe:

byte[] pngOut = new byte[width * height * 4]; // Max image size

Should be:

byte[] pngOut = new byte[width * height * 4 + 41 + 57]; // Max image size

? Assuming the header/footer sizes in the comment are correct and fixed in size.

[flibitijibibo]

https://github.com/flibitijibibo/FNA/commit/709d6e131e9997a3e62b30191e404c115217e34c

The max output works for larger images since it'll probably compress enough to fit within that space, but super small images won't compress as well. Assuming there's not additional space we need to consider, this should be fixed.

[elisee]

I found out it still crashes with some small textures on:

stream.Write(pngOut, 0, size);

For 1x1: size is 114 but pngOut.Length 102 For 2x2: size is 112 but pngOut.Length is 106 (the same color twice, somehow the size is actually smaller than for a 1x1 image. Compression magic!) For 2x2: size is 118 but pngOut.Length is 106 (two different colors) For 1x3: size is 122 but pngOut.Length is 110 For 1x4: size is 124 but pngOut.Length is 114 For 1x5: size is 130 but pngOut.Length is 118

Obviously this varies from image to image since some compress better than others

I fixed it on my end by adding a Math.Max( 200, ... ) when creating pngOut. A bit arbitrary, but hey it works.

EDIT: Previously I had tried a lower value (120) but I've now seen it grow to 130 bytes so to be safe I bumped it to 200 bytes.

[flibitijibibo]

There's probably some lower-bound we're not looking at in the zlib compression. This is where the surface pixels are packed in the RWops call:

https://hg.libsdl.org/SDL_image/file/2b0ada991468/IMG_png.c#l621

And this is the miniz function:

https://hg.libsdl.org/SDL_image/file/2b0ada991468/miniz.h#l2846

If there's some lower bound we need to care about, I wouldn't mind a Math.Min. We just need to cite the min with real code though.

EDIT: Here's the actual block that performs the compression in that miniz function:

https://hg.libsdl.org/SDL_image/file/2b0ada991468/miniz.h#l2826

[flibitijibibo]

Pulled the max alloc size found within the miniz compressor, this diff is the result:

http://www.flibitijibibo.com/pngSize.diff

Can you try this on those image sizes?

[flibitijibibo]

I added more padding to the pngOut alloc:

https://github.com/flibitijibibo/FNA/commit/5c22c68b1c85edd32edae20e2b58f5f865b83a5a

Unless you find more cases this doesn't cover, this should be done for now.

The actual root of this problem is that IMG_SavePNG requires that we alloc first, rather than simply giving us the memory that miniz allocates anyway. We could possibly call miniz directly ourselves and try to free() it manually, but perhaps it'd be more productive to make single-file PNG/JPEG encoders for C# and work with the MonoGame team on this:

https://github.com/mono/MonoGame/issues/2424

I dunno any good JPEG decoders other than stb_image's and I don't know of any single-file encoders (because, well, it's JPEG), but converting LodePNG would be my pick for encoding/decoding PNG data:

http://lodev.org/lodepng/

[elisee]

http://www.flibitijibibo.com/pngSize.diff

Can you try this on those image sizes?

It crashes with 1x1 and 1x2 at least. Shouldn't it include the header/footer size too?

Anyway using the padding instead, it seems to hold up.

[flibitijibibo]

I think the miniz default malloc size is for something even smaller like a 1x1 GL_LUMINANCE image, and it just does 1 or 2 remallocs for images between there and ~200-byte images.

Will close for now; replacing this with direct miniz or standalone encoders/decoders is a separate thing.