[TODO] Support 16bit PNG write

[syoyo]

It is useful to support writing 16bit PNG for:

• gltfutil(texture dumper)
• Serialize glTF

Extending stb_image_write looks rather complex, so our bet is using LodePNG.

[syoyo]

Created a branch. https://github.com/syoyo/tinygltf/tree/16bit-lodepng

16 bit PNG image(e.g. Normal map in AntiqueCamera) generated by gltfutil is wrong

[Ybalrid]

I agree with you, stb_image_write, as its author says is just a "quick and dirty" tool https://github.com/nothings/stb/issues/605#issuecomment-434913282

For LodePNG, I am unsure, don't you think it would be valuable to find a library that doesn't support "only" PNG images for serializing glTF files? Or do you want to use LodePNG only in the case that we are writing out PNG files, and rely in stb_write for the other formats like we do today?

[syoyo]

Normal map generated by gltfutil is wrong

The reason was lodepng::encode expects bit endian image data. I got a proper result by reordering pixel data into big endian manner: https://github.com/syoyo/tinygltf/commit/758a1240c938e2c644936924abdd9909ab4704c9

[syoyo]

@Ybalrid I have added 16bit PNG write feature using LodePNG(and also write to OpenEXR format using tinyexr as a bonus). Please take a look https://github.com/syoyo/tinygltf/tree/16bit-lodepng

Now read and write looks working well. Will merge it into master in the near future.

[Ybalrid]

I see what you did.

It's not super important, but there's maybe some slightly more efficient manner to byte-swap these from little-endian to big-endian using bitwise operators and bitshifts

e.g: if buffer here is an array of unsigned shorts (or just an usigned short* with the address of the start of the byte array)

for(size_t i = 0; i < buffer.size(); ++i)
    buffer[i] = ((0xFF00 & buffer[i]) >> 8) | ((0x00FF & buffer[i]) << 8);

If each numbers starts like high byte; low byte this will put the low byte into the high byte and the high byte into the low byte, and perform an OR between low byte; 0 and 0; high byte -> low byte; high byte.

No need to have a temporary variable of call to a function (lambda) doing so

[Ybalrid]

(also the tinyexr header commited into that branch doesn't have a "SaveEXR" function that takes a boolean to tell if we are 16bit)

[Ybalrid]

diff --git a/examples/gltfutil/texture_dumper.cc b/examples/gltfutil/texture_dumper.cc
index c46bc70..ff91e1b 100644
--- a/examples/gltfutil/texture_dumper.cc
+++ b/examples/gltfutil/texture_dumper.cc
@@ -44,21 +44,11 @@ static void ToBigEndian(std::vector<uint8_t>* image) {
     return;
   }

-  auto swap2 =
-      [](uint16_t* val) {
-        uint16_t tmp = *val;
-        uint8_t* dst = reinterpret_cast<uint8_t*>(val);
-        uint8_t* src = reinterpret_cast<uint8_t*>(&tmp);
-
-        dst[0] = src[1];
-        dst[1] = src[0];
-      };
-
   uint16_t *ptr = reinterpret_cast<uint16_t *>(image->data());
   size_t n = image->size() / 2;

   for (size_t i = 0; i < n; i++) {
-    swap2(&ptr[i]);
+    ptr[i] = ((0xFF00 & ptr[i]) >> 8) | ((0x00FF & ptr[i]) << 8);
   }
 }

You can put the in a textfile.patch and run "git apply-path" on it if you want, or I can PR this change against the 16bit-lodepng branch if you prefer.

[Ybalrid]

I've created a PR against the 16bit-lodepng branch

[syoyo]

Merged! > I've created a PR against the 16bit-lodepng branch

(also the tinyexr header commited into that branch doesn't have a "SaveEXR" function that takes a boolean to tell if we are 16bit)

Sorry, I have forgotten to commit tinyexr.h. Now it should be ok: https://github.com/syoyo/tinygltf/commit/8fd91aea04510808885f1a05e28a84fb67c4c999

[Ybalrid]

So, as of now we have code that can save a 16bit png, or a 16bit EXR image inside the gltfutil program. Do we have this functionality working for serializing glTF to disk in tiny_gltf.h ?

If not, that's the next thing to add IMHO.

[syoyo]

Do we have this functionality working for serializing glTF to disk in tiny_gltf.h ?

Not yet. PR is appreciated!