From f93046308c7a6b18302e6c3d671464741adfa36a Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 5 Jun 2019 10:10:47 -0700 Subject: [PATCH] gltf_baker: improve error handling throughout. --- libs/gltfio/include/gltfio/AssetPipeline.h | 12 +++- libs/gltfio/src/AssetLoader.cpp | 1 + libs/gltfio/src/AssetPipeline.cpp | 84 ++++++++++++---------- libs/gltfio/src/FFilamentAsset.h | 5 +- libs/gltfio/src/UbershaderLoader.cpp | 4 ++ samples/gltf_baker.cpp | 75 ++++++++++++------- third_party/cgltf/cgltf_write.h | 4 ++ 7 files changed, 121 insertions(+), 64 deletions(-) diff --git a/libs/gltfio/include/gltfio/AssetPipeline.h b/libs/gltfio/include/gltfio/AssetPipeline.h index e6528b1f04..ce21503799 100644 --- a/libs/gltfio/include/gltfio/AssetPipeline.h +++ b/libs/gltfio/include/gltfio/AssetPipeline.h @@ -65,11 +65,11 @@ public: /** * Saves a flattened asset to the filesystem as a JSON-based glTF 2.0 file, as well as a sidecar - * bin file that only contains buffer data. + * bin file that only contains buffer data. Returns false if an error occurred. * * The supplied binPath should live in the same folder as the jsonPath. */ - void save(AssetHandle, const utils::Path& jsonPath, const utils::Path& binPath); + bool save(AssetHandle, const utils::Path& jsonPath, const utils::Path& binPath); /** * Flattens and sanitizes a scene by baking transforms, dereferencing shared meshes, providing @@ -79,6 +79,8 @@ public: * resulting mesh has only one primitive. The triangles in the resulting asset should be * visually equivalent to the source, although the underlying scene structure and * resource-sharing will be lost. + * + * Assets with animation, skins, and morph targets are not supported. */ AssetHandle flatten(AssetHandle source, uint32_t flags = ~0u); @@ -116,6 +118,11 @@ public: */ void setOcclusionUri(AssetHandle source, const utils::Path& texture); + /** + * Modifies the baseColor texture URI for all materials. + */ + void setBaseColorUri(AssetHandle source, const utils::Path& texture); + /** * Signals that a region of a path-traced image is available (used for progress notification). * This can be called from any thread. @@ -167,7 +174,6 @@ public: const RenderOptions& options); static bool isFlattened(AssetHandle source); - static bool isParameterized(AssetHandle source); AssetPipeline(); ~AssetPipeline(); diff --git a/libs/gltfio/src/AssetLoader.cpp b/libs/gltfio/src/AssetLoader.cpp index 3272166351..0433cd5768 100644 --- a/libs/gltfio/src/AssetLoader.cpp +++ b/libs/gltfio/src/AssetLoader.cpp @@ -822,6 +822,7 @@ FilamentAsset* AssetLoader::createAssetFromBinary(uint8_t const* bytes, uint32_t FilamentAsset* AssetLoader::createAssetFromHandle(const void* handle) { const cgltf_data* sourceAsset = (const cgltf_data*) handle; upcast(this)->createAsset(sourceAsset); + upcast(this)->mResult->mSharedSourceAsset = true; return upcast(this)->mResult; } diff --git a/libs/gltfio/src/AssetPipeline.cpp b/libs/gltfio/src/AssetPipeline.cpp index 0a80bb8637..24bfa71ef1 100644 --- a/libs/gltfio/src/AssetPipeline.cpp +++ b/libs/gltfio/src/AssetPipeline.cpp @@ -99,6 +99,9 @@ public: // Replaces the texture URI for all primitives that have BAKED_UV_ATTRIB. void setOcclusionUri(cgltf_data* asset, const Path& texturePath); + // Replaces the baseColor URI for all materials. + void setBaseColorUri(cgltf_data* asset, const Path& texturePath); + // Take ownership of the given asset and free it when the pipeline is destroyed. void retainSourceAsset(cgltf_data* asset); @@ -173,31 +176,6 @@ bool isFlattened(const cgltf_data* asset) { !strcmp(asset->asset.generator, GENERATOR_ID); } -// Returns true if the given cgltf asset has been flattened and has BAKED_UV_ATTRIB. -// This can produce false positives in some cases so should be used only as a workflow hint. -bool isParameterized(const cgltf_data* asset) { - if (!isFlattened(asset)) { - return false; - } - const size_t numPrims = asset->meshes_count; - for (cgltf_size i = 0; i < numPrims; i++) { - const cgltf_mesh& mesh = asset->meshes[i]; - const cgltf_primitive& prim = mesh.primitives[0]; - bool good = false; - for (cgltf_size k = 0; k < prim.attributes_count && !good; ++k) { - const cgltf_attribute& attr = prim.attributes[k]; - if (attr.type == cgltf_attribute_type_texcoord && - attr.index == gltfio::AssetPipeline::BAKED_UV_ATTRIB_INDEX) { - good = true; - } - } - if (!good) { - return false; - } - } - return true; -} - // Returns true if the given primitive should be baked out, false if it should be culled away. bool Pipeline::filterPrim(const cgltf_primitive& prim) { const bool filterTriangles = mFlattenFlags & gltfio::AssetPipeline::FILTER_TRIANGLES; @@ -297,7 +275,9 @@ const cgltf_data* Pipeline::flattenBuffers(const cgltf_data* sourceAsset) { // Clone the textures. for (size_t i = 0, len = sourceAsset->textures_count; i < len; ++i) { auto& texture = textures[i] = sourceAsset->textures[i]; - texture.image = images + (texture.image - sourceAsset->images); + if (texture.image) { + texture.image = images + (texture.image - sourceAsset->images); + } } // Clone the nodes. @@ -731,8 +711,11 @@ const cgltf_data* Pipeline::flattenPrims(const cgltf_data* sourceAsset, uint32_t } } for (size_t i = 0; i < resultAsset->textures_count; ++i) { - size_t imageIndex = resultAsset->textures[i].image - sourceAsset->images; - resultAsset->textures[i].image = images + imageIndex; + auto& image = resultAsset->textures[i].image; + if (image) { + size_t imageIndex = image - sourceAsset->images; + image = images + imageIndex; + } } return resultAsset; @@ -1613,6 +1596,21 @@ void Pipeline::setOcclusionUri(cgltf_data* asset, const Path& texturePath) { } } +void Pipeline::setBaseColorUri(cgltf_data* asset, const Path& texturePath) { + if (!isFlattened(asset)) { + utils::slog.e << "Only flattened assets can be modified." << utils::io::endl; + } + std::string uri = texturePath; + char* pathString = (char*) mStorage.bufferData.alloc(uri.size() + 1); + strncpy(pathString, uri.c_str(), uri.size() + 1); + for (size_t mindex = 0, len = asset->materials_count; mindex < len; ++mindex) { + auto& texview = asset->materials[mindex].pbr_metallic_roughness.base_color_texture; + if (texview.texture && texview.texture->image) { + texview.texture->image->uri = pathString; + } + } +} + void Pipeline::retainSourceAsset(cgltf_data* asset) { mSourceAssets.push_back(asset); } @@ -1639,6 +1637,10 @@ AssetPipeline::~AssetPipeline() { AssetHandle AssetPipeline::flatten(AssetHandle source, uint32_t flags) { Pipeline* impl = (Pipeline*) mImpl; const cgltf_data* asset = (const cgltf_data*) source; + if (asset->animations_count > 0 || asset->skins_count > 0) { + utils::slog.e << "Cannot flatten assets with animation or skinning." << utils::io::endl; + return nullptr; + } if (asset->buffers_count > 1) { asset = impl->flattenBuffers(asset); } @@ -1681,30 +1683,39 @@ AssetHandle AssetPipeline::load(const utils::Path& fileOrDirectory) { utils::Path abspath = filename.getAbsolutePath(); if (cgltf_load_buffers(&options, sourceAsset, abspath.c_str()) != cgltf_result_success) { utils::slog.e << "Unable to load external buffers." << utils::io::endl; - exit(1); + return nullptr; } return sourceAsset; } -void AssetPipeline::save(AssetHandle handle, const utils::Path& jsonPath, +bool AssetPipeline::save(AssetHandle handle, const utils::Path& jsonPath, const utils::Path& binPath) { cgltf_data* asset = (cgltf_data*) handle; if (!isFlattened(asset)) { utils::slog.e << "Only flattened assets can be exported to disk." << utils::io::endl; - return; + return false; } std::string binName = binPath.getName(); asset->buffers[0].uri = (char*) (binName.c_str()); cgltf_options options { cgltf_file_type_gltf }; - cgltf_write_file(&options, jsonPath.c_str(), asset); + if (cgltf_write_file(&options, jsonPath.c_str(), asset) != cgltf_result_success) { + utils::slog.e << "Unable to write to " << jsonPath << utils::io::endl; + asset->buffers[0].uri = nullptr; + return false; + } asset->buffers[0].uri = nullptr; FILE* binFile = fopen(binPath.c_str(), "wb"); + if (!binFile) { + utils::slog.e << "Unable to write to " << binPath << utils::io::endl; + return false; + } fwrite((char*) asset->buffers[0].data, asset->buffers[0].size, 1, binFile); fclose(binFile); + return true; } AssetHandle AssetPipeline::parameterize(AssetHandle source, int maxIterations) { @@ -1727,6 +1738,11 @@ void AssetPipeline::setOcclusionUri(AssetHandle asset, const Path& texture) { impl->setOcclusionUri((cgltf_data*) asset, texture); } +void AssetPipeline::setBaseColorUri(AssetHandle asset, const Path& texture) { + Pipeline* impl = (Pipeline*) mImpl; + impl->setBaseColorUri((cgltf_data*) asset, texture); +} + void AssetPipeline::bakeAmbientOcclusion(AssetHandle source, image::LinearImage target, const RenderOptions& options) { Pipeline* impl = (Pipeline*) mImpl; @@ -1765,8 +1781,4 @@ bool AssetPipeline::isFlattened(AssetHandle source) { return ::isFlattened((const cgltf_data*) source); } -bool AssetPipeline::isParameterized(AssetHandle source) { - return ::isParameterized((const cgltf_data*) source); -} - } // namespace gltfio diff --git a/libs/gltfio/src/FFilamentAsset.h b/libs/gltfio/src/FFilamentAsset.h index 3cfb7e8e12..3bd96b25c2 100644 --- a/libs/gltfio/src/FFilamentAsset.h +++ b/libs/gltfio/src/FFilamentAsset.h @@ -163,7 +163,9 @@ struct FFilamentAsset : public FilamentAsset { if (--mSourceAssetRefCount == 0) { mGlbData.clear(); mGlbData.shrink_to_fit(); - cgltf_free((cgltf_data*) mSourceAsset); + if (!mSharedSourceAsset) { + cgltf_free((cgltf_data*) mSourceAsset); + } mSourceAsset = nullptr; } } @@ -190,6 +192,7 @@ struct FFilamentAsset : public FilamentAsset { const cgltf_data* mSourceAsset = nullptr; tsl::robin_map mNodeMap; tsl::robin_map mPrimMap; + bool mSharedSourceAsset = false; /** @} */ }; diff --git a/libs/gltfio/src/UbershaderLoader.cpp b/libs/gltfio/src/UbershaderLoader.cpp index 5077c3c7aa..13bd5bd641 100644 --- a/libs/gltfio/src/UbershaderLoader.cpp +++ b/libs/gltfio/src/UbershaderLoader.cpp @@ -115,6 +115,10 @@ Material* UbershaderLoader::getMaterial(const MaterialKey& config) const { MaterialInstance* UbershaderLoader::createMaterialInstance(MaterialKey* config, UvMap* uvmap, const char* label) { + // Diagnostics are not supported with LOAD_UBERSHADERS, please use GENERATE_SHADERS instead. + if (config->enableDiagnostics) { + return nullptr; + } gltfio::details::constrainMaterial(config, uvmap); auto getUvIndex = [uvmap](uint8_t srcIndex, bool hasTexture) -> int { return hasTexture ? int(uvmap->at(srcIndex)) - 1 : -1; diff --git a/samples/gltf_baker.cpp b/samples/gltf_baker.cpp index 2ee86f3ba7..c05f7507f8 100644 --- a/samples/gltf_baker.cpp +++ b/samples/gltf_baker.cpp @@ -94,6 +94,7 @@ struct BakerApp { bool hasTestRender = false; bool isWorking = false; std::string statusText; + ImVec4 statusColor; std::string messageBoxText; bool requestViewerUpdate = false; Visualization visualization = Visualization::MESH_CURRENT; @@ -313,6 +314,7 @@ static void updateViewerMesh(BakerApp& app) { } if (!app.viewerAsset || app.viewerAsset->getSourceAsset() != handle) { + auto previousViewerAsset = app.viewerAsset; app.viewerAsset = app.loader->createAssetFromHandle(handle); // Load external textures and buffers. @@ -326,8 +328,11 @@ static void updateViewerMesh(BakerApp& app) { // Load animation data then free the source hierarchy. app.viewerAsset->getAnimator(); - // Destroy the old currentAsset and add the renderables to the scene. + // Remove old renderables and add new renderables to the scene. app.viewer->setAsset(app.viewerAsset, app.names, !app.viewerActualSize); + + // Destory old Filament entities. + app.loader->destroyAsset(previousViewerAsset); } } @@ -509,9 +514,6 @@ static void generateUvVisualization(const utils::Path& pngOutputPath) { static void executeBakeAo(BakerApp& app) { using namespace image; - app.hasTestRender = false; - app.isWorking = true; - auto onRenderTile = makeTileCallback([](BakerApp* app) { app->requestViewerUpdate = true; }); @@ -547,6 +549,7 @@ static void executeBakeAo(BakerApp& app) { auto doRender = [&app, onRenderTile, onRenderDone] { const uint32_t res = app.bakeOptions.resolution; app.statusText.clear(); + app.hasTestRender = false; app.visualization = Visualization::IMAGE_OCCLUSION; app.ambientOcclusion = image::LinearImage(res, res, 1); app.bentNormals = image::LinearImage(res, res, 3); @@ -566,10 +569,11 @@ static void executeBakeAo(BakerApp& app) { }); }; + app.isWorking = true; app.previewAoAsset = nullptr; app.modifiedAsset = nullptr; app.previewUvAsset = nullptr; - app.ambientOcclusion = LinearImage(); + app.statusColor = ImVec4({0, 1, 0, 1}); app.statusText = "Parameterizing..."; utils::JobSystem* js = utils::JobSystem::getJobSystem(); @@ -604,48 +608,52 @@ static void executeExport(BakerApp& app) { auto exportOcclusion = [&app, occlusionPath]() { using namespace image; std::ofstream out(occlusionPath.c_str(), std::ios::binary | std::ios::trunc); - ImageEncoder::encode(out, ImageEncoder::Format::PNG_LINEAR, app.ambientOcclusion, "", + return ImageEncoder::encode(out, ImageEncoder::Format::PNG_LINEAR, app.ambientOcclusion, "", occlusionPath.c_str()); }; auto exportBentNormals = [&app, bentNormalsPath]() { using namespace image; std::ofstream out(bentNormalsPath.c_str(), std::ios::binary | std::ios::trunc); - ImageEncoder::encode(out, ImageEncoder::Format::PNG_LINEAR, app.bentNormals, "", + return ImageEncoder::encode(out, ImageEncoder::Format::PNG_LINEAR, app.bentNormals, "", bentNormalsPath.c_str()); }; - std::string msg = "Exported "; + std::string msg; + bool error = false; const std::string join = ", "; switch (options.selection) { case Visualization::MESH_CURRENT: - app.pipeline->save(app.flattenedAsset, gltfPath, binPath); - msg += options.gltfPath + join + options.binPath; + error = error || !app.pipeline->save(app.flattenedAsset, gltfPath, binPath); + msg = options.gltfPath + join + options.binPath; break; case Visualization::MESH_MODIFIED: - exportOcclusion(); + error = error || !exportOcclusion(); app.pipeline->setOcclusionUri(app.modifiedAsset, options.occlusionPath); - app.pipeline->save(app.modifiedAsset, gltfPath, binPath); - msg += options.gltfPath + join + options.binPath + join + options.occlusionPath; + error = error || !app.pipeline->save(app.modifiedAsset, gltfPath, binPath); + app.pipeline->setOcclusionUri(app.modifiedAsset, TMP_AO_FILENAME); + msg = options.gltfPath + join + options.binPath + join + options.occlusionPath; break; case Visualization::MESH_PREVIEW_AO: - exportOcclusion(); - app.pipeline->setOcclusionUri(app.previewAoAsset, options.occlusionPath); - app.pipeline->save(app.previewAoAsset, gltfPath, binPath); - msg += options.gltfPath + join + options.binPath + join + options.occlusionPath; + error = error || !exportOcclusion(); + app.pipeline->setBaseColorUri(app.previewAoAsset, options.occlusionPath); + error = error || !app.pipeline->save(app.previewAoAsset, gltfPath, binPath); + app.pipeline->setBaseColorUri(app.previewAoAsset, TMP_AO_FILENAME); + msg = options.gltfPath + join + options.binPath + join + options.occlusionPath; break; case Visualization::IMAGE_OCCLUSION: - exportOcclusion(); - msg += options.occlusionPath; + error = error || !exportOcclusion(); + msg = options.occlusionPath; break; case Visualization::IMAGE_BENT_NORMALS: - exportBentNormals(); - msg += options.bentNormalsPath; + error = error || !exportBentNormals(); + msg = options.bentNormalsPath; break; default: return; } - app.statusText = msg; + app.statusColor = error ? ImVec4({1, 0, 0, 1}) : ImVec4({0, 1, 0, 1}); + app.statusText = (error ? "Failed export to " : "Exported ") + msg; } int main(int argc, char** argv) { @@ -793,7 +801,7 @@ int main(int argc, char** argv) { // Status text if (app.statusText.size()) { ImGui::PushStyleVar(ImGuiStyleVar_ItemSpacing, {10, 10} ); - ImGui::TextColored({0, 1, 0, 1}, "%s", app.statusText.c_str()); + ImGui::TextColored(app.statusColor, "%s", app.statusText.c_str()); ImGui::PopStyleVar(); if (app.isWorking) { static float fraction = 0; @@ -913,12 +921,23 @@ int main(int argc, char** argv) { }; auto cleanup = [&app](Engine* engine, View*, Scene*) { - Fence::waitAndDestroy(engine->createFence()); + std::cout << "Destroying viewer..." << std::endl; + app.viewer->removeAsset(); delete app.viewer; + std::cout << "Destroying viewer asset..." << std::endl; + app.loader->destroyAsset(app.viewerAsset); + app.viewerAsset = nullptr; + std::cout << "Destroying pipeline..." << std::endl; + delete app.pipeline; + std::cout << "Destroying AssetLoader materials..." << std::endl; app.materials->destroyMaterials(); delete app.materials; + std::cout << "Destroying AssetLoader..." << std::endl; AssetLoader::destroy(&app.loader); + std::cout << "Destroying NameComponentManager..." << std::endl; delete app.names; + + Fence::waitAndDestroy(engine->createFence()); }; auto animate = [&app](Engine* engine, View* view, double now) { @@ -970,7 +989,15 @@ int main(int argc, char** argv) { filamentApp.setDropHandler([&] (std::string path) { app.viewer->removeAsset(); + app.loader->destroyAsset(app.viewerAsset); + app.viewerAsset = nullptr; app.filename = path; + app.hasTestRender = false; + app.ambientOcclusion = image::LinearImage(); + app.bentNormals = image::LinearImage(); + app.meshNormals = image::LinearImage(); + app.meshPositions = image::LinearImage(); + app.visualization = Visualization::MESH_CURRENT; loadAssetFromDisk(app); saveIniFile(app); }); diff --git a/third_party/cgltf/cgltf_write.h b/third_party/cgltf/cgltf_write.h index 85554d239f..9451520cee 100644 --- a/third_party/cgltf/cgltf_write.h +++ b/third_party/cgltf/cgltf_write.h @@ -630,6 +630,10 @@ cgltf_result cgltf_write_file(const cgltf_options* options, const char* path, co fprintf(stderr, "Error: expected %zu bytes but wrote %zu bytes.\n", expected, actual); } FILE* file = fopen(path, "wt"); + if (!file) + { + return cgltf_result_file_not_found; + } // Note that cgltf_write() includes a null terminator, which we omit from the file content. fwrite(buffer, actual - 1, 1, file); fclose(file);