From 3e181b6aff0b9dba7f2030765249e2dc6958167f Mon Sep 17 00:00:00 2001 From: Nick Fisher Date: Thu, 2 Jan 2025 16:46:44 +0800 Subject: [PATCH] fix: allow destroying instances independently of owner --- .../include/scene/GeometrySceneAsset.hpp | 16 +++-- thermion_dart/native/include/scene/Gizmo.hpp | 2 + .../native/include/scene/GltfSceneAsset.hpp | 11 +++- .../include/scene/GltfSceneAssetInstance.hpp | 16 ++++- .../native/include/scene/GridOverlay.hpp | 5 ++ .../native/include/scene/SceneAsset.hpp | 3 + .../native/src/scene/GeometrySceneAsset.cpp | 64 ++++--------------- .../src/scene/GeometrySceneAssetBuilder.cpp | 1 - .../native/src/scene/GltfSceneAsset.cpp | 1 + .../src/scene/GltfSceneAssetInstance.cpp | 10 ++- .../native/src/scene/SceneManager.cpp | 42 ++++++------ thermion_dart/test/geometry_tests.dart | 10 +-- thermion_dart/test/gltf_tests.dart | 51 +++++++++++++-- 13 files changed, 141 insertions(+), 91 deletions(-) diff --git a/thermion_dart/native/include/scene/GeometrySceneAsset.hpp b/thermion_dart/native/include/scene/GeometrySceneAsset.hpp index 2ad34a3d..98598fc6 100644 --- a/thermion_dart/native/include/scene/GeometrySceneAsset.hpp +++ b/thermion_dart/native/include/scene/GeometrySceneAsset.hpp @@ -16,18 +16,20 @@ namespace thermion class GeometrySceneAsset : public SceneAsset { public: - GeometrySceneAsset(bool isInstance, - Engine *engine, + GeometrySceneAsset(Engine *engine, VertexBuffer *vertexBuffer, IndexBuffer *indexBuffer, MaterialInstance **materialInstances, size_t materialInstanceCount, RenderableManager::PrimitiveType primitiveType, - Box boundingBox); + Box boundingBox, + GeometrySceneAsset *instanceParent = std::nullptr_t()); ~GeometrySceneAsset(); SceneAsset *createInstance(MaterialInstance **materialInstances = nullptr, size_t materialInstanceCount = 0) override; + void destroyInstance(SceneAsset *sceneAsset) override; + SceneAssetType getType() override { return SceneAsset::SceneAssetType::Geometry; @@ -35,7 +37,11 @@ namespace thermion bool isInstance() override { - return _isInstance; + return _instanceOwner; + } + + SceneAsset *getInstanceOwner() override { + return _instanceOwner; } utils::Entity getEntity() override @@ -132,7 +138,7 @@ namespace thermion MaterialInstance **_materialInstances = nullptr; size_t _materialInstanceCount = 0; Box _boundingBox; - bool _isInstance = false; + GeometrySceneAsset *_instanceOwner = std::nullptr_t(); utils::Entity _entity; RenderableManager::PrimitiveType _primitiveType; std::vector> _instances; diff --git a/thermion_dart/native/include/scene/Gizmo.hpp b/thermion_dart/native/include/scene/Gizmo.hpp index 2df4e2d9..abf99c7f 100644 --- a/thermion_dart/native/include/scene/Gizmo.hpp +++ b/thermion_dart/native/include/scene/Gizmo.hpp @@ -66,7 +66,9 @@ namespace thermion SceneAssetType getType() override { return SceneAssetType::Gizmo; } utils::Entity getEntity() override { return _parent; } bool isInstance() override { return false; } + SceneAsset *getInstanceOwner() override { return std::nullptr_t(); } SceneAsset *createInstance(MaterialInstance **materialInstances, size_t materialInstanceCount) override { return nullptr; } + void destroyInstance(SceneAsset *instance) override { return; } MaterialInstance **getMaterialInstances() override { return _materialInstances.data(); } size_t getMaterialInstanceCount() override { return _materialInstances.size(); } diff --git a/thermion_dart/native/include/scene/GltfSceneAsset.hpp b/thermion_dart/native/include/scene/GltfSceneAsset.hpp index 7ed274a7..947c3d34 100644 --- a/thermion_dart/native/include/scene/GltfSceneAsset.hpp +++ b/thermion_dart/native/include/scene/GltfSceneAsset.hpp @@ -13,7 +13,6 @@ #include - #include "scene/GltfSceneAssetInstance.hpp" #include "components/AnimationComponentManager.hpp" #include "components/CollisionComponentManager.hpp" @@ -49,6 +48,12 @@ namespace thermion SceneAsset *createInstance(MaterialInstance **materialInstances = nullptr, size_t materialInstanceCount = 0) override; + void destroyInstance(SceneAsset *asset) override { + auto it = std::remove_if(_instances.begin(), _instances.end(), [=](auto &sceneAsset) + { return sceneAsset.get() == asset; }); + _instances.erase(it, _instances.end()); + }; + SceneAssetType getType() override { return SceneAsset::SceneAssetType::Gltf; @@ -59,6 +64,10 @@ namespace thermion return false; } + SceneAsset *getInstanceOwner() override { + return std::nullptr_t(); + } + utils::Entity getEntity() override { return _asset->getRoot(); diff --git a/thermion_dart/native/include/scene/GltfSceneAssetInstance.hpp b/thermion_dart/native/include/scene/GltfSceneAssetInstance.hpp index 2dcc3f96..36b527af 100644 --- a/thermion_dart/native/include/scene/GltfSceneAssetInstance.hpp +++ b/thermion_dart/native/include/scene/GltfSceneAssetInstance.hpp @@ -12,7 +12,6 @@ #include #include - #include "scene/SceneAsset.hpp" namespace thermion @@ -20,16 +19,20 @@ namespace thermion using namespace filament; + class GltfSceneAsset; + class GltfSceneAssetInstance : public SceneAsset { public: GltfSceneAssetInstance( + GltfSceneAsset *instanceOwner, gltfio::FilamentInstance *instance, Engine *engine, utils::NameComponentManager* ncm, MaterialInstance **materialInstances = nullptr, size_t materialInstanceCount = 0, - int instanceIndex = -1) : _ncm(ncm), + int instanceIndex = -1) : _instanceOwner(instanceOwner), + _ncm(ncm), _instance(instance), _materialInstances(materialInstances), _materialInstanceCount(materialInstanceCount) @@ -43,6 +46,10 @@ namespace thermion return std::nullptr_t(); }; + void destroyInstance(SceneAsset *instance) override { + + } + SceneAssetType getType() override { return SceneAsset::SceneAssetType::Gltf; @@ -53,6 +60,8 @@ namespace thermion return true; } + SceneAsset *getInstanceOwner() override; + utils::Entity getEntity() override { return _instance->getRoot(); @@ -156,8 +165,9 @@ namespace thermion filament::Engine *_engine; utils::NameComponentManager *_ncm; gltfio::FilamentInstance *_instance; - MaterialInstance **_materialInstances = nullptr; + MaterialInstance **_materialInstances = std::nullptr_t(); size_t _materialInstanceCount = 0; + GltfSceneAsset *_instanceOwner = std::nullptr_t(); }; } // namespace thermion \ No newline at end of file diff --git a/thermion_dart/native/include/scene/GridOverlay.hpp b/thermion_dart/native/include/scene/GridOverlay.hpp index b8a57eee..b51cd813 100644 --- a/thermion_dart/native/include/scene/GridOverlay.hpp +++ b/thermion_dart/native/include/scene/GridOverlay.hpp @@ -22,9 +22,14 @@ public: SceneAssetType getType() override { return SceneAsset::SceneAssetType::Gizmo; } bool isInstance() override { return false; } + SceneAsset *getInstanceOwner() override { return std::nullptr_t(); } SceneAsset* createInstance(MaterialInstance** materialInstances = nullptr, size_t materialInstanceCount = 0) override; + + void destroyInstance(SceneAsset *instance) override { + + } MaterialInstance** getMaterialInstances() override { return &_materialInstance; } size_t getMaterialInstanceCount() override { return 1; } diff --git a/thermion_dart/native/include/scene/SceneAsset.hpp b/thermion_dart/native/include/scene/SceneAsset.hpp index 11336654..418cc002 100644 --- a/thermion_dart/native/include/scene/SceneAsset.hpp +++ b/thermion_dart/native/include/scene/SceneAsset.hpp @@ -30,6 +30,9 @@ class SceneAsset { } virtual bool isInstance() = 0; + virtual SceneAsset* getInstanceOwner() = 0; + virtual void destroyInstance(SceneAsset *instance) = 0; + virtual SceneAsset* createInstance(MaterialInstance **materialInstances, size_t materialInstanceCount) = 0; diff --git a/thermion_dart/native/src/scene/GeometrySceneAsset.cpp b/thermion_dart/native/src/scene/GeometrySceneAsset.cpp index 4b2e0e2c..90e1dc9a 100644 --- a/thermion_dart/native/src/scene/GeometrySceneAsset.cpp +++ b/thermion_dart/native/src/scene/GeometrySceneAsset.cpp @@ -19,16 +19,15 @@ namespace thermion using namespace filament; GeometrySceneAsset::GeometrySceneAsset( - bool isInstance, Engine *engine, VertexBuffer *vertexBuffer, IndexBuffer *indexBuffer, MaterialInstance **materialInstances, size_t materialInstanceCount, RenderableManager::PrimitiveType primitiveType, - Box boundingBox) - : _isInstance(isInstance), - _engine(engine), _vertexBuffer(vertexBuffer), _indexBuffer(indexBuffer), _materialInstances(materialInstances), _materialInstanceCount(materialInstanceCount), _primitiveType(primitiveType), _boundingBox(boundingBox) + Box boundingBox, + GeometrySceneAsset *instanceOwner) + : _engine(engine), _vertexBuffer(vertexBuffer), _indexBuffer(indexBuffer), _materialInstances(materialInstances), _materialInstanceCount(materialInstanceCount), _primitiveType(primitiveType), _boundingBox(boundingBox), _instanceOwner(instanceOwner) { _entity = utils::EntityManager::get().create(); @@ -49,75 +48,40 @@ namespace thermion { if (_engine) { - if (_vertexBuffer && !_isInstance) + if (_vertexBuffer && !isInstance()) _engine->destroy(_vertexBuffer); - if (_indexBuffer && !_isInstance) + if (_indexBuffer && !isInstance()) _engine->destroy(_indexBuffer); } } SceneAsset *GeometrySceneAsset::createInstance(MaterialInstance **materialInstances, size_t materialInstanceCount) { - if (_isInstance) + if (isInstance()) { Log("Cannot create an instance from another instance. Ensure you are calling createInstance with the original asset."); return nullptr; } std::unique_ptr instance = std::make_unique( - true, _engine, _vertexBuffer, _indexBuffer, materialInstances, materialInstanceCount, _primitiveType, - _boundingBox); + _boundingBox, + this); auto *raw = instance.get(); _instances.push_back(std::move(instance)); return raw; } - // std::unique_ptr GeometrySceneAsset::create( - // float *vertices, uint32_t numVertices, - // float *normals, uint32_t numNormals, - // float *uvs, uint32_t numUvs, - // uint16_t *indices, uint32_t numIndices, - // MaterialInstance **materialInstances, - // size_t materialInstanceCount, - // RenderableManager::PrimitiveType primitiveType, - // Engine *engine) - // { - - // // Setup texture if needed - // if (asset && uvs && numUvs > 0 && - // asset->getMaterialInstance() && - // asset->getMaterialInstance()->getMaterial()->hasParameter("baseColorMap")) - // { - // static constexpr uint32_t textureSize = 1; - // static constexpr uint32_t white = 0x00ffffff; - - // auto texture = Texture::Builder() - // .width(textureSize) - // .height(textureSize) - // .levels(1) - // .format(Texture::InternalFormat::RGBA8) - // .build(*engine); - - // filament::backend::PixelBufferDescriptor pbd( - // &white, 4, Texture::Format::RGBA, Texture::Type::UBYTE); - // texture->setImage(*engine, 0, std::move(pbd)); - - // TextureSampler sampler( - // TextureSampler::MinFilter::NEAREST, - // TextureSampler::MagFilter::NEAREST); - // sampler.setWrapModeS(TextureSampler::WrapMode::REPEAT); - // sampler.setWrapModeT(TextureSampler::WrapMode::REPEAT); - - // asset->getMaterialInstance()->setParameter("baseColorMap", texture, sampler); - // } - - // return asset; - // } + void GeometrySceneAsset::destroyInstance(SceneAsset *asset) + { + auto it = std::remove_if(_instances.begin(), _instances.end(), [=](auto &sceneAsset) + { return sceneAsset.get() == asset; }); + _instances.erase(it, _instances.end()); + } } // namespace thermion \ No newline at end of file diff --git a/thermion_dart/native/src/scene/GeometrySceneAssetBuilder.cpp b/thermion_dart/native/src/scene/GeometrySceneAssetBuilder.cpp index a157b03a..4bb8ba0c 100644 --- a/thermion_dart/native/src/scene/GeometrySceneAssetBuilder.cpp +++ b/thermion_dart/native/src/scene/GeometrySceneAssetBuilder.cpp @@ -107,7 +107,6 @@ namespace thermion boundingBox.getMax().x, boundingBox.getMax().y, boundingBox.getMax().z); auto asset = std::make_unique( - false, mEngine, vertexBuffer, indexBuffer, diff --git a/thermion_dart/native/src/scene/GltfSceneAsset.cpp b/thermion_dart/native/src/scene/GltfSceneAsset.cpp index 2bc3a694..33127738 100644 --- a/thermion_dart/native/src/scene/GltfSceneAsset.cpp +++ b/thermion_dart/native/src/scene/GltfSceneAsset.cpp @@ -49,6 +49,7 @@ namespace thermion } std::unique_ptr sceneAssetInstance = std::make_unique( + this, instance, _engine, _ncm, diff --git a/thermion_dart/native/src/scene/GltfSceneAssetInstance.cpp b/thermion_dart/native/src/scene/GltfSceneAssetInstance.cpp index 744072c7..31dc276e 100644 --- a/thermion_dart/native/src/scene/GltfSceneAssetInstance.cpp +++ b/thermion_dart/native/src/scene/GltfSceneAssetInstance.cpp @@ -1,7 +1,7 @@ - #include "scene/GltfSceneAssetInstance.hpp" -#include "gltfio/FilamentInstance.h" -#include "Log.hpp" +#include "scene/GltfSceneAsset.hpp" + + namespace thermion { @@ -10,5 +10,9 @@ namespace thermion } + SceneAsset *GltfSceneAssetInstance::getInstanceOwner() { + return static_cast(_instanceOwner); + } + } \ No newline at end of file diff --git a/thermion_dart/native/src/scene/SceneManager.cpp b/thermion_dart/native/src/scene/SceneManager.cpp index 8f146a14..6a128177 100644 --- a/thermion_dart/native/src/scene/SceneManager.cpp +++ b/thermion_dart/native/src/scene/SceneManager.cpp @@ -157,12 +157,13 @@ namespace thermion { if (!_grid) { - if(!material) { + if (!material) + { material = Material::Builder() - .package(GRID_PACKAGE, GRID_GRID_SIZE) - .build(*_engine); + .package(GRID_PACKAGE, GRID_GRID_SIZE) + .build(*_engine); } - + _grid = std::make_unique(*_engine, material); } return _grid.get(); @@ -344,7 +345,7 @@ namespace thermion _sceneAssets.push_back(std::move(sceneAsset)); - Log("Finished loading glTF from %s", uri); + Log("Loaded glTF asset from uri: %s", uri); return raw; } @@ -492,22 +493,25 @@ namespace thermion std::lock_guard lock(_mutex); - auto it = std::remove_if(_sceneAssets.begin(), _sceneAssets.end(), [=](auto &sceneAsset) - { return sceneAsset.get() == asset; }); - if (it != _sceneAssets.end()) + auto entity = asset->getEntity(); + _collisionComponentManager->removeComponent(entity); + _animationManager->removeAnimationComponent(utils::Entity::smuggle(entity)); + for (int i = 0; i < asset->getChildEntityCount(); i++) { - auto entity = (*it)->getEntity(); - _collisionComponentManager->removeComponent(entity); - _animationManager->removeAnimationComponent(utils::Entity::smuggle(entity)); - for (int i = 0; i < (*it)->getChildEntityCount(); i++) - { - auto childEntity = (*it)->getChildEntities()[i]; - _collisionComponentManager->removeComponent(childEntity); - _animationManager->removeAnimationComponent(utils::Entity::smuggle(childEntity)); - } - (*it)->removeAllEntities(_scene); + auto childEntity = asset->getChildEntities()[i]; + _collisionComponentManager->removeComponent(childEntity); + _animationManager->removeAnimationComponent(utils::Entity::smuggle(childEntity)); + } + asset->removeAllEntities(_scene); + if (asset->isInstance()) + { + asset->destroyInstance(asset); + } + else + { + auto it = std::remove_if(_sceneAssets.begin(), _sceneAssets.end(), [=](auto &sceneAsset) + { return sceneAsset.get() == asset; }); _sceneAssets.erase(it, _sceneAssets.end()); - return; } } diff --git a/thermion_dart/test/geometry_tests.dart b/thermion_dart/test/geometry_tests.dart index 925bbc87..cf16c2dc 100644 --- a/thermion_dart/test/geometry_tests.dart +++ b/thermion_dart/test/geometry_tests.dart @@ -22,7 +22,7 @@ void main() async { final cube = await viewer .createGeometry(GeometryHelper.cube(normals: false, uvs: false)); await testHelper.capture(viewer, "geometry_cube_no_normals_uvs"); - await viewer.removeEntity(cube); + await viewer.removeAsset(cube); await testHelper.capture(viewer, "geometry_remove_cube"); }); }); @@ -86,6 +86,8 @@ void main() async { instance.entity, Matrix4.translation(Vector3.all(1))); await testHelper.capture(viewer, "geometry_instanced"); + await viewer.removeAsset(instance); + await testHelper.capture(viewer, "geometry_instance_removed"); }); }); @@ -167,7 +169,7 @@ void main() async { "baseColorFactor", 0.0, 1.0, 0.0, 0.0); await testHelper.capture( viewer, "geometry_cube_with_custom_material_ubershader"); - await viewer.removeEntity(cube); + await viewer.removeAsset(cube); }); }); @@ -191,7 +193,7 @@ void main() async { await viewer.applyTexture(texture as ThermionFFITexture, cube.entity); await testHelper.capture( viewer, "geometry_cube_with_custom_material_ubershader_texture"); - await viewer.removeEntity(cube); + await viewer.removeAsset(cube); await viewer.destroyTexture(texture); }); @@ -221,7 +223,7 @@ void main() async { var texture = await viewer.createTexture(textureData); await viewer.applyTexture(texture, cube.entity); await testHelper.capture(viewer, "unlit_material_texture_only"); - await viewer.removeEntity(cube); + await viewer.removeAsset(cube); }); }); diff --git a/thermion_dart/test/gltf_tests.dart b/thermion_dart/test/gltf_tests.dart index 306eac4e..dd288e39 100644 --- a/thermion_dart/test/gltf_tests.dart +++ b/thermion_dart/test/gltf_tests.dart @@ -16,7 +16,7 @@ void main() async { var model = await viewer .loadGlb("file://${testHelper.testDir}/assets/cube.glb"); await testHelper.capture(viewer, "load_glb_from_file"); - await viewer.removeEntity(model); + await viewer.removeAsset(model); }); }); @@ -29,6 +29,28 @@ void main() async { }); }); + test('load glb from buffer with instances', () async { + await testHelper.withViewer((viewer) async { + var buffer = + File("${testHelper.testDir}/assets/cube.glb").readAsBytesSync(); + var model = await viewer.loadGlbFromBuffer(buffer, numInstances: 2); + var instance = await model.createInstance(); + await instance.addToScene(); + await viewer.setTransform( + instance.entity, Matrix4.translation(Vector3(1, 0, 0))); + + await testHelper.capture(viewer, "load_glb_from_buffer_with_instances"); + + await viewer.removeAsset(instance); + + await testHelper.capture(viewer, "load_glb_from_buffer_instance_removed"); + + await viewer.removeAsset(model); + + await testHelper.capture(viewer, "load_glb_from_buffer_original_removed"); + }, bg: kRed); + }); + test('load glb from buffer with priority', () async { await testHelper.withViewer((viewer) async { viewer.addDirectLight(DirectLight.sun()); @@ -58,7 +80,7 @@ void main() async { await testHelper.withViewer((viewer) async { var model = await viewer.loadGlb( "file://${testHelper.testDir}/assets/cube.glb", - numInstances: 2); + numInstances: 32); await testHelper.capture(viewer, "gltf_create_instance_0"); var instance = await model.createInstance(); await instance.addToScene(); @@ -75,17 +97,36 @@ void main() async { var model = await viewer.loadGlb( "file://${testHelper.testDir}/assets/cube.glb", numInstances: 2); - await testHelper.capture(viewer, "gltf_create_instance_with_material_0"); + await testHelper.capture( + viewer, "gltf_create_instance_with_material_0"); final materialInstance = await viewer.createUnlitMaterialInstance(); await materialInstance.setParameterFloat4( "baseColorFactor", 1.0, 0.0, 0.0, 1.0); - var instance = await model.createInstance(materialInstances: [materialInstance]); + var instance = + await model.createInstance(materialInstances: [materialInstance]); await instance.addToScene(); await viewer.setTransform( instance.entity, Matrix4.translation(Vector3.all(1))); - await testHelper.capture(viewer, "gltf_create_instance_with_material_1"); + await testHelper.capture( + viewer, "gltf_create_instance_with_material_1"); + await viewer.destroyMaterialInstance(materialInstance); + }); + }); + + test('replace material instance for gltf', () async { + await testHelper.withViewer((viewer) async { + var model = await viewer + .loadGlb("file://${testHelper.testDir}/assets/cube.glb"); + await testHelper.capture(viewer, "gltf_default_material_instance"); + var materialInstance = await viewer.createUnlitMaterialInstance(); + await materialInstance.setParameterFloat4( + "baseColorFactor", 1.0, 1.0, 0.0, 1.0); + await model.setMaterialInstanceAt(materialInstance); + await testHelper.capture(viewer, "gltf_set_material_instance"); + await viewer.removeAsset(model); + await viewer.destroyMaterialInstance(materialInstance); }); }); });