From 45e8c57f77ebdad7df7ee99d44bebb3c2f14da0d Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Mon, 29 Aug 2022 09:57:59 -0700 Subject: [PATCH] gltfio: Innocuous comment fixes / renamings. --- .../android/filament/gltfio/AssetLoader.java | 1 - libs/gltfio/include/gltfio/AssetLoader.h | 10 +++--- libs/gltfio/src/AssetLoader.cpp | 6 ++-- libs/gltfio/src/DependencyGraph.cpp | 36 ++++++------------- libs/gltfio/src/DependencyGraph.h | 36 ++++++------------- libs/gltfio/src/ResourceLoader.cpp | 4 +-- 6 files changed, 33 insertions(+), 60 deletions(-) diff --git a/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/AssetLoader.java b/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/AssetLoader.java index 3a3fc81cbc..60da99720a 100644 --- a/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/AssetLoader.java +++ b/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/AssetLoader.java @@ -160,7 +160,6 @@ public class AssetLoader { * create/destroy churn, as noted above. * * This cannot be called after FilamentAsset#releaseSourceData(). - * Animation is not supported in new instances. * See also AssetLoader#createInstancedAsset(). */ @Nullable diff --git a/libs/gltfio/include/gltfio/AssetLoader.h b/libs/gltfio/include/gltfio/AssetLoader.h index 7023c0638e..689ed3d53c 100644 --- a/libs/gltfio/include/gltfio/AssetLoader.h +++ b/libs/gltfio/include/gltfio/AssetLoader.h @@ -157,9 +157,10 @@ public: * Consumes the contents of a glTF 2.0 file and produces a primary asset with one or more * instances. The primary asset has ownership over the instances. * - * The returned instances share their textures, material instances, and vertex buffers with the - * primary asset. However each instance has its own unique set of entities, transform - * components, and renderable components. Instances are freed when the primary asset is freed. + * The returned instances share their textures, materials, and vertex buffers with the primary + * asset. However each instance has its own unique set of entities, transform components, + * material instances, and renderable components. Instances are freed when the primary asset is + * freed. * * Light components are not instanced, they belong only to the primary asset. * @@ -201,7 +202,8 @@ public: void enableDiagnostics(bool enable = true); /** - * Destroys the given asset and all of its associated Filament objects. + * Destroys the given asset, all of its associated Filament objects, and all associated + * FilamentInstance objects. * * This destroys entities, components, material instances, vertex buffers, index buffers, * and textures. This does not necessarily immediately free all source data, since diff --git a/libs/gltfio/src/AssetLoader.cpp b/libs/gltfio/src/AssetLoader.cpp index 303c8b1ae3..9b9d7afe78 100644 --- a/libs/gltfio/src/AssetLoader.cpp +++ b/libs/gltfio/src/AssetLoader.cpp @@ -234,7 +234,7 @@ FilamentInstance* FAssetLoader::createInstance(FFilamentAsset* primary) { primary->mAnimator->addInstance(instance); } - primary->mDependencyGraph.refinalize(); + primary->mDependencyGraph.commitEdges(); return instance; } @@ -474,6 +474,8 @@ void FAssetLoader::createRenderable(const cgltf_data* srcAsset, const cgltf_node builder.morphing(numMorphTargets); // For each prim, create a Filament VertexBuffer, IndexBuffer, and MaterialInstance. + // The VertexBuffer and IndexBuffer objects are cached for possible re-use, but MaterialInstance + // is not. for (cgltf_size index = 0; index < nprims; ++index, ++outputPrim, ++inputPrim) { RenderableManager::PrimitiveType primType; if (!getPrimitiveType(inputPrim->type, &primType)) { @@ -1327,7 +1329,7 @@ void FAssetLoader::addTextureBinding(MaterialInstance* materialInstance, const c dstSampler.setWrapModeS(TextureSampler::WrapMode::REPEAT); dstSampler.setWrapModeT(TextureSampler::WrapMode::REPEAT); - // These defaults are up the implementation but since we try to provide mipmaps, + // These defaults are up to the implementation but since we try to provide mipmaps, // we might as well use them. In practice the conformance models look awful without // using mipmapping by default. dstSampler.setMagFilter(TextureSampler::MagFilter::LINEAR); diff --git a/libs/gltfio/src/DependencyGraph.cpp b/libs/gltfio/src/DependencyGraph.cpp index 1586f96e99..cdd164c3cc 100644 --- a/libs/gltfio/src/DependencyGraph.cpp +++ b/libs/gltfio/src/DependencyGraph.cpp @@ -34,36 +34,21 @@ size_t DependencyGraph::popRenderables(Entity* result, size_t count) noexcept { } void DependencyGraph::addEdge(Entity entity, MaterialInstance* mi) { - - // Permit adding an Entity-Material edge to a finalized graph as long as the material is already - // known. Since we already encountered this material instance, we already know what textures it - // is associated with. - assert(!mFinalized || mMaterialToEntity.find(mi) != mMaterialToEntity.end()); - mMaterialToEntity[mi].insert(entity); mEntityToMaterial[entity].materials.insert(mi); } void DependencyGraph::addEdge(MaterialInstance* mi, const char* parameter) { - assert(!mFinalized); + if (auto iter = mMaterialToTexture.find(mi); iter != mMaterialToTexture.end()) { + const tsl::robin_map& params = iter.value().params; + if (params.find(parameter) != params.end()) { + return; + } + } mMaterialToTexture[mi].params[parameter] = nullptr; } -// During finalization, the structure of the glTF is known but we have not yet created texture -// objects. Find all non-textured entities and immediately add mark them as ready. -void DependencyGraph::finalize() { - assert(!mFinalized); - for (const auto& pair : mMaterialToEntity) { - auto mi = pair.first; - if (mMaterialToTexture.find(mi) == mMaterialToTexture.end()) { - markAsReady(mi); - } - } - mFinalized = true; -} - -void DependencyGraph::refinalize() { - assert(mFinalized); +void DependencyGraph::commitEdges() { for (const auto& pair : mMaterialToEntity) { auto material = pair.first; if (mMaterialToTexture.find(material) == mMaterialToTexture.end()) { @@ -75,7 +60,7 @@ void DependencyGraph::refinalize() { } void DependencyGraph::addEdge(Texture* texture, MaterialInstance* material, const char* parameter) { - assert(texture && !mFinalized); + assert(texture); mTextureToMaterial[texture].insert(material); mMaterialToTexture.at(material).params.at(parameter) = getStatus(texture); } @@ -86,8 +71,7 @@ void DependencyGraph::checkReadiness(Material* material) { // Check this material's texture parameters, there are 5 in the worst case. bool materialIsReady = true; for (const auto& pair : status.params) { - assert(pair.second && "Parameter-to-Texture edge is missing."); - if (!pair.second->ready) { + if (!pair.second || !pair.second->ready) { materialIsReady = false; break; } @@ -100,7 +84,7 @@ void DependencyGraph::checkReadiness(Material* material) { } void DependencyGraph::markAsReady(Texture* texture) { - assert(texture && mFinalized); + assert(texture); mTextureNodes.at(texture)->ready = true; // Iterate over the materials associated with this texture to check if any have become ready. diff --git a/libs/gltfio/src/DependencyGraph.h b/libs/gltfio/src/DependencyGraph.h index 0e3ef7d71c..7ecd40edca 100644 --- a/libs/gltfio/src/DependencyGraph.h +++ b/libs/gltfio/src/DependencyGraph.h @@ -55,11 +55,11 @@ namespace filament::gltfio { * Texture Texture Texture * * Note that the left-most entity in the above graph has no textures, so it becomes ready as soon as - * finalize is called. + * commitEdges is called. */ class DependencyGraph { public: - using Material = filament::MaterialInstance; + using Material = MaterialInstance; using Entity = utils::Entity; // Pops up to "count" ready-to-render entities off the queue. @@ -68,23 +68,16 @@ public: // If "result" is null, returns the number of available entities. size_t popRenderables(Entity* result, size_t count) noexcept; - // These are called during the initial asset loader phase. void addEdge(Entity entity, Material* material); void addEdge(Material* material, const char* parameter); - void addEdge(filament::Texture* texture, Material* material, const char* parameter); + void addEdge(Texture* texture, Material* material, const char* parameter); - // Marks the end of synchronous asset loading. - // - // At this point, the graph enters a finalized state and all non-textured entities are - // immediately marked as "ready". However textures are not yet fully decoded. - // - // After finalization, the only nodes that can be added to the graph are entities. - void finalize(); + // Commits a set of edges to the graph. This simply triggers a check to see if + // any entities are already ready, e.g. if any entities are non-textured. + void commitEdges(); // Marks the given texture as being fully decoded, with all miplevels initialized. - // - // This can only be called on a finalized graph. - void markAsReady(filament::Texture* texture); + void markAsReady(Texture* texture); // Marks the material as ready, but due to an error. // @@ -92,15 +85,9 @@ public: // dependencies will never become available. void markAsError(Material* material) { markAsReady(material); } - // Re-checks the readiness of all entities after finalization. - // - // This exists only to support dynamic instancing. It is slower than finalize() because it - // checks the readiness of existing materials. - void refinalize(); - private: struct TextureNode { - filament::Texture* texture; + Texture* texture; bool ready; }; @@ -115,21 +102,20 @@ private: void checkReadiness(Material* material); void markAsReady(Material* material); - TextureNode* getStatus(filament::Texture* texture); + TextureNode* getStatus(Texture* texture); // The following maps contain the directed edges in the graph. tsl::robin_map mEntityToMaterial; tsl::robin_map> mMaterialToEntity; tsl::robin_map mMaterialToTexture; - tsl::robin_map> mTextureToMaterial; + tsl::robin_map> mTextureToMaterial; // Each texture (and its readiness flag) can be referenced from multiple nodes, so we own // a collection of wrapper objects in the following map. This uses std::unique_ptr to allow // nodes to refer to a texture wrapper using a stable weak pointer. - tsl::robin_map> mTextureNodes; + tsl::robin_map> mTextureNodes; std::queue mReadyRenderables; - bool mFinalized = false; }; } // namespace filament::gltfio diff --git a/libs/gltfio/src/ResourceLoader.cpp b/libs/gltfio/src/ResourceLoader.cpp index 820163460a..e28a28b23a 100644 --- a/libs/gltfio/src/ResourceLoader.cpp +++ b/libs/gltfio/src/ResourceLoader.cpp @@ -504,8 +504,8 @@ bool ResourceLoader::loadResources(FFilamentAsset* asset, bool async) { pImpl->createTextures(asset, async); // Non-textured renderables are now considered ready, and we can guarantee that no new - // materials or textures will be added. notify the dependency graph. - asset->mDependencyGraph.finalize(); + // materials or textures will be added. Notify the dependency graph. + asset->mDependencyGraph.commitEdges(); asset->createAnimators();