From 42824b8a22c8141f4b1e761f831890bac96f8789 Mon Sep 17 00:00:00 2001 From: Mikael Dallaire Cote <110583667+0mdc@users.noreply.github.com> Date: Sun, 25 Jun 2023 16:21:19 -0400 Subject: [PATCH 1/9] Header clean-up in player and replay renderers. --- src/esp/gfx/replay/Player.cpp | 7 +------ src/esp/sim/AbstractReplayRenderer.cpp | 3 --- src/esp/sim/AbstractReplayRenderer.h | 7 ------- src/esp/sim/BatchReplayRenderer.cpp | 2 -- src/esp/sim/ClassicReplayRenderer.cpp | 5 +---- 5 files changed, 2 insertions(+), 22 deletions(-) diff --git a/src/esp/gfx/replay/Player.cpp b/src/esp/gfx/replay/Player.cpp index 08e150ee3d..08f12a783b 100644 --- a/src/esp/gfx/replay/Player.cpp +++ b/src/esp/gfx/replay/Player.cpp @@ -4,15 +4,10 @@ #include "Player.h" +#include #include -#include "esp/assets/ResourceManager.h" -#include "esp/core/Esp.h" -#include "esp/gfx/replay/Keyframe.h" #include "esp/io/Json.h" -#include "esp/io/JsonAllTypes.h" - -#include namespace esp { namespace gfx { diff --git a/src/esp/sim/AbstractReplayRenderer.cpp b/src/esp/sim/AbstractReplayRenderer.cpp index 73c88ff927..2f202d31fd 100644 --- a/src/esp/sim/AbstractReplayRenderer.cpp +++ b/src/esp/sim/AbstractReplayRenderer.cpp @@ -4,9 +4,6 @@ #include "AbstractReplayRenderer.h" -#include -#include - #include "esp/gfx/replay/Player.h" namespace esp { diff --git a/src/esp/sim/AbstractReplayRenderer.h b/src/esp/sim/AbstractReplayRenderer.h index d1308c816b..b3a39b384d 100644 --- a/src/esp/sim/AbstractReplayRenderer.h +++ b/src/esp/sim/AbstractReplayRenderer.h @@ -5,16 +5,9 @@ #ifndef ESP_SIM_ABSTRACTREPLAYRENDERER_H_ #define ESP_SIM_ABSTRACTREPLAYRENDERER_H_ -#include -#include - -#include "esp/core/Check.h" -#include "esp/core/Esp.h" #include "esp/geo/Geo.h" #include "esp/gfx/DebugLineRender.h" -#include - namespace esp { namespace gfx { diff --git a/src/esp/sim/BatchReplayRenderer.cpp b/src/esp/sim/BatchReplayRenderer.cpp index c8101ae893..9cd88e145c 100644 --- a/src/esp/sim/BatchReplayRenderer.cpp +++ b/src/esp/sim/BatchReplayRenderer.cpp @@ -8,10 +8,8 @@ #include "esp/sensor/CameraSensor.h" #include -#include #include #include -#include #include namespace esp { diff --git a/src/esp/sim/ClassicReplayRenderer.cpp b/src/esp/sim/ClassicReplayRenderer.cpp index a75168ffef..806e856263 100644 --- a/src/esp/sim/ClassicReplayRenderer.cpp +++ b/src/esp/sim/ClassicReplayRenderer.cpp @@ -6,11 +6,8 @@ #include "esp/assets/ResourceManager.h" #include "esp/gfx/RenderTarget.h" -#include "esp/gfx/Renderer.h" #include "esp/metadata/MetadataMediator.h" -#include "esp/sensor/Sensor.h" #include "esp/sensor/SensorFactory.h" -#include "esp/sim/SimulatorConfiguration.h" #include #include @@ -31,7 +28,7 @@ ClassicReplayRenderer::ClassicReplayRenderer( resourceManager_ = std::make_unique(std::move(metadataMediator)); - // hack to get ReplicCAD non-baked stages to render correctly + // hack to get ReplicaCAD non-baked stages to render correctly resourceManager_->getShaderManager().setFallback( esp::gfx::getDefaultLights()); From fc1c70155754a88008a382e0d5fe9fba547a0119 Mon Sep 17 00:00:00 2001 From: Mikael Dallaire Cote <110583667+0mdc@users.noreply.github.com> Date: Sun, 25 Jun 2023 17:49:48 -0400 Subject: [PATCH 2/9] Implement partial scene deletion when using the batch renderer. --- src/esp/gfx/replay/Player.cpp | 82 ++++++++++++++++++++++++----- src/esp/gfx/replay/Player.h | 31 +++++++++-- src/esp/sim/BatchReplayRenderer.cpp | 12 +++++ 3 files changed, 108 insertions(+), 17 deletions(-) diff --git a/src/esp/gfx/replay/Player.cpp b/src/esp/gfx/replay/Player.cpp index 08f12a783b..3aef4f9ed5 100644 --- a/src/esp/gfx/replay/Player.cpp +++ b/src/esp/gfx/replay/Player.cpp @@ -64,6 +64,17 @@ void AbstractSceneGraphPlayerImplementation::setNodeTransform( .setRotation(rotation); } +void AbstractSceneGraphPlayerImplementation::setNodeTransform( + const NodeHandle node, + const Mn::Matrix4& transform) { + (*reinterpret_cast(node)).setTransformation(transform); +} + +const Mn::Matrix4 AbstractSceneGraphPlayerImplementation::getNodeTransform( + const NodeHandle node) const { + return (*reinterpret_cast(node)).transformation(); +} + void AbstractSceneGraphPlayerImplementation::setNodeSemanticId( const NodeHandle node, const unsigned id) { @@ -165,6 +176,7 @@ void Player::clearFrame() { implementation_->deleteAssetInstances(createdInstances_); createdInstances_.clear(); assetInfos_.clear(); + creationInfos_.clear(); frameIndex_ = -1; } @@ -178,15 +190,6 @@ void Player::applyKeyframe(const Keyframe& keyframe) { assetInfos_[assetInfo.filepath] = assetInfo; } - // If all current instances are being deleted, clear the frame. This enables - // the implementation to clear its memory and optimize its internals. - bool frameCleared = keyframe.deletions.size() > 0 && - createdInstances_.size() == keyframe.deletions.size(); - if (frameCleared) { - implementation_->deleteAssetInstances(createdInstances_); - createdInstances_.clear(); - } - for (const auto& pair : keyframe.creations) { const auto& creation = pair.second; @@ -218,27 +221,78 @@ void Player::applyKeyframe(const Keyframe& keyframe) { const auto& instanceKey = pair.first; CORRADE_INTERNAL_ASSERT(createdInstances_.count(instanceKey) == 0); createdInstances_[instanceKey] = node; + creationInfos_[instanceKey] = adjustedCreation; } - if (!frameCleared) { + // TODO: Move the code below into renderer-specific implementations. + bool isClassicReplayRenderer = + dynamic_cast( + implementation_.get()) != nullptr; + if (isClassicReplayRenderer) { for (const auto& deletionInstanceKey : keyframe.deletions) { const auto& it = createdInstances_.find(deletionInstanceKey); if (it == createdInstances_.end()) { - // missing instance for this key, probably due to a failed instance - // creation + // Missing instance for this key due to a failed instance creation continue; } implementation_->deleteAssetInstance(it->second); createdInstances_.erase(deletionInstanceKey); } + // The batch renderer can only clear the scene entirely; it cannot delete + // individual objects. To process deletions, all instances are deleted, + // remaining instances are re-created and latest transform updates are + // re-applied. + } else if (keyframe.deletions.size() > 0) { + // Cache latest transforms + std::unordered_map latestTransforms{}; + for (const auto& pair : this->createdInstances_) { + const RenderAssetInstanceKey key = pair.first; + latestTransforms[key] = implementation_->getNodeTransform(pair.second); + } + + // Delete all instances + implementation_->deleteAssetInstances(createdInstances_); + + // Remove deleted instances from records + for (const auto& deletion : keyframe.deletions) { + const auto& createInstanceIt = createdInstances_.find(deletion); + if (createInstanceIt == createdInstances_.end()) { + // Missing instance for this key due to a failed instance creation + continue; + } + createdInstances_.erase(createInstanceIt); + const auto& creationInfoIt = creationInfos_.find(deletion); + if (creationInfoIt == creationInfos_.end()) { + // Missing instance for this key due to a failed instance creation + continue; + } + creationInfos_.erase(creationInfoIt); + } + + for (const auto& pair : createdInstances_) { + const RenderAssetInstanceKey key = pair.first; + const auto& creationInfoIt = creationInfos_.find(key); + if (creationInfoIt == creationInfos_.end()) { + // Missing instance for this key due to a failed instance creation + continue; + } + const auto& creationInfo = creationInfoIt->second; + auto instance = implementation_->loadAndCreateRenderAssetInstance( + assetInfos_[creationInfo.filepath], creationInfo); + + // Replace dangling reference + createdInstances_[key] = instance; + + // Re-apply latest transform updates + implementation_->setNodeTransform(instance, latestTransforms[key]); + } } for (const auto& pair : keyframe.stateUpdates) { const auto& it = createdInstances_.find(pair.first); if (it == createdInstances_.end()) { - // missing instance for this key, probably due to a failed instance - // creation + // Missing instance for this key due to a failed instance creation continue; } auto* node = it->second; diff --git a/src/esp/gfx/replay/Player.h b/src/esp/gfx/replay/Player.h index 53c51eef75..1658132dba 100644 --- a/src/esp/gfx/replay/Player.h +++ b/src/esp/gfx/replay/Player.h @@ -85,15 +85,32 @@ class AbstractPlayerImplementation { instances) = 0; /** - * @brief Set node transform + * @brief Set node transform from translation and rotation components. * * The @p handle is expected to be returned from an earlier call to * @ref loadAndCreateRenderAssetInstance() on the same instance. */ - virtual void setNodeTransform(NodeHandle node, + virtual void setNodeTransform(const NodeHandle node, const Magnum::Vector3& translation, const Magnum::Quaternion& rotation) = 0; + /** + * @brief Set node transform. + * + * The @p handle is expected to be returned from an earlier call to + * @ref loadAndCreateRenderAssetInstance() on the same instance. + */ + virtual void setNodeTransform(const NodeHandle node, + const Mn::Matrix4& transform) = 0; + + /** + * @brief Get node transform. + * + * The @p handle is expected to be returned from an earlier call to + * @ref loadAndCreateRenderAssetInstance() on the same instance. + */ + virtual const Mn::Matrix4 getNodeTransform(const NodeHandle node) const = 0; + /** * @brief Set node semantic ID * @@ -127,10 +144,15 @@ class AbstractSceneGraphPlayerImplementation const std::unordered_map& instances) override; - void setNodeTransform(NodeHandle node, + void setNodeTransform(const NodeHandle node, const Magnum::Vector3& translation, const Magnum::Quaternion& rotation) override; + void setNodeTransform(const NodeHandle node, + const Mn::Matrix4& transform) override; + + const Mn::Matrix4 getNodeTransform(const NodeHandle node) const override; + void setNodeSemanticId(NodeHandle node, unsigned id) override; }; @@ -258,6 +280,9 @@ class Player { std::vector keyframes_; std::unordered_map assetInfos_; std::unordered_map createdInstances_; + std::unordered_map + creationInfos_; std::set failedFilepaths_; ESP_SMART_POINTERS(Player) diff --git a/src/esp/sim/BatchReplayRenderer.cpp b/src/esp/sim/BatchReplayRenderer.cpp index 9cd88e145c..10082081f0 100644 --- a/src/esp/sim/BatchReplayRenderer.cpp +++ b/src/esp/sim/BatchReplayRenderer.cpp @@ -145,6 +145,18 @@ BatchReplayRenderer::BatchReplayRenderer( Mn::Matrix4::from(rotation.toMatrix(), translation); } + void setNodeTransform(const gfx::replay::NodeHandle node, + const Mn::Matrix4& transform) override { + renderer_.transformations( + sceneId_)[reinterpret_cast(node) - 1] = transform; + } + + const Mn::Matrix4 getNodeTransform( + const gfx::replay::NodeHandle node) const override { + return renderer_.transformations( + sceneId_)[reinterpret_cast(node) - 1]; + } + void changeLightSetup(const esp::gfx::LightSetup& lights) override { if (!renderer_.maxLightCount()) { ESP_WARNING() << "Attempted to change" << lights.size() From c2f1362d5eb75d55ee9f8a810457e48929fde5fb Mon Sep 17 00:00:00 2001 From: Mikael Dallaire Cote <110583667+0mdc@users.noreply.github.com> Date: Mon, 26 Jun 2023 14:26:49 -0400 Subject: [PATCH 3/9] Separate batch replay renderer deletions from the main keyframe processing method. --- src/esp/gfx/replay/Player.cpp | 104 ++++++++++++++++++---------------- src/esp/gfx/replay/Player.h | 1 + 2 files changed, 57 insertions(+), 48 deletions(-) diff --git a/src/esp/gfx/replay/Player.cpp b/src/esp/gfx/replay/Player.cpp index 3aef4f9ed5..f21e3da313 100644 --- a/src/esp/gfx/replay/Player.cpp +++ b/src/esp/gfx/replay/Player.cpp @@ -239,54 +239,8 @@ void Player::applyKeyframe(const Keyframe& keyframe) { implementation_->deleteAssetInstance(it->second); createdInstances_.erase(deletionInstanceKey); } - // The batch renderer can only clear the scene entirely; it cannot delete - // individual objects. To process deletions, all instances are deleted, - // remaining instances are re-created and latest transform updates are - // re-applied. - } else if (keyframe.deletions.size() > 0) { - // Cache latest transforms - std::unordered_map latestTransforms{}; - for (const auto& pair : this->createdInstances_) { - const RenderAssetInstanceKey key = pair.first; - latestTransforms[key] = implementation_->getNodeTransform(pair.second); - } - - // Delete all instances - implementation_->deleteAssetInstances(createdInstances_); - - // Remove deleted instances from records - for (const auto& deletion : keyframe.deletions) { - const auto& createInstanceIt = createdInstances_.find(deletion); - if (createInstanceIt == createdInstances_.end()) { - // Missing instance for this key due to a failed instance creation - continue; - } - createdInstances_.erase(createInstanceIt); - const auto& creationInfoIt = creationInfos_.find(deletion); - if (creationInfoIt == creationInfos_.end()) { - // Missing instance for this key due to a failed instance creation - continue; - } - creationInfos_.erase(creationInfoIt); - } - - for (const auto& pair : createdInstances_) { - const RenderAssetInstanceKey key = pair.first; - const auto& creationInfoIt = creationInfos_.find(key); - if (creationInfoIt == creationInfos_.end()) { - // Missing instance for this key due to a failed instance creation - continue; - } - const auto& creationInfo = creationInfoIt->second; - auto instance = implementation_->loadAndCreateRenderAssetInstance( - assetInfos_[creationInfo.filepath], creationInfo); - - // Replace dangling reference - createdInstances_[key] = instance; - - // Re-apply latest transform updates - implementation_->setNodeTransform(instance, latestTransforms[key]); - } + } else { + processBatchRendererDeletions(keyframe); } for (const auto& pair : keyframe.stateUpdates) { @@ -307,6 +261,60 @@ void Player::applyKeyframe(const Keyframe& keyframe) { } } +void Player::processBatchRendererDeletions(const Keyframe& keyframe) { + // The batch renderer can only clear the scene entirely; it cannot delete + // individual objects. To process deletions, all instances are deleted, + // remaining instances are re-created and latest transform updates are + // re-applied. + if (keyframe.deletions.size() > 0) { + return; + } + + // Cache latest transforms + std::unordered_map latestTransforms{}; + for (const auto& pair : this->createdInstances_) { + const RenderAssetInstanceKey key = pair.first; + latestTransforms[key] = implementation_->getNodeTransform(pair.second); + } + + // Delete all instances + implementation_->deleteAssetInstances(createdInstances_); + + // Remove deleted instances from records + for (const auto& deletion : keyframe.deletions) { + const auto& createInstanceIt = createdInstances_.find(deletion); + if (createInstanceIt == createdInstances_.end()) { + // Missing instance for this key due to a failed instance creation + continue; + } + createdInstances_.erase(createInstanceIt); + const auto& creationInfoIt = creationInfos_.find(deletion); + if (creationInfoIt == creationInfos_.end()) { + // Missing instance for this key due to a failed instance creation + continue; + } + creationInfos_.erase(creationInfoIt); + } + + for (const auto& pair : createdInstances_) { + const RenderAssetInstanceKey key = pair.first; + const auto& creationInfoIt = creationInfos_.find(key); + if (creationInfoIt == creationInfos_.end()) { + // Missing instance for this key due to a failed instance creation + continue; + } + const auto& creationInfo = creationInfoIt->second; + auto instance = implementation_->loadAndCreateRenderAssetInstance( + assetInfos_[creationInfo.filepath], creationInfo); + + // Replace dangling reference + createdInstances_[key] = instance; + + // Re-apply latest transform updates + implementation_->setNodeTransform(instance, latestTransforms[key]); + } +} + void Player::appendKeyframe(Keyframe&& keyframe) { keyframes_.emplace_back(std::move(keyframe)); } diff --git a/src/esp/gfx/replay/Player.h b/src/esp/gfx/replay/Player.h index 1658132dba..7d7af270fc 100644 --- a/src/esp/gfx/replay/Player.h +++ b/src/esp/gfx/replay/Player.h @@ -273,6 +273,7 @@ class Player { void applyKeyframe(const Keyframe& keyframe); void readKeyframesFromJsonDocument(const rapidjson::Document& d); void clearFrame(); + void processBatchRendererDeletions(const Keyframe& keyframe); std::shared_ptr implementation_; From a8ed055e10f1eea860525e4bba252956fd79be07 Mon Sep 17 00:00:00 2001 From: Mikael Dallaire Cote <110583667+0mdc@users.noreply.github.com> Date: Mon, 26 Jun 2023 15:29:49 -0400 Subject: [PATCH 4/9] Review pass. --- src/esp/gfx/replay/Player.cpp | 30 ++++++++++------------------- src/esp/gfx/replay/Player.h | 17 ++++++++-------- src/esp/sim/BatchReplayRenderer.cpp | 2 +- 3 files changed, 20 insertions(+), 29 deletions(-) diff --git a/src/esp/gfx/replay/Player.cpp b/src/esp/gfx/replay/Player.cpp index f21e3da313..d2571c8f14 100644 --- a/src/esp/gfx/replay/Player.cpp +++ b/src/esp/gfx/replay/Player.cpp @@ -70,7 +70,7 @@ void AbstractSceneGraphPlayerImplementation::setNodeTransform( (*reinterpret_cast(node)).setTransformation(transform); } -const Mn::Matrix4 AbstractSceneGraphPlayerImplementation::getNodeTransform( +Mn::Matrix4 AbstractSceneGraphPlayerImplementation::hackGetNodeTransform( const NodeHandle node) const { return (*reinterpret_cast(node)).transformation(); } @@ -240,7 +240,7 @@ void Player::applyKeyframe(const Keyframe& keyframe) { createdInstances_.erase(deletionInstanceKey); } } else { - processBatchRendererDeletions(keyframe); + hackProcessBatchRendererDeletions(keyframe); } for (const auto& pair : keyframe.stateUpdates) { @@ -261,7 +261,7 @@ void Player::applyKeyframe(const Keyframe& keyframe) { } } -void Player::processBatchRendererDeletions(const Keyframe& keyframe) { +void Player::hackProcessBatchRendererDeletions(const Keyframe& keyframe) { // The batch renderer can only clear the scene entirely; it cannot delete // individual objects. To process deletions, all instances are deleted, // remaining instances are re-created and latest transform updates are @@ -271,10 +271,10 @@ void Player::processBatchRendererDeletions(const Keyframe& keyframe) { } // Cache latest transforms - std::unordered_map latestTransforms{}; for (const auto& pair : this->createdInstances_) { const RenderAssetInstanceKey key = pair.first; - latestTransforms[key] = implementation_->getNodeTransform(pair.second); + latestTransformCache_[key] = + implementation_->hackGetNodeTransform(pair.second); } // Delete all instances @@ -288,30 +288,20 @@ void Player::processBatchRendererDeletions(const Keyframe& keyframe) { continue; } createdInstances_.erase(createInstanceIt); - const auto& creationInfoIt = creationInfos_.find(deletion); - if (creationInfoIt == creationInfos_.end()) { - // Missing instance for this key due to a failed instance creation - continue; - } - creationInfos_.erase(creationInfoIt); + creationInfos_.erase(deletion); } for (const auto& pair : createdInstances_) { - const RenderAssetInstanceKey key = pair.first; - const auto& creationInfoIt = creationInfos_.find(key); - if (creationInfoIt == creationInfos_.end()) { - // Missing instance for this key due to a failed instance creation - continue; - } - const auto& creationInfo = creationInfoIt->second; - auto instance = implementation_->loadAndCreateRenderAssetInstance( + const auto key = pair.first; + const auto& creationInfo = creationInfos_[key]; + auto* instance = implementation_->loadAndCreateRenderAssetInstance( assetInfos_[creationInfo.filepath], creationInfo); // Replace dangling reference createdInstances_[key] = instance; // Re-apply latest transform updates - implementation_->setNodeTransform(instance, latestTransforms[key]); + implementation_->setNodeTransform(instance, latestTransformCache_[key]); } } diff --git a/src/esp/gfx/replay/Player.h b/src/esp/gfx/replay/Player.h index 7d7af270fc..e6e0da3c34 100644 --- a/src/esp/gfx/replay/Player.h +++ b/src/esp/gfx/replay/Player.h @@ -90,7 +90,7 @@ class AbstractPlayerImplementation { * The @p handle is expected to be returned from an earlier call to * @ref loadAndCreateRenderAssetInstance() on the same instance. */ - virtual void setNodeTransform(const NodeHandle node, + virtual void setNodeTransform(NodeHandle node, const Magnum::Vector3& translation, const Magnum::Quaternion& rotation) = 0; @@ -100,7 +100,7 @@ class AbstractPlayerImplementation { * The @p handle is expected to be returned from an earlier call to * @ref loadAndCreateRenderAssetInstance() on the same instance. */ - virtual void setNodeTransform(const NodeHandle node, + virtual void setNodeTransform(NodeHandle node, const Mn::Matrix4& transform) = 0; /** @@ -109,7 +109,7 @@ class AbstractPlayerImplementation { * The @p handle is expected to be returned from an earlier call to * @ref loadAndCreateRenderAssetInstance() on the same instance. */ - virtual const Mn::Matrix4 getNodeTransform(const NodeHandle node) const = 0; + virtual Mn::Matrix4 hackGetNodeTransform(NodeHandle node) const = 0; /** * @brief Set node semantic ID @@ -144,14 +144,13 @@ class AbstractSceneGraphPlayerImplementation const std::unordered_map& instances) override; - void setNodeTransform(const NodeHandle node, + void setNodeTransform(NodeHandle node, const Magnum::Vector3& translation, const Magnum::Quaternion& rotation) override; - void setNodeTransform(const NodeHandle node, - const Mn::Matrix4& transform) override; + void setNodeTransform(NodeHandle node, const Mn::Matrix4& transform) override; - const Mn::Matrix4 getNodeTransform(const NodeHandle node) const override; + Mn::Matrix4 hackGetNodeTransform(NodeHandle node) const override; void setNodeSemanticId(NodeHandle node, unsigned id) override; }; @@ -273,7 +272,7 @@ class Player { void applyKeyframe(const Keyframe& keyframe); void readKeyframesFromJsonDocument(const rapidjson::Document& d); void clearFrame(); - void processBatchRendererDeletions(const Keyframe& keyframe); + void hackProcessBatchRendererDeletions(const Keyframe& keyframe); std::shared_ptr implementation_; @@ -284,6 +283,8 @@ class Player { std::unordered_map creationInfos_; + std::unordered_map + latestTransformCache_{}; std::set failedFilepaths_; ESP_SMART_POINTERS(Player) diff --git a/src/esp/sim/BatchReplayRenderer.cpp b/src/esp/sim/BatchReplayRenderer.cpp index 10082081f0..b4d862a68c 100644 --- a/src/esp/sim/BatchReplayRenderer.cpp +++ b/src/esp/sim/BatchReplayRenderer.cpp @@ -151,7 +151,7 @@ BatchReplayRenderer::BatchReplayRenderer( sceneId_)[reinterpret_cast(node) - 1] = transform; } - const Mn::Matrix4 getNodeTransform( + Mn::Matrix4 hackGetNodeTransform( const gfx::replay::NodeHandle node) const override { return renderer_.transformations( sceneId_)[reinterpret_cast(node) - 1]; From 0496cbb7077d7876ddeeea984ba2cebf68ac275a Mon Sep 17 00:00:00 2001 From: Mikael Dallaire Cote <110583667+0mdc@users.noreply.github.com> Date: Mon, 26 Jun 2023 15:34:30 -0400 Subject: [PATCH 5/9] Move cast into deletion processing function. --- src/esp/gfx/replay/Player.cpp | 92 +++++++++++++++++------------------ src/esp/gfx/replay/Player.h | 2 +- 2 files changed, 45 insertions(+), 49 deletions(-) diff --git a/src/esp/gfx/replay/Player.cpp b/src/esp/gfx/replay/Player.cpp index d2571c8f14..60c191162a 100644 --- a/src/esp/gfx/replay/Player.cpp +++ b/src/esp/gfx/replay/Player.cpp @@ -224,24 +224,7 @@ void Player::applyKeyframe(const Keyframe& keyframe) { creationInfos_[instanceKey] = adjustedCreation; } - // TODO: Move the code below into renderer-specific implementations. - bool isClassicReplayRenderer = - dynamic_cast( - implementation_.get()) != nullptr; - if (isClassicReplayRenderer) { - for (const auto& deletionInstanceKey : keyframe.deletions) { - const auto& it = createdInstances_.find(deletionInstanceKey); - if (it == createdInstances_.end()) { - // Missing instance for this key due to a failed instance creation - continue; - } - - implementation_->deleteAssetInstance(it->second); - createdInstances_.erase(deletionInstanceKey); - } - } else { - hackProcessBatchRendererDeletions(keyframe); - } + hackProcessDeletions(keyframe); for (const auto& pair : keyframe.stateUpdates) { const auto& it = createdInstances_.find(pair.first); @@ -261,47 +244,60 @@ void Player::applyKeyframe(const Keyframe& keyframe) { } } -void Player::hackProcessBatchRendererDeletions(const Keyframe& keyframe) { +void Player::hackProcessDeletions(const Keyframe& keyframe) { + // HACK: Classic and batch renderers currently handle deletions differently. // The batch renderer can only clear the scene entirely; it cannot delete // individual objects. To process deletions, all instances are deleted, // remaining instances are re-created and latest transform updates are // re-applied. - if (keyframe.deletions.size() > 0) { - return; - } + bool isClassicReplayRenderer = + dynamic_cast( + implementation_.get()) != nullptr; + if (isClassicReplayRenderer) { + for (const auto& deletionInstanceKey : keyframe.deletions) { + const auto& it = createdInstances_.find(deletionInstanceKey); + if (it == createdInstances_.end()) { + // Missing instance for this key due to a failed instance creation + continue; + } - // Cache latest transforms - for (const auto& pair : this->createdInstances_) { - const RenderAssetInstanceKey key = pair.first; - latestTransformCache_[key] = - implementation_->hackGetNodeTransform(pair.second); - } + implementation_->deleteAssetInstance(it->second); + createdInstances_.erase(deletionInstanceKey); + } + } else if (keyframe.deletions.size() > 0) { + // Cache latest transforms + for (const auto& pair : this->createdInstances_) { + const RenderAssetInstanceKey key = pair.first; + latestTransformCache_[key] = + implementation_->hackGetNodeTransform(pair.second); + } - // Delete all instances - implementation_->deleteAssetInstances(createdInstances_); + // Delete all instances + implementation_->deleteAssetInstances(createdInstances_); - // Remove deleted instances from records - for (const auto& deletion : keyframe.deletions) { - const auto& createInstanceIt = createdInstances_.find(deletion); - if (createInstanceIt == createdInstances_.end()) { - // Missing instance for this key due to a failed instance creation - continue; + // Remove deleted instances from records + for (const auto& deletion : keyframe.deletions) { + const auto& createInstanceIt = createdInstances_.find(deletion); + if (createInstanceIt == createdInstances_.end()) { + // Missing instance for this key due to a failed instance creation + continue; + } + createdInstances_.erase(createInstanceIt); + creationInfos_.erase(deletion); } - createdInstances_.erase(createInstanceIt); - creationInfos_.erase(deletion); - } - for (const auto& pair : createdInstances_) { - const auto key = pair.first; - const auto& creationInfo = creationInfos_[key]; - auto* instance = implementation_->loadAndCreateRenderAssetInstance( - assetInfos_[creationInfo.filepath], creationInfo); + for (const auto& pair : createdInstances_) { + const auto key = pair.first; + const auto& creationInfo = creationInfos_[key]; + auto* instance = implementation_->loadAndCreateRenderAssetInstance( + assetInfos_[creationInfo.filepath], creationInfo); - // Replace dangling reference - createdInstances_[key] = instance; + // Replace dangling reference + createdInstances_[key] = instance; - // Re-apply latest transform updates - implementation_->setNodeTransform(instance, latestTransformCache_[key]); + // Re-apply latest transform updates + implementation_->setNodeTransform(instance, latestTransformCache_[key]); + } } } diff --git a/src/esp/gfx/replay/Player.h b/src/esp/gfx/replay/Player.h index e6e0da3c34..44f42477e6 100644 --- a/src/esp/gfx/replay/Player.h +++ b/src/esp/gfx/replay/Player.h @@ -272,7 +272,7 @@ class Player { void applyKeyframe(const Keyframe& keyframe); void readKeyframesFromJsonDocument(const rapidjson::Document& d); void clearFrame(); - void hackProcessBatchRendererDeletions(const Keyframe& keyframe); + void hackProcessDeletions(const Keyframe& keyframe); std::shared_ptr implementation_; From 82f42b3507235a327497069a239cfe84578fc024 Mon Sep 17 00:00:00 2001 From: Mikael Dallaire Cote <110583667+0mdc@users.noreply.github.com> Date: Mon, 26 Jun 2023 19:39:09 -0400 Subject: [PATCH 6/9] Clear transform cache before use. --- src/esp/gfx/replay/Player.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/esp/gfx/replay/Player.cpp b/src/esp/gfx/replay/Player.cpp index 60c191162a..277e364aee 100644 --- a/src/esp/gfx/replay/Player.cpp +++ b/src/esp/gfx/replay/Player.cpp @@ -266,6 +266,7 @@ void Player::hackProcessDeletions(const Keyframe& keyframe) { } } else if (keyframe.deletions.size() > 0) { // Cache latest transforms + latestTransformCache_.clear(); for (const auto& pair : this->createdInstances_) { const RenderAssetInstanceKey key = pair.first; latestTransformCache_[key] = From c8cb254ce438f4949ee8db6248a7d0dce112031f Mon Sep 17 00:00:00 2001 From: Mikael Dallaire Cote <110583667+0mdc@users.noreply.github.com> Date: Mon, 26 Jun 2023 22:34:47 -0400 Subject: [PATCH 7/9] Move BatchPlayerImplementation into its own file. --- src/esp/sim/BatchPlayerImplementation.cpp | 169 ++++++++++++++++++++++ src/esp/sim/BatchPlayerImplementation.h | 51 +++++++ src/esp/sim/BatchReplayRenderer.cpp | 166 +-------------------- src/esp/sim/CMakeLists.txt | 2 + 4 files changed, 223 insertions(+), 165 deletions(-) create mode 100644 src/esp/sim/BatchPlayerImplementation.cpp create mode 100644 src/esp/sim/BatchPlayerImplementation.h diff --git a/src/esp/sim/BatchPlayerImplementation.cpp b/src/esp/sim/BatchPlayerImplementation.cpp new file mode 100644 index 0000000000..76a2b8f5ef --- /dev/null +++ b/src/esp/sim/BatchPlayerImplementation.cpp @@ -0,0 +1,169 @@ +// Copyright (c) Meta Platforms, Inc. and its affiliates. +// This source code is licensed under the MIT license found in the +// LICENSE file in the root directory of this source tree. + +#include "BatchPlayerImplementation.h" + +#include + +#include + +namespace { +bool isSupportedRenderAsset(const Corrade::Containers::StringView& filepath) { + // Primitives aren't directly supported in the Magnum batch renderer. See + // https://docs.google.com/document/d/1ngA73cXl3YRaPfFyICSUHONZN44C-XvieS7kwyQDbkI/edit#bookmark=id.yq39718gqbwz + + const std::array primNamePrefixes = { + "capsule3DSolid", "capsule3DWireframe", "coneSolid", + "coneWireframe", "cubeSolid", "cubeWireframe", + "cylinderSolid", "cylinderWireframe", "icosphereSolid", + "icosphereWireframe", "uvSphereSolid", "uvSphereWireframe"}; + + // primitive render asset filepaths start with one of the above prefixes. + // Examples: icosphereSolid_subdivs_1 + // capsule3DSolid_hemiRings_4_cylRings_1_segments_12_halfLen_3.25_useTexCoords_false_useTangents_false + for (const auto& primNamePrefix : primNamePrefixes) { + if (filepath.size() < primNamePrefix.size()) { + continue; + } + + if (filepath.prefix(primNamePrefix.size()) == primNamePrefix) { + return false; + } + } + + return true; +} +} // namespace + +namespace esp { +namespace sim { + +BatchPlayerImplementation::BatchPlayerImplementation( + gfx_batch::Renderer& renderer, + Mn::UnsignedInt sceneId) + : renderer_{renderer}, sceneId_{sceneId} {} + +gfx::replay::NodeHandle +BatchPlayerImplementation::loadAndCreateRenderAssetInstance( + const esp::assets::AssetInfo& assetInfo, + const esp::assets::RenderAssetInstanceCreationInfo& creation) { + // TODO anything to use creation.flags for? + // TODO is creation.lightSetupKey actually mapping to anything in the + // replay file? + + if (!::isSupportedRenderAsset(creation.filepath)) { + ESP_WARNING() << "Unsupported render asset: " << creation.filepath; + return nullptr; + } + + /* If no such name is known yet, add as a file */ + if (!renderer_.hasNodeHierarchy(creation.filepath)) { + ESP_WARNING() + << creation.filepath + << "not found in any composite file, loading from the filesystem"; + + ESP_CHECK( + renderer_.addFile(creation.filepath, + gfx_batch::RendererFileFlag::Whole | + gfx_batch::RendererFileFlag::GenerateMipmap), + "addFile failed for " << creation.filepath); + CORRADE_INTERNAL_ASSERT(renderer_.hasNodeHierarchy(creation.filepath)); + } + + return reinterpret_cast( + renderer_.addNodeHierarchy( + sceneId_, creation.filepath, + /* Baking the initial scaling and coordinate frame into the + transformation */ + Mn::Matrix4::scaling(creation.scale ? *creation.scale + : Mn::Vector3{1.0f}) * + Mn::Matrix4::from( + Mn::Quaternion{assetInfo.frame.rotationFrameToWorld()} + .toMatrix(), + {})) + /* Returning incremented by 1 because 0 (nullptr) is treated as an + error */ + + 1); +} + +void BatchPlayerImplementation::deleteAssetInstance( + const gfx::replay::NodeHandle node) { + // TODO actually remove from the scene instead of setting a zero scale + renderer_.transformations(sceneId_)[reinterpret_cast(node) - 1] = + Mn::Matrix4{Mn::Math::ZeroInit}; +} + +void BatchPlayerImplementation::deleteAssetInstances( + const std::unordered_map&) { + renderer_.clear(sceneId_); +} + +void BatchPlayerImplementation::setNodeTransform( + const gfx::replay::NodeHandle node, + const Mn::Vector3& translation, + const Mn::Quaternion& rotation) { + renderer_.transformations(sceneId_)[reinterpret_cast(node) - 1] = + Mn::Matrix4::from(rotation.toMatrix(), translation); +} + +void BatchPlayerImplementation::setNodeTransform( + const gfx::replay::NodeHandle node, + const Mn::Matrix4& transform) { + renderer_.transformations(sceneId_)[reinterpret_cast(node) - 1] = + transform; +} + +Mn::Matrix4 BatchPlayerImplementation::hackGetNodeTransform( + const gfx::replay::NodeHandle node) const { + return renderer_.transformations( + sceneId_)[reinterpret_cast(node) - 1]; +} + +void BatchPlayerImplementation::changeLightSetup( + const esp::gfx::LightSetup& lights) { + if (!renderer_.maxLightCount()) { + ESP_WARNING() << "Attempted to change" << lights.size() + << "lights for scene" << sceneId_ + << "but the renderer is configured without lights"; + return; + } + + renderer_.clearLights(sceneId_); + for (std::size_t i = 0; i != lights.size(); ++i) { + const gfx::LightInfo& light = lights[i]; + CORRADE_INTERNAL_ASSERT(light.model == gfx::LightPositionModel::Global); + + const std::size_t nodeId = renderer_.addEmptyNode(sceneId_); + + std::size_t lightId; // NOLINT + if (light.vector.w()) { + renderer_.transformations(sceneId_)[nodeId] = + Mn::Matrix4::translation(light.vector.xyz()); + lightId = renderer_.addLight(sceneId_, nodeId, + gfx_batch::RendererLightType::Point); + } else { + /* The matrix will be partially NaNs if the light vector is in the + direction of the Y axis, but that's fine -- we only really care + about the Z axis direction, which is always the "target" vector + normalized. */ + // TODO for more robustness use something that "invents" some + // arbitrary orthogonal axes instead of the NaNs, once Magnum has + // such utility + renderer_.transformations(sceneId_)[nodeId] = + Mn::Matrix4::lookAt({}, light.vector.xyz(), Mn::Vector3::yAxis()); + lightId = renderer_.addLight(sceneId_, nodeId, + gfx_batch::RendererLightType::Directional); + } + + renderer_.lightColors(sceneId_)[lightId] = light.color; + // TODO use gfx::getAmbientLightColor(lights) once it's not hardcoded + // to an arbitrary value and once it's possible to change the ambient + // factor in the renderer at runtime (and not just in + // RendererConfiguration::setAmbientFactor()) + // TODO range, once Habitat has that + } +} +} // namespace sim +} // namespace esp diff --git a/src/esp/sim/BatchPlayerImplementation.h b/src/esp/sim/BatchPlayerImplementation.h new file mode 100644 index 0000000000..38cd153477 --- /dev/null +++ b/src/esp/sim/BatchPlayerImplementation.h @@ -0,0 +1,51 @@ +// Copyright (c) Meta Platforms, Inc. and its affiliates. +// This source code is licensed under the MIT license found in the +// LICENSE file in the root directory of this source tree. + +#ifndef ESP_SIM_BATCHPLAYERIMPLEMENTATION_H_ +#define ESP_SIM_BATCHPLAYERIMPLEMENTATION_H_ + +#include + +namespace esp { +namespace gfx_batch { +class Renderer; +} +namespace sim { + +class BatchPlayerImplementation + : public gfx::replay::AbstractPlayerImplementation { + public: + BatchPlayerImplementation(gfx_batch::Renderer& renderer, + Mn::UnsignedInt sceneId); + + private: + gfx::replay::NodeHandle loadAndCreateRenderAssetInstance( + const esp::assets::AssetInfo& assetInfo, + const esp::assets::RenderAssetInstanceCreationInfo& creation) override; + + void deleteAssetInstance(const gfx::replay::NodeHandle node) override; + + void deleteAssetInstances( + const std::unordered_map&) override; + + void setNodeTransform(const gfx::replay::NodeHandle node, + const Mn::Vector3& translation, + const Mn::Quaternion& rotation) override; + + void setNodeTransform(const gfx::replay::NodeHandle node, + const Mn::Matrix4& transform) override; + + Mn::Matrix4 hackGetNodeTransform( + const gfx::replay::NodeHandle node) const override; + + void changeLightSetup(const esp::gfx::LightSetup& lights) override; + + gfx_batch::Renderer& renderer_; + Mn::UnsignedInt sceneId_; +}; +} // namespace sim +} // namespace esp + +#endif diff --git a/src/esp/sim/BatchReplayRenderer.cpp b/src/esp/sim/BatchReplayRenderer.cpp index b4d862a68c..fc78b22d94 100644 --- a/src/esp/sim/BatchReplayRenderer.cpp +++ b/src/esp/sim/BatchReplayRenderer.cpp @@ -5,6 +5,7 @@ #include "BatchReplayRenderer.h" #include +#include #include "esp/sensor/CameraSensor.h" #include @@ -15,7 +16,6 @@ namespace esp { namespace sim { -// clang-tidy you're NOT HELPING using namespace Mn::Math::Literals; // NOLINT BatchReplayRenderer::BatchReplayRenderer( @@ -47,170 +47,6 @@ BatchReplayRenderer::BatchReplayRenderer( theOnlySensorName_ = sensor.uuid; theOnlySensorProjection_ = sensor.projectionMatrix(); - class BatchPlayerImplementation - : public gfx::replay::AbstractPlayerImplementation { - public: - BatchPlayerImplementation(gfx_batch::Renderer& renderer, - Mn::UnsignedInt sceneId) - : renderer_{renderer}, sceneId_{sceneId} {} - - private: - bool isSupportedRenderAsset( - const Corrade::Containers::StringView& filepath) { - // Primitives aren't directly supported in the Magnum batch renderer. See - // https://docs.google.com/document/d/1ngA73cXl3YRaPfFyICSUHONZN44C-XvieS7kwyQDbkI/edit#bookmark=id.yq39718gqbwz - - const std::array primNamePrefixes = { - "capsule3DSolid", "capsule3DWireframe", "coneSolid", - "coneWireframe", "cubeSolid", "cubeWireframe", - "cylinderSolid", "cylinderWireframe", "icosphereSolid", - "icosphereWireframe", "uvSphereSolid", "uvSphereWireframe"}; - - // primitive render asset filepaths start with one of the above prefixes. - // Examples: icosphereSolid_subdivs_1 - // capsule3DSolid_hemiRings_4_cylRings_1_segments_12_halfLen_3.25_useTexCoords_false_useTangents_false - for (const auto& primNamePrefix : primNamePrefixes) { - if (filepath.size() < primNamePrefix.size()) { - continue; - } - - if (filepath.prefix(primNamePrefix.size()) == primNamePrefix) { - return false; - } - } - - return true; - } - - gfx::replay::NodeHandle loadAndCreateRenderAssetInstance( - const esp::assets::AssetInfo& assetInfo, - const esp::assets::RenderAssetInstanceCreationInfo& creation) override { - // TODO anything to use creation.flags for? - // TODO is creation.lightSetupKey actually mapping to anything in the - // replay file? - - if (!isSupportedRenderAsset(creation.filepath)) { - ESP_WARNING() << "Unsupported render asset: " << creation.filepath; - return nullptr; - } - - /* If no such name is known yet, add as a file */ - if (!renderer_.hasNodeHierarchy(creation.filepath)) { - ESP_WARNING() - << creation.filepath - << "not found in any composite file, loading from the filesystem"; - - ESP_CHECK( - renderer_.addFile(creation.filepath, - gfx_batch::RendererFileFlag::Whole | - gfx_batch::RendererFileFlag::GenerateMipmap), - "addFile failed for " << creation.filepath); - CORRADE_INTERNAL_ASSERT(renderer_.hasNodeHierarchy(creation.filepath)); - } - - return reinterpret_cast( - renderer_.addNodeHierarchy( - sceneId_, creation.filepath, - /* Baking the initial scaling and coordinate frame into the - transformation */ - Mn::Matrix4::scaling(creation.scale ? *creation.scale - : Mn::Vector3{1.0f}) * - Mn::Matrix4::from( - Mn::Quaternion{assetInfo.frame.rotationFrameToWorld()} - .toMatrix(), - {})) - /* Returning incremented by 1 because 0 (nullptr) is treated as an - error */ - + 1); - } - - void deleteAssetInstance(const gfx::replay::NodeHandle node) override { - // TODO actually remove from the scene instead of setting a zero scale - renderer_.transformations( - sceneId_)[reinterpret_cast(node) - 1] = - Mn::Matrix4{Mn::Math::ZeroInit}; - } - - void deleteAssetInstances( - const std::unordered_map&) override { - renderer_.clear(sceneId_); - } - - void setNodeTransform(const gfx::replay::NodeHandle node, - const Mn::Vector3& translation, - const Mn::Quaternion& rotation) override { - renderer_.transformations( - sceneId_)[reinterpret_cast(node) - 1] = - Mn::Matrix4::from(rotation.toMatrix(), translation); - } - - void setNodeTransform(const gfx::replay::NodeHandle node, - const Mn::Matrix4& transform) override { - renderer_.transformations( - sceneId_)[reinterpret_cast(node) - 1] = transform; - } - - Mn::Matrix4 hackGetNodeTransform( - const gfx::replay::NodeHandle node) const override { - return renderer_.transformations( - sceneId_)[reinterpret_cast(node) - 1]; - } - - void changeLightSetup(const esp::gfx::LightSetup& lights) override { - if (!renderer_.maxLightCount()) { - ESP_WARNING() << "Attempted to change" << lights.size() - << "lights for scene" << sceneId_ - << "but the renderer is configured without lights"; - return; - } - - renderer_.clearLights(sceneId_); - for (std::size_t i = 0; i != lights.size(); ++i) { - const gfx::LightInfo& light = lights[i]; - CORRADE_INTERNAL_ASSERT(light.model == gfx::LightPositionModel::Global); - - const std::size_t nodeId = renderer_.addEmptyNode(sceneId_); - - /* Clang Tidy, you're stupid, why do you say that "lightId" is not - initialized?! I'm initializing it right in the branches below, I - won't zero-init it just to "prevent bugs" because an accidentally - zero-initialized variable is *also* a bug, you know? Plus I have - range asserts in all functions so your "suggestions" are completely - unhelpful. */ - std::size_t lightId; // NOLINT - if (light.vector.w()) { - renderer_.transformations(sceneId_)[nodeId] = - Mn::Matrix4::translation(light.vector.xyz()); - lightId = renderer_.addLight(sceneId_, nodeId, - gfx_batch::RendererLightType::Point); - } else { - /* The matrix will be partially NaNs if the light vector is in the - direction of the Y axis, but that's fine -- we only really care - about the Z axis direction, which is always the "target" vector - normalized. */ - // TODO for more robustness use something that "invents" some - // arbitrary orthogonal axes instead of the NaNs, once Magnum has - // such utility - renderer_.transformations(sceneId_)[nodeId] = - Mn::Matrix4::lookAt({}, light.vector.xyz(), Mn::Vector3::yAxis()); - lightId = renderer_.addLight( - sceneId_, nodeId, gfx_batch::RendererLightType::Directional); - } - - renderer_.lightColors(sceneId_)[lightId] = light.color; - // TODO use gfx::getAmbientLightColor(lights) once it's not hardcoded - // to an arbitrary value and once it's possible to change the ambient - // factor in the renderer at runtime (and not just in - // RendererConfiguration::setAmbientFactor()) - // TODO range, once Habitat has that - } - } - - gfx_batch::Renderer& renderer_; - Mn::UnsignedInt sceneId_; - }; - for (Mn::UnsignedInt i = 0; i != cfg.numEnvironments; ++i) { arrayAppend( envs_, EnvironmentRecord{ diff --git a/src/esp/sim/CMakeLists.txt b/src/esp/sim/CMakeLists.txt index 91fa02c098..7f7c22afed 100644 --- a/src/esp/sim/CMakeLists.txt +++ b/src/esp/sim/CMakeLists.txt @@ -6,6 +6,8 @@ add_library( sim STATIC AbstractReplayRenderer.cpp AbstractReplayRenderer.h + BatchPlayerImplementation.cpp + BatchPlayerImplementation.h BatchReplayRenderer.cpp BatchReplayRenderer.h ClassicReplayRenderer.cpp From e0bac2f3ba8cba0cce67abc0a81bea7ad9b0127c Mon Sep 17 00:00:00 2001 From: Mikael Dallaire Cote <110583667+0mdc@users.noreply.github.com> Date: Mon, 26 Jun 2023 22:35:10 -0400 Subject: [PATCH 8/9] Add BatchPlayerImplementation node deletion test. --- src/tests/BatchReplayRendererTest.cpp | 102 +++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 4 deletions(-) diff --git a/src/tests/BatchReplayRendererTest.cpp b/src/tests/BatchReplayRendererTest.cpp index c035cafe3e..f3d850fab3 100644 --- a/src/tests/BatchReplayRendererTest.cpp +++ b/src/tests/BatchReplayRendererTest.cpp @@ -3,11 +3,8 @@ // LICENSE file in the root directory of this source tree. #include "Corrade/Containers/EnumSet.h" -#include "Corrade/Utility/Assert.h" -#include "Magnum/DebugTools/Screenshot.h" +#include "Corrade/Containers/StridedArrayView.h" #include "Magnum/GL/Context.h" -#include "Magnum/Magnum.h" -#include "Magnum/Trade/AbstractImageConverter.h" #include "configure.h" #include "esp/gfx/replay/Recorder.h" @@ -21,6 +18,9 @@ #include "esp/sim/ClassicReplayRenderer.h" #include "esp/sim/Simulator.h" +#include +#include + #include #include #include @@ -49,6 +49,7 @@ struct BatchReplayRendererTest : Cr::TestSuite::Tester { void testIntegration(); void testUnproject(); + void testBatchPlayerDeletion(); const Magnum::Float maxThreshold = 255.f; const Magnum::Float meanThreshold = 0.75f; @@ -161,6 +162,8 @@ BatchReplayRendererTest::BatchReplayRendererTest() { addInstancedTests({&BatchReplayRendererTest::testIntegration}, Cr::Containers::arraySize(TestIntegrationData)); + + addTests({&BatchReplayRendererTest::testBatchPlayerDeletion}); } // ctor // test recording and playback through the simulator interface @@ -355,6 +358,97 @@ void BatchReplayRendererTest::testIntegration() { CORRADE_VERIFY(!Mn::GL::Context::hasCurrent()); } +// test batch replay renderer node deletion +void BatchReplayRendererTest::testBatchPlayerDeletion() { + const std::string assetPath = + Cr::Utility::Path::join(TEST_ASSETS, "objects/sphere.glb"); + const auto assetInfo = esp::assets::AssetInfo::fromPath(assetPath); + + esp::assets::RenderAssetInstanceCreationInfo::Flags flags; + flags |= esp::assets::RenderAssetInstanceCreationInfo::Flag::IsRGBD; + flags |= esp::assets::RenderAssetInstanceCreationInfo::Flag::IsSemantic; + auto creationInfo = esp::assets::RenderAssetInstanceCreationInfo(); + creationInfo.filepath = assetPath; + creationInfo.flags = flags; + + std::vector keyframes; + + // Record sequence + { + SimulatorConfiguration simConfig{}; + simConfig.enableGfxReplaySave = true; + simConfig.createRenderer = false; + auto sim = Simulator::create_unique(simConfig); + auto& sceneRoot = sim->getActiveSceneGraph().getRootNode(); + auto& recorder = *sim->getGfxReplayManager()->getRecorder(); + + { + // Frame 0 + auto* sphereX = + sim->loadAndCreateRenderAssetInstance(assetInfo, creationInfo); + auto* sphereY = + sim->loadAndCreateRenderAssetInstance(assetInfo, creationInfo); + CORRADE_VERIFY(sphereX); + CORRADE_VERIFY(sphereY); + sphereX->setTranslation(Mn::Vector3(1.0f, 0.0f, 0.0f)); + sphereY->setTranslation(Mn::Vector3(0.0f, 1.0f, 0.0f)); + keyframes.emplace_back(std::move(recorder.extractKeyframe())); + + // Frame 1 + delete sphereX; + auto sphereZ = + sim->loadAndCreateRenderAssetInstance(assetInfo, creationInfo); + CORRADE_VERIFY(sphereZ); + sphereZ->setTranslation(Mn::Vector3(0.0f, 0.0f, 1.0f)); + delete sphereZ; + keyframes.emplace_back(std::move(recorder.extractKeyframe())); + + // Frame 2 + delete sphereY; + keyframes.emplace_back(std::move(recorder.extractKeyframe())); + } + } + CORRADE_COMPARE(keyframes.size(), 3); + + // Play sequence + { + // clang-format off + esp::gfx_batch::RendererStandalone renderer{ + esp::gfx_batch::RendererConfiguration{} + .setTileSizeCount({16, 16}, {1, 1}), + esp::gfx_batch::RendererStandaloneConfiguration{} + .setFlags(esp::gfx_batch::RendererStandaloneFlag::QuietLog) + }; + // clang-format on + auto batchPlayer = + std::make_shared(renderer, 0); + esp::gfx::replay::Player player(batchPlayer); + player.debugSetKeyframes(std::move(keyframes)); + CORRADE_COMPARE(player.getNumKeyframes(), 3); + constexpr size_t transformsPerInstance = 2; + + // Frame 0 + player.setKeyframeIndex(0); + CORRADE_COMPARE(renderer.transformations(0).size(), + 2 * transformsPerInstance); + CORRADE_COMPARE(renderer.transformations(0)[0], + Mn::Matrix4::translation(Mn::Vector3(1.0f, 0.0f, 0.0f))); + CORRADE_COMPARE(renderer.transformations(0)[1 * transformsPerInstance], + Mn::Matrix4::translation(Mn::Vector3(0.0f, 1.0f, 0.0f))); + + // Frame 2 + player.setKeyframeIndex(1); + CORRADE_COMPARE(renderer.transformations(0).size(), + 1 * transformsPerInstance); + CORRADE_COMPARE(renderer.transformations(0)[0], + Mn::Matrix4::translation(Mn::Vector3(0.0f, 1.0f, 0.0f))); + + // Frame 2 + player.setKeyframeIndex(2); + CORRADE_COMPARE(renderer.transformations(0).size(), 0); + } +} + } // namespace CORRADE_TEST_MAIN(BatchReplayRendererTest) From 8ef2b02a73f5456f248a38ba9a145209c40173cb Mon Sep 17 00:00:00 2001 From: Mikael Dallaire Cote <110583667+0mdc@users.noreply.github.com> Date: Mon, 26 Jun 2023 23:14:16 -0400 Subject: [PATCH 9/9] Clang-tidy fixes. --- src/esp/sim/BatchPlayerImplementation.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/esp/sim/BatchPlayerImplementation.h b/src/esp/sim/BatchPlayerImplementation.h index 38cd153477..72bcdd07c7 100644 --- a/src/esp/sim/BatchPlayerImplementation.h +++ b/src/esp/sim/BatchPlayerImplementation.h @@ -24,21 +24,20 @@ class BatchPlayerImplementation const esp::assets::AssetInfo& assetInfo, const esp::assets::RenderAssetInstanceCreationInfo& creation) override; - void deleteAssetInstance(const gfx::replay::NodeHandle node) override; + void deleteAssetInstance(gfx::replay::NodeHandle node) override; void deleteAssetInstances( const std::unordered_map&) override; - void setNodeTransform(const gfx::replay::NodeHandle node, + void setNodeTransform(gfx::replay::NodeHandle node, const Mn::Vector3& translation, const Mn::Quaternion& rotation) override; - void setNodeTransform(const gfx::replay::NodeHandle node, + void setNodeTransform(gfx::replay::NodeHandle node, const Mn::Matrix4& transform) override; - Mn::Matrix4 hackGetNodeTransform( - const gfx::replay::NodeHandle node) const override; + Mn::Matrix4 hackGetNodeTransform(gfx::replay::NodeHandle node) const override; void changeLightSetup(const esp::gfx::LightSetup& lights) override;