Merge pull request #491 from ptc-tgamper/bug/image_saving

Fix images not being saved due to missing filesystem callbacks

tests/tester.cc

@@ -474,7 +474,7 @@ TEST_CASE("image-uri-spaces", "[issue-236]") {
     }
     REQUIRE(true == ret);
     REQUIRE(err.empty());
-    REQUIRE(!warn.empty());  // relative image path won't exist in tests/
+    REQUIRE(warn.empty());
     REQUIRE(saved.images.size() == model.images.size());
 
     // The image uri in CubeImageUriMultipleSpaces.gltf is not encoded and
@@ -662,10 +662,11 @@ TEST_CASE("serialize-image-callback", "[issue-394]") {
 
   auto writer = [](const std::string *basepath, const std::string *filename,
                    const tinygltf::Image *image, bool embedImages,
-                   const tinygltf::URICallbacks *uri_cb, std::string *out_uri,
+                   const tinygltf::FsCallbacks* fs, const tinygltf::URICallbacks *uri_cb,
-                   void *user_pointer) -> bool {
+                   std::string *out_uri, void *user_pointer) -> bool {
     (void)basepath;
     (void)image;
+    (void)fs;
     (void)uri_cb;
     REQUIRE(*filename == "foo");
     REQUIRE(embedImages == true);
@@ -699,12 +700,13 @@ TEST_CASE("serialize-image-failure", "[issue-394]") {
 
   auto writer = [](const std::string *basepath, const std::string *filename,
                    const tinygltf::Image *image, bool embedImages,
-                   const tinygltf::URICallbacks *uri_cb, std::string *out_uri,
+                   const tinygltf::FsCallbacks* fs, const tinygltf::URICallbacks *uri_cb,
-                   void *user_pointer) -> bool {
+                   std::string *out_uri, void *user_pointer) -> bool {
     (void)basepath;
     (void)filename;
     (void)image;
     (void)embedImages;
+    (void)fs;
     (void)uri_cb;
     (void)out_uri;
     (void)user_pointer;
@@ -1056,3 +1058,127 @@ TEST_CASE("serialize-lods", "[lods]") {
     CHECK(nodeWithoutLods.extensions.count("MSFT_lod") == 0);
   }
 }
+
+TEST_CASE("write-image-issue", "[issue-473]") {
+  std::string err;
+  std::string warn;
+  tinygltf::Model model;
+  tinygltf::TinyGLTF ctx;
+  bool ok = ctx.LoadASCIIFromFile(&model, &err, &warn, "../models/Cube/Cube.gltf");
+  REQUIRE(ok);
+  REQUIRE(err.empty());
+  REQUIRE(warn.empty());
+
+  REQUIRE(model.images.size() == 2);
+  REQUIRE(model.images[0].uri == "Cube_BaseColor.png");
+  REQUIRE(model.images[1].uri == "Cube_MetallicRoughness.png");
+
+  REQUIRE_FALSE(model.images[0].image.empty());
+  REQUIRE_FALSE(model.images[1].image.empty());
+
+  ok = ctx.WriteGltfSceneToFile(&model, "Cube.gltf");
+  REQUIRE(ok);
+
+  for (const auto& image : model.images) {
+    std::fstream file(image.uri);
+    CHECK(file.good());
+  }
+}
+
+TEST_CASE("images-as-is", "[issue-487]") {
+  std::string err;
+  std::string warn;
+  tinygltf::Model model;
+  tinygltf::TinyGLTF ctx;
+  ctx.SetImagesAsIs(true);
+  bool ok = ctx.LoadASCIIFromFile(&model, &err, &warn, "../models/Cube/Cube.gltf");
+  REQUIRE(ok);
+  REQUIRE(err.empty());
+  REQUIRE(warn.empty());
+
+  for (const auto& image : model.images) {
+    CHECK(image.as_is == true);
+    CHECK_FALSE(image.uri.empty());
+    CHECK_FALSE(image.image.empty());
+
+#ifndef TINYGLTF_NO_STB_IMAGE
+    // Make sure we can decode the images
+    int w = -1, h = -1, component = -1;
+    unsigned char *data = stbi_load_from_memory(image.image.data(), static_cast<int>(image.image.size()), &w, &h, &component, 0);
+    CHECK(data != nullptr);
+    CHECK(w == 512);
+    CHECK(h == 512);
+    CHECK(component >= 3);
+    stbi_image_free(data);
+#endif
+  }
+
+  // Write glTF model to disk, and images as separate files
+  {
+    ok = ctx.WriteGltfSceneToFile(&model, "Cube_with_image_files.gltf");
+    REQUIRE(ok);
+
+    // All the images should have been written to disk with their original data
+    for (const auto& image : model.images)  {
+      // Make sure the image files exist
+      std::fstream file(image.uri);
+      CHECK(file.good());
+#ifndef TINYGLTF_NO_STB_IMAGE
+      // Make sure we can load the images
+      int w = -1, h = -1, component = -1;
+      unsigned char *data = stbi_load(image.uri.c_str(), &w, &h, &component, 0);
+      CHECK(data != nullptr);
+      CHECK(w == 512);
+      CHECK(h == 512);
+      CHECK(component >= 3);
+      stbi_image_free(data);
+#endif
+    }
+  }
+
+  // Write glTF model to disk, and embed images as data URIs
+  {
+    ok = ctx.WriteGltfSceneToFile(&model, "Cube_with_embedded_images.gltf", true, false);
+    REQUIRE(ok);
+
+    // Load above model again, and check if the images are loaded properly
+    tinygltf::Model embeddedImages;
+    ctx.SetImagesAsIs(false);
+    bool ok = ctx.LoadASCIIFromFile(&embeddedImages, &err, &warn, "Cube_with_embedded_images.gltf");
+    REQUIRE(ok);
+    REQUIRE(err.empty());
+    REQUIRE(warn.empty());
+
+    for (const auto& image : embeddedImages.images) {
+      CHECK(image.as_is == false);
+      CHECK_FALSE(image.mimeType.empty());
+      CHECK_FALSE(image.image.empty());
+      CHECK(image.width == 512);
+      CHECK(image.height == 512);
+      CHECK(image.component >= 3);
+    }
+  }
+
+  // Write glTF model to disk, as GLB
+  {
+    ok = ctx.WriteGltfSceneToFile(&model, "Cube.glb", true, true, true, true);
+    REQUIRE(ok);
+
+    // Load above model again, and check if the images are loaded properly
+    tinygltf::Model glbModel;
+    ctx.SetImagesAsIs(false);
+    bool ok = ctx.LoadBinaryFromFile(&glbModel, &err, &warn, "Cube.glb");
+    REQUIRE(ok);
+    REQUIRE(err.empty());
+    REQUIRE(warn.empty());
+
+    for (const auto& image : glbModel.images) {
+      CHECK(image.as_is == false);
+      CHECK_FALSE(image.mimeType.empty());
+      CHECK_FALSE(image.image.empty());
+      CHECK(image.width == 512);
+      CHECK(image.height == 512);
+      CHECK(image.component >= 3);
+    }
+  }
+}

tiny_gltf.h

@@ -649,9 +649,7 @@ struct Image {
   // When this flag is true, data is stored to `image` in as-is format(e.g. jpeg
   // compressed for "image/jpeg" mime) This feature is good if you use custom
   // image loader function. (e.g. delayed decoding of images for faster glTF
-  // parsing) Default parser for Image does not provide as-is loading feature at
+  // parsing).
-  // the moment. (You can manipulate this by providing your own LoadImageData
-  // function)
   bool as_is{false};
 
   Image() = default;
@@ -1292,39 +1290,6 @@ struct URICallbacks {
   void *user_data;  // An argument that is passed to all uri callbacks
 };
 
-///
-/// LoadImageDataFunction type. Signature for custom image loading callbacks.
-///
-using LoadImageDataFunction = std::function<bool(
-    Image * /* image */, const int /* image_idx */, std::string * /* err */,
-    std::string * /* warn */, int /* req_width */, int /* req_height */,
-    const unsigned char * /* bytes */, int /* size */, void * /*user_data */)>;
-
-///
-/// WriteImageDataFunction type. Signature for custom image writing callbacks.
-/// The out_uri parameter becomes the URI written to the gltf and may reference
-/// a file or contain a data URI.
-///
-using WriteImageDataFunction = std::function<bool(
-    const std::string * /* basepath */, const std::string * /* filename */,
-    const Image *image, bool /* embedImages */,
-    const URICallbacks * /* uri_cb */, std::string * /* out_uri */,
-    void * /* user_pointer */)>;
-
-#ifndef TINYGLTF_NO_STB_IMAGE
-// Declaration of default image loader callback
-bool LoadImageData(Image *image, const int image_idx, std::string *err,
-                   std::string *warn, int req_width, int req_height,
-                   const unsigned char *bytes, int size, void *);
-#endif
-
-#ifndef TINYGLTF_NO_STB_IMAGE_WRITE
-// Declaration of default image writer callback
-bool WriteImageData(const std::string *basepath, const std::string *filename,
-                    const Image *image, bool embedImages,
-                    const URICallbacks *uri_cb, std::string *out_uri, void *);
-#endif
-
 ///
 /// FileExistsFunction type. Signature for custom filesystem callbacks.
 ///
@@ -1396,6 +1361,40 @@ bool GetFileSizeInBytes(size_t *filesize_out, std::string *err,
                         const std::string &filepath, void *);
 #endif
 
+///
+/// LoadImageDataFunction type. Signature for custom image loading callbacks.
+///
+using LoadImageDataFunction = std::function<bool(
+    Image * /* image */, const int /* image_idx */, std::string * /* err */,
+    std::string * /* warn */, int /* req_width */, int /* req_height */,
+    const unsigned char * /* bytes */, int /* size */, void * /*user_data */)>;
+
+///
+/// WriteImageDataFunction type. Signature for custom image writing callbacks.
+/// The out_uri parameter becomes the URI written to the gltf and may reference
+/// a file or contain a data URI.
+///
+using WriteImageDataFunction = std::function<bool(
+    const std::string * /* basepath */, const std::string * /* filename */,
+    const Image *image, bool /* embedImages */,
+    const FsCallbacks * /* fs_cb */, const URICallbacks * /* uri_cb */,
+    std::string * /* out_uri */, void * /* user_pointer */)>;
+
+#ifndef TINYGLTF_NO_STB_IMAGE
+// Declaration of default image loader callback
+bool LoadImageData(Image *image, const int image_idx, std::string *err,
+                   std::string *warn, int req_width, int req_height,
+                   const unsigned char *bytes, int size, void *);
+#endif
+
+#ifndef TINYGLTF_NO_STB_IMAGE_WRITE
+// Declaration of default image writer callback
+bool WriteImageData(const std::string *basepath, const std::string *filename,
+                    const Image *image, bool embedImages,
+                    const FsCallbacks* fs_cb, const URICallbacks *uri_cb,
+                    std::string *out_uri, void *);
+#endif
+
 ///
 /// glTF Parser/Serializer context.
 ///
@@ -1543,6 +1542,17 @@ class TinyGLTF {
     preserve_image_channels_ = onoff;
   }
 
+  bool GetPreserveImageChannels() const { return preserve_image_channels_; }
+
+  ///
+  /// Specifiy whether image data is decoded/decompressed during load, or left as is
+  ///
+  void SetImagesAsIs(bool onoff) {
+      images_as_is_ = onoff;
+  }
+
+  bool GetImagesAsIs() const { return images_as_is_; }
+
   ///
   /// Set maximum allowed external file size in bytes.
   /// Default: 2GB
@@ -1554,8 +1564,6 @@ class TinyGLTF {
 
   size_t GetMaxExternalFileSize() const { return max_external_file_size_; }
 
-  bool GetPreserveImageChannels() const { return preserve_image_channels_; }
-
  private:
   ///
   /// Loads glTF asset from string(memory).
@@ -1580,6 +1588,8 @@ class TinyGLTF {
   bool preserve_image_channels_ = false;  /// Default false(expand channels to
                                           /// RGBA) for backward compatibility.
 
+  bool images_as_is_ = false; /// Default false (decode/decompress images)
+
   size_t max_external_file_size_{
       size_t((std::numeric_limits<int32_t>::max)())};  // Default 2GB
 
@@ -1905,6 +1915,9 @@ struct LoadImageDataOption {
   // channels) default `false`(channels are expanded to RGBA for backward
   // compatibility).
   bool preserve_channels{false};
+  // true: do not decode/decompress image data.
+  // default `false`: decode/decompress image data.
+  bool as_is{false};
 };
 
 // Equals function for Value, for recursivity
@@ -2613,48 +2626,65 @@ bool LoadImageData(Image *image, const int image_idx, std::string *err,
 
   int w = 0, h = 0, comp = 0, req_comp = 0;
 
-  unsigned char *data = nullptr;
+  // Try to decode image header
+  if (!stbi_info_from_memory(bytes, size, &w, &h, &comp)) {
+    // On failure, if we load images as is, we just warn.
+    std::string* msgOut = option.as_is ? warn : err;
+    if (msgOut) {
+      (*msgOut) +=
+        "Unknown image format. STB cannot decode image header for image[" +
+        std::to_string(image_idx) + "] name = \"" + image->name + "\".\n";
+    }
+    if (!option.as_is) {
+      // If we decode images, error out.
+      return false;
+    } else {
+      // If we load images as is, we copy the image data,
+      // set all image properties to invalid, and report success.
+      image->width = image->height = image->component = -1;
+      image->bits = image->pixel_type = -1;
+      image->image.resize(static_cast<size_t>(size));
+      std::copy(bytes, bytes + size, image->image.begin());
+      return true;
+    }
+  }
+
+  int bits = 8;
+  int pixel_type = TINYGLTF_COMPONENT_TYPE_UNSIGNED_BYTE;
+
+  if (stbi_is_16_bit_from_memory(bytes, size)) {
+    bits = 16;
+    pixel_type = TINYGLTF_COMPONENT_TYPE_UNSIGNED_SHORT;
+  }
 
   // preserve_channels true: Use channels stored in the image file.
   // false: force 32-bit textures for common Vulkan compatibility. It appears
   // that some GPU drivers do not support 24-bit images for Vulkan
-  req_comp = option.preserve_channels ? 0 : 4;
+  req_comp = (option.preserve_channels || option.as_is) ? 0 : 4;
-  int bits = 8;
-  int pixel_type = TINYGLTF_COMPONENT_TYPE_UNSIGNED_BYTE;
 
-  // It is possible that the image we want to load is a 16bit per channel image
+  unsigned char* data = nullptr;
-  // We are going to attempt to load it as 16bit per channel, and if it worked,
+  // Perform image decoding if requested
-  // set the image data accordingly. We are casting the returned pointer into
+  if (!option.as_is) {
-  // unsigned char, because we are representing "bytes". But we are updating
+    // If the image is marked as 16 bit per channel, attempt to decode it as such first.
-  // the Image metadata to signal that this image uses 2 bytes (16bits) per
+    // If that fails, we are going to attempt to load it as 8 bit per channel image.
-  // channel:
+    if (bits == 16) {
-  if (stbi_is_16_bit_from_memory(bytes, size)) {
+      data = reinterpret_cast<unsigned char *>(stbi_load_16_from_memory(bytes, size, &w, &h, &comp, req_comp));
-    data = reinterpret_cast<unsigned char *>(
-        stbi_load_16_from_memory(bytes, size, &w, &h, &comp, req_comp));
-    if (data) {
-      bits = 16;
-      pixel_type = TINYGLTF_COMPONENT_TYPE_UNSIGNED_SHORT;
     }
-  }
+    // Load as 8 bit per channel data
-
+    if (!data) {
-  // at this point, if data is still NULL, it means that the image wasn't
+      data = stbi_load_from_memory(bytes, size, &w, &h, &comp, req_comp);
-  // 16bit per channel, we are going to load it as a normal 8bit per channel
+      if (!data) {
-  // image as we used to do:
+        if (err) {
-  // if image cannot be decoded, ignore parsing and keep it by its path
+          (*err) +=
-  // don't break in this case
+            "Unknown image format. STB cannot decode image data for image[" +
-  // FIXME we should only enter this function if the image is embedded. If
+            std::to_string(image_idx) + "] name = \"" + image->name + "\".\n";
-  // image->uri references
+        }
-  // an image file, it should be left as it is. Image loading should not be
+        return false;
-  // mandatory (to support other formats)
+      }
-  if (!data) data = stbi_load_from_memory(bytes, size, &w, &h, &comp, req_comp);
+      // If we were succesful, mark as 8 bit
-  if (!data) {
+      bits = 8;
-    // NOTE: you can use `warn` instead of `err`
+      pixel_type = TINYGLTF_COMPONENT_TYPE_UNSIGNED_BYTE;
-    if (err) {
-      (*err) +=
-          "Unknown image format. STB cannot decode image data for image[" +
-          std::to_string(image_idx) + "] name = \"" + image->name + "\".\n";
     }
-    return false;
   }
 
   if ((w < 1) || (h < 1)) {
@@ -2700,10 +2730,20 @@ bool LoadImageData(Image *image, const int image_idx, std::string *err,
   image->component = comp;
   image->bits = bits;
   image->pixel_type = pixel_type;
-  image->image.resize(static_cast<size_t>(w * h * comp) * size_t(bits / 8));
+  image->as_is = option.as_is;
-  std::copy(data, data + w * h * comp * (bits / 8), image->image.begin());
-  stbi_image_free(data);
 
+  if (option.as_is) {
+    // Store the original image data
+    image->image.resize(static_cast<size_t>(size));
+    std::copy(bytes, bytes + size, image->image.begin());
+  }
+  else {
+    // Store the decoded image data
+    image->image.resize(static_cast<size_t>(w * h * comp) * size_t(bits / 8));
+    std::copy(data, data + w * h * comp * (bits / 8), image->image.begin());
+  }
+
+  stbi_image_free(data);
   return true;
 }
 #endif
@@ -2725,36 +2765,45 @@ static void WriteToMemory_stbi(void *context, void *data, int size) {
 
 bool WriteImageData(const std::string *basepath, const std::string *filename,
                     const Image *image, bool embedImages,
-                    const URICallbacks *uri_cb, std::string *out_uri,
+                    const FsCallbacks* fs_cb, const URICallbacks *uri_cb,
-                    void *fsPtr) {
+                    std::string *out_uri, void *) {
   const std::string ext = GetFilePathExtension(*filename);
 
   // Write image to temporary buffer
   std::string header;
   std::vector<unsigned char> data;
 
-  if (ext == "png") {
+  // If the image data is already encoded, take it as is
-    if ((image->bits != 8) ||
+  if (image->as_is) {
-        (image->pixel_type != TINYGLTF_COMPONENT_TYPE_UNSIGNED_BYTE)) {
+      data = image->image;
-      // Unsupported pixel format
+  }
-      return false;
-    }
 
-    if (!stbi_write_png_to_func(WriteToMemory_stbi, &data, image->width,
+  if (ext == "png") {
-                                image->height, image->component,
+    if (!image->as_is) {
-                                &image->image[0], 0)) {
+      if ((image->bits != 8) ||
-      return false;
+          (image->pixel_type != TINYGLTF_COMPONENT_TYPE_UNSIGNED_BYTE)) {
+          // Unsupported pixel format
+          return false;
+      }
+
+      if (!stbi_write_png_to_func(WriteToMemory_stbi, &data, image->width,
+                                  image->height, image->component,
+                                  &image->image[0], 0)) {
+        return false;
+      }
     }
     header = "data:image/png;base64,";
   } else if (ext == "jpg") {
-    if (!stbi_write_jpg_to_func(WriteToMemory_stbi, &data, image->width,
+    if (!image->as_is &&
+        !stbi_write_jpg_to_func(WriteToMemory_stbi, &data, image->width,
                                 image->height, image->component,
                                 &image->image[0], 100)) {
       return false;
     }
     header = "data:image/jpeg;base64,";
   } else if (ext == "bmp") {
-    if (!stbi_write_bmp_to_func(WriteToMemory_stbi, &data, image->width,
+    if (!image->as_is &&
+        !stbi_write_bmp_to_func(WriteToMemory_stbi, &data, image->width,
                                 image->height, image->component,
                                 &image->image[0])) {
       return false;
@@ -2775,12 +2824,11 @@ bool WriteImageData(const std::string *basepath, const std::string *filename,
     }
   } else {
     // Write image to disc
-    FsCallbacks *fs = reinterpret_cast<FsCallbacks *>(fsPtr);
+    if ((fs_cb != nullptr) && (fs_cb->WriteWholeFile != nullptr)) {
-    if ((fs != nullptr) && (fs->WriteWholeFile != nullptr)) {
       const std::string imagefilepath = JoinPath(*basepath, *filename);
       std::string writeError;
-      if (!fs->WriteWholeFile(&writeError, imagefilepath, data,
+      if (!fs_cb->WriteWholeFile(&writeError, imagefilepath, data,
-                              fs->user_data)) {
+                              fs_cb->user_data)) {
         // Could not write image file to disc; Throw error ?
         return false;
       }
@@ -3233,6 +3281,7 @@ static std::string MimeToExt(const std::string &mimeType) {
 
 static bool UpdateImageObject(const Image &image, std::string &baseDir,
                               int index, bool embedImages,
+                              const FsCallbacks *fs_cb,
                               const URICallbacks *uri_cb,
                               const WriteImageDataFunction& WriteImageData,
                               void *user_data, std::string *out_uri) {
@@ -3266,7 +3315,7 @@ static bool UpdateImageObject(const Image &image, std::string &baseDir,
   bool imageWritten = false;
   if (WriteImageData != nullptr && !filename.empty() && !image.image.empty()) {
     imageWritten = WriteImageData(&baseDir, &filename, &image, embedImages,
-                                  uri_cb, out_uri, user_data);
+                                  fs_cb, uri_cb, out_uri, user_data);
     if (!imageWritten) {
       return false;
     }
@@ -6359,6 +6408,7 @@ bool TinyGLTF::LoadFromString(Model *model, std::string *err, std::string *warn,
     load_image_user_data = load_image_user_data_;
   } else {
     load_image_option.preserve_channels = preserve_image_channels_;
+    load_image_option.as_is = images_as_is_;
     load_image_user_data = reinterpret_cast<void *>(&load_image_option);
   }
 
@@ -8547,7 +8597,7 @@ bool TinyGLTF::WriteGltfSceneToStream(const Model *model, std::ostream &stream,
       // we
       std::string uri;
       if (!UpdateImageObject(model->images[i], dummystring, int(i), true,
-                             &uri_cb, this->WriteImageData,
+                             &fs, &uri_cb, this->WriteImageData,
                              this->write_image_user_data_, &uri)) {
         return false;
       }
@@ -8655,7 +8705,7 @@ bool TinyGLTF::WriteGltfSceneToFile(const Model *model,
 
       std::string uri;
       if (!UpdateImageObject(model->images[i], baseDir, int(i), embedImages,
-                             &uri_cb, this->WriteImageData,
+                             &fs, &uri_cb, this->WriteImageData,
                              this->write_image_user_data_, &uri)) {
         return false;
       }