syoyo / tinygltf

Header only C++11 tiny glTF 2.0 library
MIT License
1.94k stars 395 forks source link

as_is flag of images is ignored during writing #487

Closed DanielLorenzLaubwerk closed 4 days ago

DanielLorenzLaubwerk commented 1 month ago

Describe the issue

The tinygltf::Image class contains an as_is variable, that indicates that the image data contains an encoded image. The declaration of as as_is variable says:

 // When this flag is true, data is stored to `image` in as-is format(e.g. jpeg
 // compressed for "image/jpeg" mime)

However, when I store a encoded/compressed image and set the as_is flag to true, WriteGltfSceneToFile() crashes. As far as I can see, the writer ignores the as_is flag completely and tries to encode the already compressed data anew.

To Reproduce

int main() { // Load an image tinygltf::Image image; std::string image_path = "example_image.png"; unsigned char *raw_image = stbi_load(image_path.c_str(), &image.width, &image.height, &image.component, 0);

// Encode it as png
int encoded_length;
unsigned char *encoded_image = stbi_write_png_to_mem(raw_image, 0, image.width, image.height, image.component, &encoded_length);

// Store it in a tinygltf image as_is
image.image.resize(encoded_length);
memcpy(&image.image[0], encoded_image, encoded_length);
image.as_is = true;
image.mimeType = "image/png";
image.name = "example_image";
image.bits = 8;
image.pixel_type = TINYGLTF_COMPONENT_TYPE_UNSIGNED_BYTE;

// Write scene
tinygltf::Model model;
model.images.push_back(image);
tinygltf::TinyGLTF writer;
writer.WriteGltfSceneToFile(&model,
                            "example.gltf",
                            true, /* embed images */
                            true, /* embed buffers */
                            true, /* pretty */
                            false); /* binary */

}



**Expected behaviour**

I would expect that the writer would not try to compress the image data if `as_is` is set to true, but write the data as it is. (It might encode it base64, though if it puts it into the URI)
syoyo commented 1 month ago

Good find!

as_is flag is somewhat hidden feature and not considered in WriteGltfSceneToFile, so this is the issue.

We'll probably need to bypass WriteImageData function when as_is is set to true:

https://github.com/syoyo/tinygltf/blob/f03fe2657971332916533c03d0c8c751972df9ee/tiny_gltf.h#L3233

something like...


if (!filename.empty() && !image.image.empty()) {
  if (image.as_is) {
    // serialize to buffer.
  } else if (*WriteImageData != nullptr) {
    call WriteImageData(...)
 }
}
DanielLorenzLaubwerk commented 1 month ago

Yes, that looks good.

syoyo commented 1 month ago

You can contribute. PR is much appreciated!

ptc-tgamper commented 6 days ago

I'll take this one. Working in this area of the code anyways.

syoyo commented 6 days ago

🙏

tdapper commented 6 days ago

Cool, let us know if and when we can help testing anything, @ptc-tgamper!

syoyo commented 6 days ago

@ptc-tgamper Would it possible to add this fix to this PR https://github.com/syoyo/tinygltf/pull/491 ? It'd be convenient to add as_is flag fixes to a single PR.

ptc-tgamper commented 6 days ago

I can, if you prefer it that way.

syoyo commented 6 days ago

Thanks! Yes I prefer it, since I'll bump the minor version of tinygltf once PR #491 has been merged. Putting 'as-is' flag fix to #491 will save redundant version increments