Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tag entity exclusion #59

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Code/Include/RGL/RGLBus.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ namespace RGL
//! Updates scene to the RGL
virtual void UpdateScene() = 0;

//! Forces a reevaluation of entity presence conditions.
//! Results of this condition reevaluation influence entity's inclusion in raycasting.
virtual void ReviseEntityPresence(AZ::EntityId entityId) = 0;

protected:
~RGLRequests() = default;
};
Expand Down
3 changes: 3 additions & 0 deletions Code/Include/RGL/SceneConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

#include <Atom/RPI.Reflect/Image/StreamingImageAsset.h>
#include <AzCore/Asset/AssetCommon.h>
#include <AzCore/std/containers/vector.h>
#include <AzCore/std/string/string.h>

namespace RGL
{
Expand All @@ -37,6 +39,7 @@ namespace RGL
static void Reflect(AZ::ReflectContext* context);

TerrainIntensityConfiguration m_terrainIntensityConfig;
AZStd::vector<AZStd::string> m_excludedTagNames;
// clang-format off
bool m_isSkinnedMeshUpdateEnabled{ true }; //!< If set to true, all skinned meshes will be updated. Otherwise they will remain unchanged.
// clang-format on
Expand Down
58 changes: 58 additions & 0 deletions Code/Source/Entity/EntityTagListener.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/* Copyright 2024, Robotec.ai sp. z o.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <Entity/EntityTagListener.h>
#include <RGL/RGLBus.h>

namespace RGL
{
EntityTagListener::EntityTagListener(AZ::EntityId entityId)
: m_entityId(entityId)
{
AZ::EntityBus::Handler::BusConnect(m_entityId);
}

EntityTagListener::~EntityTagListener()
{
AZ::EntityBus::Handler::BusDisconnect(m_entityId);
}

void EntityTagListener::OnEntityActivated([[maybe_unused]] const AZ::EntityId& entityId)
{
LmbrCentral::Tags entityTags;
LmbrCentral::TagComponentRequestBus::EventResult(entityTags, m_entityId, &LmbrCentral::TagComponentRequests::GetTags);

if (!entityTags.empty())
{
RGLInterface::Get()->ReviseEntityPresence(m_entityId);
}

LmbrCentral::TagComponentNotificationsBus::Handler::BusConnect(m_entityId);
}

void EntityTagListener::OnEntityDeactivated(const AZ::EntityId& entity_id)
{
LmbrCentral::TagComponentNotificationsBus::Handler::BusDisconnect(m_entityId);
}

void EntityTagListener::OnTagAdded(const LmbrCentral::Tag& crc32)
{
RGLInterface::Get()->ReviseEntityPresence(m_entityId);
}

void EntityTagListener::OnTagRemoved(const LmbrCentral::Tag& crc32)
{
RGLInterface::Get()->ReviseEntityPresence(m_entityId);
}
} // namespace RGL
40 changes: 40 additions & 0 deletions Code/Source/Entity/EntityTagListener.h
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating the new class and having separate container for it in RGLSystemComponent I'd extend EntityManager with LmbrCentral::TagComponentNotificationsBus::Handler. I see no advantages in having this separation.

Copy link
Collaborator Author

@alek-kam-robotec-ai alek-kam-robotec-ai Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to allow for a scenario where an entity is excluded from the raycasting due it being tagged with an excluded tag after which the tag is removed and the entity must be processed. With the bus handled by the entity manager we would not be able to achieve this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if dynamic handling of tag creation/destruction is necessary (if there are any real use cases). If not, this solution could be significantly simplified to checking ShouldEntityBeExcluded in OnEntityContextCreateEntity and discarding entities that should be excluded (skipping processing them and putting them to m_unprocessedEntities). Did you get the detailed requirements for this feature?

Copy link
Collaborator Author

@alek-kam-robotec-ai alek-kam-robotec-ai Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Tag Component allows for tag addition and removal at runtime. I believe we should cover these cases as some proved useful before (as seen in this pr).

Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/* Copyright 2024, Robotec.ai sp. z o.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#pragma once

#include <AzCore/Component/EntityBus.h>
#include <LmbrCentral/Scripting/TagComponentBus.h>

namespace RGL
{
class EntityTagListener
: AZ::EntityBus::Handler
, LmbrCentral::TagComponentNotificationsBus::Handler
{
public:
EntityTagListener(AZ::EntityId entityId);
~EntityTagListener();

void OnEntityActivated(const AZ::EntityId&) override;
void OnEntityDeactivated(const AZ::EntityId&) override;

// TagComponentNotificationsBus overrides
void OnTagAdded(const LmbrCentral::Tag&) override;
void OnTagRemoved(const LmbrCentral::Tag&) override;

private:
AZ::EntityId m_entityId;
};
} // namespace RGL
117 changes: 115 additions & 2 deletions Code/Source/RGLSystemComponent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,15 @@ namespace RGL

AzFramework::EntityContextEventBus::Handler::BusConnect(gameEntityContextId);
LidarSystemNotificationBus::Handler::BusConnect();
AZ::TickBus::Handler::BusConnect();

m_rglLidarSystem.Activate();
}

void RGLSystemComponent::Deactivate()
{
m_rglLidarSystem.Deactivate();
AZ::TickBus::Handler::BusDisconnect();
LidarSystemNotificationBus::Handler::BusDisconnect();
AzFramework::EntityContextEventBus::Handler::BusDisconnect();

Expand All @@ -113,6 +115,18 @@ namespace RGL
void RGLSystemComponent::SetSceneConfiguration(const SceneConfiguration& config)
{
m_sceneConfig = config;

m_excludedTags.resize(config.m_excludedTagNames.size());
AZStd::transform(
config.m_excludedTagNames.begin(),
config.m_excludedTagNames.end(),
m_excludedTags.begin(),
[](const AZStd::string& tagName) -> LmbrCentral::Tag
{
return LmbrCentral::Tag(tagName);
});
UpdateTagExcludedEntities();

RGLNotificationBus::Broadcast(&RGLNotifications::OnSceneConfigurationSet, config);
}

Expand All @@ -121,14 +135,37 @@ namespace RGL
return m_sceneConfig;
}

static bool HasExcludedTag(AZ::EntityId entityId, const AZStd::vector<LmbrCentral::Tag>& excludedTags)
{
LmbrCentral::Tags entityTags;
LmbrCentral::TagComponentRequestBus::EventResult(entityTags, entityId, &LmbrCentral::TagComponentRequests::GetTags);

if (entityTags.empty())
{
return false;
}

for (const auto tag : excludedTags)
{
if (entityTags.contains(tag))
{
return true;
}
}

return false;
alek-kam-robotec-ai marked this conversation as resolved.
Show resolved Hide resolved
}

void RGLSystemComponent::OnEntityContextCreateEntity(AZ::Entity& entity)
{
if (m_excludedEntities.contains(entity.GetId()))
if (!HasVisuals(entity))
{
return;
}

if (m_activeLidarCount < 1U)
m_entityTagListeners.emplace_back(entity.GetId());
alek-kam-robotec-ai marked this conversation as resolved.
Show resolved Hide resolved

if (m_activeLidarCount < 1U || ShouldEntityBeExcluded(entity.GetId()))
{
m_unprocessedEntities.emplace(entity.GetId());
return;
Expand Down Expand Up @@ -163,6 +200,12 @@ namespace RGL
RGLNotificationBus::Broadcast(&RGLNotifications::OnAnyLidarExists);
for (auto entityIdIt = m_unprocessedEntities.begin(); entityIdIt != m_unprocessedEntities.end();)
{
if (ShouldEntityBeExcluded(*entityIdIt))
{
++entityIdIt;
continue;
}

AZ::Entity* entity = nullptr;
AZ::ComponentApplicationBus::BroadcastResult(entity, &AZ::ComponentApplicationRequests::FindEntity, *entityIdIt);
AZ_Assert(entity, "Failed to find entity with provided id!");
Expand All @@ -189,6 +232,26 @@ namespace RGL
m_modelLibrary.Clear();
}

void RGLSystemComponent::OnTick(float deltaTime, AZ::ScriptTimePoint time)
{
for (auto entityId : m_managersToBeRemoved)
{
m_entityManagers.erase(entityId);
m_unprocessedEntities.insert(entityId);
}
m_managersToBeRemoved.clear();
}
Comment on lines +234 to +242
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why couldn't we erase entity manager in ReviseEntityPresence(AZ::EntityId entityId)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so that the caller can signal revision inside a bus notification and can then safely disconnect from said bus after the call has ended (on destruction).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, the only caller is EntityTagListener which is not destroyed in ReviseEntityPresence() so your case doesn't occur. Is it solution for the future usages?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. I believe that this could lead to bugs that would be hard to track down as in previous iterations of the solution the entity manager handlet the tag notification bus and the ReviseEntityPresence method was called from within the callbacks which lead to segfaults caused by destruction of the entity manager before the callback was finished. In my opinion a small amount of additional complexity in this case is worth it.


bool RGLSystemComponent::HasVisuals(const AZ::Entity& entity)
{
return entity.FindComponent<EMotionFX::Integration::ActorComponent>() || entity.FindComponent(AZ::Render::MeshComponentTypeId);
}

bool RGLSystemComponent::ShouldEntityBeExcluded(AZ::EntityId entityId) const
{
return m_excludedEntities.contains(entityId) || HasExcludedTag(entityId, m_excludedTags);
}

void RGLSystemComponent::ProcessEntity(const AZ::Entity& entity)
{
AZStd::unique_ptr<EntityManager> entityManager;
Expand All @@ -209,6 +272,27 @@ namespace RGL
AZ_Error(__func__, inserted, "Object with provided entityId already exists.");
}

void RGLSystemComponent::UpdateTagExcludedEntities()
{
if (m_excludedTags.empty())
{
return;
}

for (auto entityManagerIt = m_entityManagers.begin(); entityManagerIt != m_entityManagers.end();)
{
if (HasExcludedTag(entityManagerIt->first, m_excludedTags))
{
m_unprocessedEntities.insert(entityManagerIt->first);
entityManagerIt = m_entityManagers.erase(entityManagerIt);
}
else
{
++entityManagerIt;
}
}
}

void RGLSystemComponent::UpdateScene()
{
AZ::ScriptTimePoint currentTime;
Expand All @@ -225,4 +309,33 @@ namespace RGL
entityManager->Update();
}
}

void RGLSystemComponent::ReviseEntityPresence(AZ::EntityId entityId)
{
if (m_activeLidarCount < 1U)
{
return; // No lidars exist. Every entity should stay as unprocessed until they do.
}

if (m_excludedEntities.contains(entityId))
{
return; // Already not included.
}

if (HasExcludedTag(entityId, m_excludedTags))
{
if (m_entityManagers.contains(entityId))
{
m_managersToBeRemoved.push_back(entityId);
}
}
else if (const auto it = m_unprocessedEntities.find(entityId); it != m_unprocessedEntities.end())
{
m_unprocessedEntities.erase(it);
AZ::Entity* entity = nullptr;
AZ::ComponentApplicationBus::BroadcastResult(entity, &AZ::ComponentApplicationRequests::FindEntity, entityId);
AZ_Assert(entity, "Failed to find entity with provided id!");
ProcessEntity(*entity);
}
}
} // namespace RGL
21 changes: 18 additions & 3 deletions Code/Source/RGLSystemComponent.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@
#pragma once

#include <AzCore/Component/Component.h>
#include <AzCore/Component/TickBus.h>
#include <AzCore/Math/Vector3.h>
#include <AzCore/Script/ScriptTimePoint.h>
#include <AzFramework/Entity/EntityContextBus.h>
#include <Entity/EntityTagListener.h>
#include <Lidar/LidarSystem.h>
#include <Lidar/LidarSystemNotificationBus.h>
#include <LmbrCentral/Scripting/TagComponentBus.h>
#include <Model/ModelLibrary.h>
#include <RGL/RGLBus.h>

Expand All @@ -32,6 +35,7 @@ namespace RGL
, protected RGLRequestBus::Handler
, protected AzFramework::EntityContextEventBus::Handler
, protected LidarSystemNotificationBus::Handler
, protected AZ::TickBus::Handler
{
public:
AZ_COMPONENT(RGL::RGLSystemComponent, "{dbd5b1c5-249f-4eca-a142-2533ebe7f680}");
Expand All @@ -56,6 +60,7 @@ namespace RGL
void SetSceneConfiguration(const SceneConfiguration& config) override;
[[nodiscard]] const SceneConfiguration& GetSceneConfiguration() const override;
void UpdateScene() override;
void ReviseEntityPresence(AZ::EntityId entityId) override;

// AzFramework::EntityContextEventBus overrides
void OnEntityContextCreateEntity(AZ::Entity& entity) override;
Expand All @@ -66,18 +71,28 @@ namespace RGL
void OnLidarCreated() override;
void OnLidarDestroyed() override;

// AZ::TickBus overrides
void OnTick(float deltaTime, AZ::ScriptTimePoint time) override;

private:
static bool HasVisuals(const AZ::Entity& entity);
bool ShouldEntityBeExcluded(AZ::EntityId entityId) const;
void ProcessEntity(const AZ::Entity& entity);
void UpdateTagExcludedEntities();

SceneConfiguration m_sceneConfig;
LidarSystem m_rglLidarSystem;

ModelLibrary m_modelLibrary;

AZStd::set<AZ::EntityId> m_excludedEntities;
AZStd::set<AZ::EntityId> m_unprocessedEntities;
SceneConfiguration m_sceneConfig;
AZStd::vector<AZ::EntityId> m_managersToBeRemoved;
AZStd::unordered_map<AZ::EntityId, AZStd::unique_ptr<EntityManager>> m_entityManagers;
AZ::ScriptTimePoint m_sceneUpdateLastTime{};
AZStd::vector<EntityTagListener> m_entityTagListeners;

AZStd::vector<LmbrCentral::Tag> m_excludedTags;

AZ::ScriptTimePoint m_sceneUpdateLastTime{};
size_t m_activeLidarCount{};
};
} // namespace RGL
6 changes: 6 additions & 0 deletions Code/Source/SceneConfiguration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,19 @@ namespace RGL
serializeContext->Class<SceneConfiguration>()
->Version(0)
->Field("TerrainIntensityConfig", &SceneConfiguration::m_terrainIntensityConfig)
->Field("excludedTagNames", &SceneConfiguration::m_excludedTagNames)
->Field("SkinnedMeshUpdate", &SceneConfiguration::m_isSkinnedMeshUpdateEnabled);

if (auto* editContext = serializeContext->GetEditContext())
{
editContext->Class<SceneConfiguration>("RGL Scene Configuration", "")
->DataElement(
AZ::Edit::UIHandlers::Default, &SceneConfiguration::m_terrainIntensityConfig, "Terrain Intensity Configuration", "")
->DataElement(
AZ::Edit::UIHandlers::Default,
&SceneConfiguration::m_excludedTagNames,
"Raycast excluded Tag names",
"Entities tagged with names present in this list will not be included in raycasting.")
->DataElement(
AZ::Edit::UIHandlers::Default,
&SceneConfiguration::m_isSkinnedMeshUpdateEnabled,
Expand Down
2 changes: 2 additions & 0 deletions Code/rgl_files.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ set(FILES
Source/Entity/MeshEntityManager.h
Source/Entity/EntityManager.cpp
Source/Entity/EntityManager.h
Source/Entity/EntityTagListener.cpp
Source/Entity/EntityTagListener.h
Source/Entity/MaterialEntityManager.cpp
Source/Entity/MaterialEntityManager.h
Source/Entity/Terrain/TerrainData.cpp
Expand Down