fix a Transform component leak in CameraManager

CameraManager creates a Transform component for each Camera component
is not already present. However, it didn't destroy the transform
component when it's itself destroyed. the leaked transform component
would eventually be garbage collected, but caused significant
slow down and memory pressure. This is because camera components are
created every frame for the shadow maps.

FIXES=[303914944]

filament/src/components/CameraManager.cpp

@@ -23,70 +23,82 @@
 #include <utils/Log.h>
 #include <utils/debug.h>
 
-#include <math/mat4.h>
-
 using namespace utils;
 using namespace filament::math;
 
 namespace filament {
 
-FCameraManager::FCameraManager(FEngine& engine) noexcept
-        : mEngine(engine) {
+FCameraManager::FCameraManager(FEngine&) noexcept {
 }
 
-FCameraManager::~FCameraManager() noexcept {
-}
+FCameraManager::~FCameraManager() noexcept = default;
 
-void FCameraManager::terminate() noexcept {
+void FCameraManager::terminate(FEngine& engine) noexcept {
     auto& manager = mManager;
     if (!manager.empty()) {
 #ifndef NDEBUG
         slog.d << "cleaning up " << manager.getComponentCount()
                << " leaked Camera components" << io::endl;
 #endif
-        while (!manager.empty()) {
-            Instance const ci = manager.end() - 1;
-            destroy(manager.getEntity(ci));
+        utils::Slice<Entity> const entities{ manager.getEntities(), manager.getComponentCount() };
+        for (Entity const e : entities) {
+            destroy(engine, e);
         }
     }
 }
 
-void FCameraManager::gc(utils::EntityManager& em) noexcept {
+void FCameraManager::gc(FEngine& engine, utils::EntityManager& em) noexcept {
     auto& manager = mManager;
-    manager.gc(em, 4, [this](Entity e) {
-        destroy(e);
+    manager.gc(em, 4, [this, &engine](Entity e) {
+        destroy(engine, e);
     });
 }
 
-FCamera* FCameraManager::create(Entity entity) {
-    FEngine& engine = mEngine;
+FCamera* FCameraManager::create(FEngine& engine, Entity entity) {
     auto& manager = mManager;
 
+    // if this entity already has Camera component, destroy it.
     if (UTILS_UNLIKELY(manager.hasComponent(entity))) {
-        destroy(entity);
+        destroy(engine, entity);
     }
+
+    // add the Camera component to the entity
     Instance const i = manager.addComponent(entity);
 
-    FCamera* camera = engine.getHeapAllocator().make<FCamera>(engine, entity);
+    // For historical reasons, FCamera must not move. So the CameraManager stores a pointer.
+    FCamera* const camera = engine.getHeapAllocator().make<FCamera>(engine, entity);
     manager.elementAt<CAMERA>(i) = camera;
+    manager.elementAt<OWNS_TRANSFORM_COMPONENT>(i) = false;
 
     // Make sure we have a transform component
-    FTransformManager& transformManager = engine.getTransformManager();
-    if (!transformManager.hasComponent(entity)) {
-        transformManager.create(entity);
+    FTransformManager& tcm = engine.getTransformManager();
+    if (!tcm.hasComponent(entity)) {
+        tcm.create(entity);
+        manager.elementAt<OWNS_TRANSFORM_COMPONENT>(i) = true;
     }
     return camera;
 }
 
-void FCameraManager::destroy(Entity e) noexcept {
+void FCameraManager::destroy(FEngine& engine, Entity e) noexcept {
     auto& manager = mManager;
-    Instance const i = manager.getInstance(e);
-    if (i) {
-        FCamera* camera = manager.elementAt<CAMERA>(i);
-        assert_invariant(camera);
-        camera->terminate(mEngine);
-        mEngine.getHeapAllocator().destroy(camera);
-        manager.removeComponent(e);
+    if (Instance const i = manager.getInstance(e) ; i) {
+        // destroy the FCamera object
+        bool const ownsTransformComponent = manager.elementAt<OWNS_TRANSFORM_COMPONENT>(i);
+
+        { // scope for camera -- it's invalid after this scope.
+            FCamera* const camera = manager.elementAt<CAMERA>(i);
+            assert_invariant(camera);
+            camera->terminate(engine);
+            engine.getHeapAllocator().destroy(camera);
+
+            // Remove the camera component
+            manager.removeComponent(e);
+        }
+
+        // if we added the transform component, remove it.
+        if (ownsTransformComponent) {
+            engine.getTransformManager().destroy(e);
+        }
     }
 }
 

filament/src/components/CameraManager.h

@@ -44,9 +44,9 @@ public:
     ~FCameraManager() noexcept;
 
     // free-up all resources
-    void terminate() noexcept;
+    void terminate(FEngine& engine) noexcept;
 
-    void gc(utils::EntityManager& em) noexcept;
+    void gc(FEngine& engine, utils::EntityManager& em) noexcept;
 
     /*
     * Component Manager APIs
@@ -64,25 +64,24 @@ public:
         return mManager.elementAt<CAMERA>(i);
     }
 
-    FCamera* create(utils::Entity entity);
+    FCamera* create(FEngine& engine, utils::Entity entity);
 
-    void destroy(utils::Entity e) noexcept;
+    void destroy(FEngine& engine, utils::Entity e) noexcept;
 
 private:
 
     enum {
-        CAMERA
+        CAMERA,
+        OWNS_TRANSFORM_COMPONENT
     };
 
-    using Base = utils::SingleInstanceComponentManager<FCamera *>;
+    using Base = utils::SingleInstanceComponentManager<FCamera*, bool>;
 
     struct CameraManagerImpl : public Base {
         using Base::gc;
         using Base::swap;
         using Base::hasComponent;
     } mManager;
-
-    FEngine& mEngine;
 };
 
 } // namespace filament

filament/src/details/Engine.cpp

@@ -424,7 +424,7 @@ void FEngine::shutdown() {
     mDFG.terminate(*this);                  // free-up the DFG
     mRenderableManager.terminate();         // free-up all renderables
     mLightManager.terminate();              // free-up all lights
-    mCameraManager.terminate();             // free-up all cameras
+    mCameraManager.terminate(*this);        // free-up all cameras
 
     driver.destroyRenderPrimitive(mFullScreenTriangleRph);
     destroy(mFullScreenTriangleIb);
@@ -537,7 +537,7 @@ void FEngine::gc() {
     mRenderableManager.gc(em);
     mLightManager.gc(em);
     mTransformManager.gc(em);
-    mCameraManager.gc(em);
+    mCameraManager.gc(*this, em);
 }
 
 void FEngine::flush() {
@@ -828,7 +828,7 @@ FSwapChain* FEngine::createSwapChain(uint32_t width, uint32_t height, uint64_t f
 
 
 FCamera* FEngine::createCamera(Entity entity) noexcept {
-    return mCameraManager.create(entity);
+    return mCameraManager.create(*this, entity);
 }
 
 FCamera* FEngine::getCameraComponent(Entity entity) noexcept {
@@ -837,7 +837,7 @@ FCamera* FEngine::getCameraComponent(Entity entity) noexcept {
 }
 
 void FEngine::destroyCameraComponent(utils::Entity entity) noexcept {
-    mCameraManager.destroy(entity);
+    mCameraManager.destroy(*this, entity);
 }
 
 
@@ -1053,7 +1053,7 @@ void FEngine::destroy(Entity e) {
     mRenderableManager.destroy(e);
     mLightManager.destroy(e);
     mTransformManager.destroy(e);
-    mCameraManager.destroy(e);
+    mCameraManager.destroy(*this, e);
 }
 
 bool FEngine::isValid(const FBufferObject* p) {