Images are not saved to disk properly
ptc-brainer opened this issue · 3 comments
Describe the issue
Writing a glTF that contains images referenced as uris without embedding images does not save the images to the disk next to the glTF model.
To Reproduce
OS: MacOS
Compiler: Apple clang version 15.0.0
The following unit test fails with with the latest version of tinygltf:
TEST_CASE("write-image-issue") {
tinygltf::Model model;
tinygltf::TinyGLTF ctx;
std::string err;
std::string warn;
REQUIRE(ctx.LoadASCIIFromFile(&model, &err, &warn, "../models/Cube/Cube.gltf"));
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());
REQUIRE(ctx.WriteGltfSceneToFile(&model, "Cube.gltf", false, true));
for (const auto image : model.images)
{
std::fstream file(image.uri);
CHECK(file.good());
}
}
Expected behaviour
I would expect that the writer saves the images to the disk at the relative location described in the image uri.
Additional context
I traced this issue down to the WriteImageData()
function that is passed to UpdateImageObject()
. This function expects the FsCallback
passed as the user data. However, the default write_image_user_data
with is initialized withnullptr
, causing the file to never be written to disk. An experimental change the initialize write_image_user_data
with a pointer to the default FsCallback (&fs
) fixed this issue, but caused a different unit test failure.
Confirmed the issue.
Passing FsCallback
to write_image_user_data
is a confusing, so we are better to modify WriteImageDataFunction
API to accept FsCallbacks
callback pointer as an argument.
something like...
typedef bool (*WriteImageDataFunction)(const std::string *basepath,
const std::string *filename,
const Image *image, bool embedImages,
const FsCallback *fs_cb,
const URICallbacks *uri_cb,
std::string *out_uri,
void *user_pointer);
We may also need to add overwrite existing image file
flag to the argument.
Thanks for the quick investigation! Your proposal looks good to me!
One point to consider as well is to omit unneeded copies of the source and dest paths are the same
Let me give some time to figure out what is best solution for WriteImageDataFunction