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.

Sign up for GitHub

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

[Citadel][TPE] Cache Entity data #73

Merged
merged 1 commit into from
Jun 29, 2020
Merged

[Citadel][TPE] Cache Entity data #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions tpe/lib/src/Collision.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ math::AxisAlignedBox Collision::GetBoundingBox(bool /*_force*/) // NOLINT
void Collision::SetCollideBitmask(uint16_t _mask)
{
this->dataPtr->collideBitmask = _mask;
if (this->GetParent())
this->GetParent()->ChildrenChanged();
}

//////////////////////////////////////////////////
Expand Down
48 changes: 44 additions & 4 deletions tpe/lib/src/Entity.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,17 @@ class ignition::physics::tpelib::EntityPrivate
/// \brief Bounding Box
public: math::AxisAlignedBox bbox;

/// \brief Collide bitmask
public: uint16_t collideBitmask = 0xFF;

/// \brief Flag to indicate if bounding box changed
public: bool bboxDirty = true;

/// \brief Flag to indicate if collide bitmask changed
public: bool collideBitmaskDirty = true;

/// \brief Parent of this entity
public: Entity *parent = nullptr;
};

using namespace ignition;
Expand All @@ -63,6 +72,7 @@ Entity::Entity(const Entity &_other)
this->dataPtr->pose = _other.dataPtr->pose;
this->dataPtr->children = _other.dataPtr->children;
this->dataPtr->bbox = _other.dataPtr->bbox;
this->dataPtr->collideBitmask = _other.dataPtr->collideBitmask;
}

//////////////////////////////////////////////////
Expand Down Expand Up @@ -187,6 +197,7 @@ bool Entity::RemoveChildById(std::size_t _id)
if (it != this->dataPtr->children.end())
{
this->dataPtr->children.erase(it);
this->ChildrenChanged();
return true;
}

Expand All @@ -202,6 +213,7 @@ bool Entity::RemoveChildByName(const std::string &_name)
if (it->second->GetName() == _name)
{
this->dataPtr->children.erase(it);
this->ChildrenChanged();
return true;
}
}
Expand Down Expand Up @@ -243,12 +255,18 @@ void Entity::UpdateBoundingBox(bool _force)
//////////////////////////////////////////////////
uint16_t Entity::GetCollideBitmask() const
{
uint16_t mask = 0u;
for (auto &it : this->dataPtr->children)
if (this->dataPtr->collideBitmaskDirty)
{
mask |= it.second->GetCollideBitmask();
uint16_t mask = 0u;
for (auto &it : this->dataPtr->children)
{
mask |= it.second->GetCollideBitmask();
}
this->dataPtr->collideBitmask = mask;
this->dataPtr->collideBitmaskDirty = false;
}
return mask;

return this->dataPtr->collideBitmask;
}

//////////////////////////////////////////////////
Expand All @@ -262,3 +280,25 @@ std::size_t Entity::GetNextId()
{
return nextId++;
}

//////////////////////////////////////////////////
void Entity::ChildrenChanged()
{
this->dataPtr->bboxDirty = true;
this->dataPtr->collideBitmaskDirty= true;

if (this->dataPtr->parent)
this->dataPtr->parent->ChildrenChanged();
}

//////////////////////////////////////////////////
void Entity::SetParent(Entity *_parent)
{
this->dataPtr->parent = _parent;
}

//////////////////////////////////////////////////
Entity *Entity::GetParent() const
{
return this->dataPtr->parent;
}
15 changes: 15 additions & 0 deletions tpe/lib/src/Entity.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,21 @@ class IGNITION_PHYSICS_TPELIB_VISIBLE Entity
/// \return Collision's collide bitmask
public: virtual uint16_t GetCollideBitmask() const;

/// \internal
/// \brief Set the parent of this entity.
/// \param[in] _parent Parent to set to
public: void SetParent(Entity *_parent);

/// \internal
/// \brief Get the parent of this entity.
/// \return Parent of this entity
public: Entity *GetParent() const;

/// \internal
/// \brief Mark that the children of the entity has changed, e.g. a child
/// entity is added or removed, or child entity properties changed.
public: void ChildrenChanged();

/// \brief Get number of children
/// \return Map of child id's to child entities
protected: std::map<std::size_t, std::shared_ptr<Entity>> &GetChildren()
Expand Down
2 changes: 2 additions & 0 deletions tpe/lib/src/Link.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,7 @@ Entity &Link::AddCollision()
std::size_t collisionId = Entity::GetNextId();
const auto[it, success] = this->GetChildren().insert(
{collisionId, std::make_shared<Collision>(collisionId)});
it->second->SetParent(this);
this->ChildrenChanged();
return *it->second.get();
}
3 changes: 3 additions & 0 deletions tpe/lib/src/Model.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ Entity &Model::AddLink()
std::size_t linkId = Entity::GetNextId();
const auto[it, success] = this->GetChildren().insert(
{linkId, std::make_shared<Link>(linkId)});

it->second->SetParent(this);
this->ChildrenChanged();
return *it->second.get();
}

Expand Down
8 changes: 4 additions & 4 deletions tpe/lib/src/Model_TEST.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,11 @@ TEST(Model, BoundingBox)

math::AxisAlignedBox expectedBoxLinkFrame(
math::Vector3d(-2, -2, -2), math::Vector3d(2, 2, 2));
EXPECT_EQ(expectedBoxLinkFrame, linkEnt.GetBoundingBox(true));
EXPECT_EQ(expectedBoxLinkFrame, linkEnt.GetBoundingBox());
Copy link
Member

Choose a reason for hiding this comment

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

Oh interesting, so basically since _force was being set to true before the dirty flag was actually not being tested?
Can you add a few lines to set bitmasks for the Collision objects here and test the OR-ing / collideBitmaskDirty logic as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah that's correct, removing the force arg revealed that bounding box caching was not working.

there is already a CollideBitmask test added in pull request #69 (here) and I've been using that to test this PR. It should cover collideBitmaskDirty logic.

Copy link
Member

Choose a reason for hiding this comment

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

Ok saw it, looks great!


math::AxisAlignedBox expectedBoxModelFrame(
math::Vector3d(-2, -2, -1), math::Vector3d(2, 2, 3));
EXPECT_EQ(expectedBoxModelFrame, model.GetBoundingBox(true));
EXPECT_EQ(expectedBoxModelFrame, model.GetBoundingBox());

// add another link with box collision shape
Entity &linkEnt2 = model.AddLink();
Expand All @@ -154,11 +154,11 @@ TEST(Model, BoundingBox)

expectedBoxLinkFrame = math::AxisAlignedBox(
math::Vector3d(-1.5, -2, -2.5), math::Vector3d(1.5, 2, 2.5));
EXPECT_EQ(expectedBoxLinkFrame, linkEnt2.GetBoundingBox(true));
EXPECT_EQ(expectedBoxLinkFrame, linkEnt2.GetBoundingBox());

expectedBoxModelFrame = math::AxisAlignedBox(
math::Vector3d(-2, -2, -2.5), math::Vector3d(2, 3, 3));
EXPECT_EQ(expectedBoxModelFrame, model.GetBoundingBox(true));
EXPECT_EQ(expectedBoxModelFrame, model.GetBoundingBox());
}

/////////////////////////////////////////////////
Expand Down