Skip to content

Parse category bitmask SDF element in Surface DOM #1630

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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iche033 merged 4 commits into from
Mar 3, 2026
Merged

Parse category bitmask SDF element in Surface DOM #1630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c445027
add support for parsing categorty bitmask
iche033 Feb 18, 2026
1f1aca0
use optional
iche033 Feb 19, 2026
aadd53a
add include
iche033 Feb 19, 2026
a1bbbb4
add python bindings
iche033 Feb 26, 2026
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
12 changes: 12 additions & 0 deletions include/sdf/Surface.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
#ifndef SDF_SURFACE_HH_
#define SDF_SURFACE_HH_

#include <cstdint>
#include <optional>

#include <gz/math/Vector3.hh>
#include <gz/utils/ImplPtr.hh>
#include "sdf/Element.hh"
Expand Down Expand Up @@ -53,8 +56,17 @@ namespace sdf
public: uint16_t CollideBitmask() const;

/// \brief Set the collide bitmask parameter.
/// \param[in] _bitmask Category bitmask to set
public: void SetCollideBitmask(const uint16_t _bitmask);

/// \brief Get the category bitmask parameter.
/// \return The category bitmask parameter.
public: std::optional<uint16_t> CategoryBitmask() const;
Comment thread
iche033 marked this conversation as resolved.

@scpeters scpeters Feb 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

does it make sense to return a const reference?

Suggested change
public: std::optional<uint16_t> CategoryBitmask() const;
public: const std::optional<uint16_t> &CategoryBitmask() const;

@iche033 iche033 Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

after doing some reading online and asking AI, it seems that for simple primitive types like uint16_t it's more efficient to just return by value, i.e.std::optional<uint16_t> instead of const ref.

I do see some places in sdformat that returns const std::optional & (there are also other places that just return by value), e.g.

sdformat/include/sdf/InterfaceElements.hh

Line 102 in dcd3c2d

public: const std::optional<bool> &IsStatic() const;

so I'm ok to match the style for consistency or clarity reasons.

@scpeters scpeters Feb 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

CPP core guideline F.15 suggests preferring simple and conventional ways of passing information, and the example indicate returning by value rather than a const reference. In the case where data is expensive to move, it recommends passing an input/output reference to the function

so I think you can ignore my earlier suggestion and keep it as is

I remember seeing lint warnings recommending more use of const references and I took it a little farther than necessary


/// \brief Set the category bitmask parameter.
/// \param[in] _bitmask Category bitmask to set
public: void SetCategoryBitmask(const uint16_t _bitmask);

/// \brief Private data pointer.
GZ_UTILS_IMPL_PTR(dataPtr)
};
Expand Down
6 changes: 6 additions & 0 deletions python/src/sdf/pySurface.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "pySurface.hh"

#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "sdf/Surface.hh"

Expand All @@ -37,6 +38,11 @@ void defineContact(pybind11::object module)
"Get the collide bitmask parameter.")
.def("set_collide_bitmask", &sdf::Contact::SetCollideBitmask,
"Set the collide bitmask parameter.")
.def("category_bitmask", &sdf::Contact::CategoryBitmask,
"Get the category bitmask parameter.")
.def("set_category_bitmask", &sdf::Contact::SetCategoryBitmask,
"Set the category bitmask parameter.")

.def("__copy__", [](const sdf::Contact &self) {
return sdf::Contact(self);
})
Expand Down
19 changes: 19 additions & 0 deletions python/test/pySurface_TEST.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class SurfaceTEST(unittest.TestCase):
def test_default_construction(self):
surface = Surface()
self.assertEqual(surface.contact().collide_bitmask(), 0xFF)
self.assertFalse(surface.contact().category_bitmask())


def test_assigment_construction(self):
Expand Down Expand Up @@ -54,11 +55,13 @@ def test_assigment_construction(self):
friction.set_bullet_friction(bullet)
friction.set_torsional(torsional)
contact.set_collide_bitmask(0x12)
contact.set_category_bitmask(0x22)
surface1.set_contact(contact)
surface1.set_friction(friction)

surface2 = surface1
self.assertEqual(surface2.contact().collide_bitmask(), 0x12)
self.assertEqual(surface2.contact().category_bitmask(), 0x22)
self.assertEqual(surface2.friction().ode().mu(), 0.1)
self.assertEqual(surface2.friction().ode().mu2(), 0.2)
self.assertEqual(surface2.friction().ode().slip1(), 3)
Expand All @@ -78,9 +81,12 @@ def test_assigment_construction(self):
self.assertEqual(surface2.friction().torsional().ode_slip(), 0.01)

contact.set_collide_bitmask(0x21)
contact.set_category_bitmask(0x33)
surface1.set_contact(contact)
self.assertEqual(surface1.contact().collide_bitmask(), 0x21)
self.assertEqual(surface2.contact().collide_bitmask(), 0x21)
self.assertEqual(surface1.contact().category_bitmask(), 0x33)
self.assertEqual(surface2.contact().category_bitmask(), 0x33)

ode.set_mu(1.1)
ode.set_mu2(1.2)
Expand Down Expand Up @@ -165,11 +171,13 @@ def test_copy_construction(self):
friction.set_bullet_friction(bullet)
friction.set_torsional(torsional)
contact.set_collide_bitmask(0x12)
contact.set_category_bitmask(0x22)
surface1.set_contact(contact)
surface1.set_friction(friction)

surface2 = Surface(surface1)
self.assertEqual(surface2.contact().collide_bitmask(), 0x12)
self.assertEqual(surface2.contact().category_bitmask(), 0x22)
self.assertEqual(surface2.friction().ode().mu(), 0.1)
self.assertEqual(surface2.friction().ode().mu2(), 0.2)
self.assertEqual(surface2.friction().ode().slip1(), 3)
Expand All @@ -189,9 +197,12 @@ def test_copy_construction(self):
self.assertEqual(surface2.friction().torsional().ode_slip(), 0.01)

contact.set_collide_bitmask(0x21)
contact.set_category_bitmask(0x33)
surface1.set_contact(contact)
self.assertEqual(surface1.contact().collide_bitmask(), 0x21)
self.assertEqual(surface2.contact().collide_bitmask(), 0x12)
self.assertEqual(surface1.contact().category_bitmask(), 0x33)
self.assertEqual(surface2.contact().category_bitmask(), 0x22)

ode.set_mu(1.1)
ode.set_mu2(1.2)
Expand Down Expand Up @@ -276,11 +287,13 @@ def test_deepcopy(self):
friction.set_bullet_friction(bullet)
friction.set_torsional(torsional)
contact.set_collide_bitmask(0x12)
contact.set_category_bitmask(0x22)
surface1.set_contact(contact)
surface1.set_friction(friction)

surface2 = copy.deepcopy(surface1)
self.assertEqual(surface2.contact().collide_bitmask(), 0x12)
self.assertEqual(surface2.contact().category_bitmask(), 0x22)
self.assertEqual(surface2.friction().ode().mu(), 0.1)
self.assertEqual(surface2.friction().ode().mu2(), 0.2)
self.assertEqual(surface2.friction().ode().slip1(), 3)
Expand All @@ -300,9 +313,12 @@ def test_deepcopy(self):
self.assertEqual(surface2.friction().torsional().ode_slip(), 0.01)

contact.set_collide_bitmask(0x21)
contact.set_category_bitmask(0x33)
surface1.set_contact(contact)
self.assertEqual(surface1.contact().collide_bitmask(), 0x21)
self.assertEqual(surface2.contact().collide_bitmask(), 0x12)
self.assertEqual(surface1.contact().category_bitmask(), 0x33)
self.assertEqual(surface2.contact().category_bitmask(), 0x22)

ode.set_mu(1.1)
ode.set_mu2(1.2)
Expand Down Expand Up @@ -363,14 +379,17 @@ def test_deepcopy(self):
def test_default_contact_construction(self):
contact = Contact()
self.assertEqual(contact.collide_bitmask(), 0xFF)
self.assertFalse(contact.category_bitmask())


def test_copy_contact_construction(self):
contact1 = Contact()
contact1.set_collide_bitmask(0x12)
contact1.set_category_bitmask(0x21)

contact2 = Contact(contact1)
self.assertEqual(contact2.collide_bitmask(), 0x12)
self.assertEqual(contact2.category_bitmask(), 0x21)

def test_default_ode_construction(self):
ode = ODE()
Expand Down
29 changes: 28 additions & 1 deletion src/Surface.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ using namespace sdf;

class sdf::Contact::Implementation
{
// \brief The bitmask used to filter collisions.
// \brief The collide bitmask used to filter collisions.
public: uint16_t collideBitmask = 0xff;

// \brief The category bitmask used to filter collisions.
public: std::optional<uint16_t> categoryBitmask;

/// \brief The SDF element pointer used during load.
public: sdf::ElementPtr sdf{nullptr};
};
Expand Down Expand Up @@ -605,6 +608,13 @@ Errors Contact::Load(ElementPtr _sdf)
errors, "collide_bitmask"));
}

if (_sdf->HasElement("category_bitmask"))
{
this->dataPtr->categoryBitmask =
static_cast<uint16_t>(_sdf->Get<unsigned int>(
errors, "category_bitmask"));
}

// \todo(nkoenig) Parse the remaining collide properties.
return errors;
}
Expand All @@ -627,6 +637,18 @@ void Contact::SetCollideBitmask(const uint16_t _bitmask)
this->dataPtr->collideBitmask = _bitmask;
}

/////////////////////////////////////////////////
std::optional<uint16_t> Contact::CategoryBitmask() const
{
return this->dataPtr->categoryBitmask;
}

/////////////////////////////////////////////////
void Contact::SetCategoryBitmask(const uint16_t _bitmask)
{
this->dataPtr->categoryBitmask = _bitmask;
}

/////////////////////////////////////////////////
Surface::Surface()
: dataPtr(gz::utils::MakeImpl<Implementation>())
Expand Down Expand Up @@ -724,6 +746,11 @@ sdf::ElementPtr Surface::ToElement(sdf::Errors &_errors) const
sdf::ElementPtr contactElem = elem->GetElement("contact", _errors);
contactElem->GetElement("collide_bitmask", _errors)->Set(
_errors, this->dataPtr->contact.CollideBitmask());
if (this->dataPtr->contact.CategoryBitmask().has_value())
{
contactElem->GetElement("category_bitmask", _errors)->Set(
_errors, this->dataPtr->contact.CategoryBitmask().value());
}

sdf::ElementPtr frictionElem = elem->GetElement("friction", _errors);

Expand Down
24 changes: 24 additions & 0 deletions src/Surface_TEST.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ TEST(DOMsurface, DefaultConstruction)
sdf::Surface surface;
EXPECT_EQ(nullptr, surface.Element());
EXPECT_EQ(surface.Contact()->CollideBitmask(), 0xFF);
EXPECT_FALSE(surface.Contact()->CategoryBitmask().has_value());
EXPECT_EQ(surface.Contact()->Element(), nullptr);
EXPECT_EQ(surface.Friction()->Element(), nullptr);
}
Expand All @@ -44,11 +45,13 @@ TEST(DOMsurface, CopyOperator)
sdf::Friction friction;
friction.SetODE(ode);
contact.SetCollideBitmask(0x12);
contact.SetCategoryBitmask(0x21);
surface1.SetContact(contact);
surface1.SetFriction(friction);

sdf::Surface surface2(surface1);
EXPECT_EQ(surface2.Contact()->CollideBitmask(), 0x12);
EXPECT_EQ(surface2.Contact()->CategoryBitmask(), 0x21);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Mu(), 0.1);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Mu2(), 0.2);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Slip1(), 3);
Expand All @@ -71,11 +74,13 @@ TEST(DOMsurface, CopyAssignmentOperator)
sdf::Friction friction;
friction.SetODE(ode);
contact.SetCollideBitmask(0x12);
contact.SetCategoryBitmask(0x21);
surface1.SetContact(contact);
surface1.SetFriction(friction);

sdf::Surface surface2 = surface1;
EXPECT_EQ(surface2.Contact()->CollideBitmask(), 0x12);
EXPECT_EQ(surface2.Contact()->CategoryBitmask(), 0x21);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Mu(), 0.1);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Mu2(), 0.2);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Slip1(), 3);
Expand All @@ -92,9 +97,11 @@ TEST(DOMsurface, CopyAssignmentAfterMove)

sdf::Contact contact1;
contact1.SetCollideBitmask(0x12);
contact1.SetCategoryBitmask(0x21);
surface1.SetContact(contact1);
sdf::Contact contact2;
contact2.SetCollideBitmask(0x34);
contact2.SetCategoryBitmask(0x43);
surface2.SetContact(contact2);

sdf::ODE ode1;
Expand Down Expand Up @@ -124,6 +131,8 @@ TEST(DOMsurface, CopyAssignmentAfterMove)

EXPECT_EQ(surface2.Contact()->CollideBitmask(), 0x12);
EXPECT_EQ(surface1.Contact()->CollideBitmask(), 0x34);
EXPECT_EQ(surface2.Contact()->CategoryBitmask(), 0x21);
EXPECT_EQ(surface1.Contact()->CategoryBitmask(), 0x43);
EXPECT_DOUBLE_EQ(surface1.Friction()->ODE()->Mu(), 0.2);
EXPECT_DOUBLE_EQ(surface1.Friction()->ODE()->Mu2(), 0.1);
EXPECT_DOUBLE_EQ(surface1.Friction()->ODE()->Slip1(), 7);
Expand Down Expand Up @@ -165,6 +174,7 @@ TEST(DOMsurface, ToElement)
torsional.SetODESlip(0.2);
friction.SetTorsional(torsional);
contact.SetCollideBitmask(0x12);
contact.SetCategoryBitmask(0x21);
surface1.SetContact(contact);
surface1.SetFriction(friction);

Expand All @@ -175,6 +185,7 @@ TEST(DOMsurface, ToElement)
surface2.Load(elem);

EXPECT_EQ(surface2.Contact()->CollideBitmask(), 0x12);
EXPECT_EQ(surface2.Contact()->CategoryBitmask(), 0x21);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Mu(), 0.1);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Mu2(), 0.2);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Slip1(), 3);
Expand Down Expand Up @@ -224,6 +235,7 @@ TEST(DOMsurface, ToElementErrorOutput)
ode.SetFdir1(gz::math::Vector3d(1, 2, 3));
friction.SetODE(ode);
contact.SetCollideBitmask(0x12);
contact.SetCategoryBitmask(0x21);
surface1.SetContact(contact);
surface1.SetFriction(friction);

Expand All @@ -236,6 +248,7 @@ TEST(DOMsurface, ToElementErrorOutput)
EXPECT_TRUE(errors.empty());

EXPECT_EQ(surface2.Contact()->CollideBitmask(), 0x12);
EXPECT_EQ(surface2.Contact()->CategoryBitmask(), 0x21);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Mu(), 0.1);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Mu2(), 0.2);
EXPECT_DOUBLE_EQ(surface2.Friction()->ODE()->Slip1(), 3);
Expand All @@ -254,6 +267,7 @@ TEST(DOMcontact, DefaultConstruction)
sdf::Contact contact;
EXPECT_EQ(nullptr, contact.Element());
EXPECT_EQ(contact.CollideBitmask(), 0xFF);
EXPECT_FALSE(contact.CategoryBitmask().has_value());
EXPECT_EQ(contact.Element(), nullptr);
}

Expand All @@ -262,19 +276,23 @@ TEST(DOMcontact, CopyOperator)
{
sdf::Contact contact1;
contact1.SetCollideBitmask(0x12);
contact1.SetCategoryBitmask(0x21);

sdf::Contact contact2(contact1);
EXPECT_EQ(contact2.CollideBitmask(), 0x12);
EXPECT_EQ(contact2.CategoryBitmask(), 0x21);
}

/////////////////////////////////////////////////
TEST(DOMcontact, CopyAssignmentOperator)
{
sdf::Contact contact1;
contact1.SetCollideBitmask(0x12);
contact1.SetCategoryBitmask(0x21);

sdf::Contact contact2 = contact1;
EXPECT_EQ(contact2.CollideBitmask(), 0x12);
EXPECT_EQ(contact2.CategoryBitmask(), 0x21);
}

/////////////////////////////////////////////////
Expand All @@ -285,21 +303,27 @@ TEST(DOMcontact, CopyAssignmentAfterMove)

contact1.SetCollideBitmask(0x12);
contact2.SetCollideBitmask(0x34);
contact1.SetCategoryBitmask(0x21);
contact2.SetCategoryBitmask(0x43);

sdf::Contact tmp = std::move(contact1);
contact1 = contact2;
contact2 = tmp;

EXPECT_EQ(contact2.CollideBitmask(), 0x12);
EXPECT_EQ(contact1.CollideBitmask(), 0x34);
EXPECT_EQ(contact2.CategoryBitmask(), 0x21);
EXPECT_EQ(contact1.CategoryBitmask(), 0x43);
}

/////////////////////////////////////////////////
TEST(DOMcontact, CollideBitmask)
{
sdf::Contact contact;
contact.SetCollideBitmask(0x67);
contact.SetCategoryBitmask(0x76);
EXPECT_EQ(contact.CollideBitmask(), 0x67);
EXPECT_EQ(contact.CategoryBitmask(), 0x76);
}

/////////////////////////////////////////////////
Expand Down
1 change: 1 addition & 0 deletions test/integration/surface_dom.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ TEST(DOMSurface, Shapes)
ASSERT_NE(nullptr, boxCol->Surface());
ASSERT_NE(nullptr, boxCol->Surface()->Contact());
EXPECT_EQ(boxCol->Surface()->Contact()->CollideBitmask(), 0xAB);
EXPECT_EQ(boxCol->Surface()->Contact()->CategoryBitmask(), 0xBA);
EXPECT_DOUBLE_EQ(boxCol->Surface()->Friction()->ODE()->Mu(), 0.6);
EXPECT_DOUBLE_EQ(boxCol->Surface()->Friction()->ODE()->Mu2(), 0.7);
EXPECT_DOUBLE_EQ(boxCol->Surface()->Friction()->ODE()->Slip1(), 4);
Expand Down
1 change: 1 addition & 0 deletions test/sdf/shapes.sdf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<surface>
<contact>
<collide_bitmask>0xAB</collide_bitmask>
<category_bitmask>0xBA</category_bitmask>
</contact>
<friction>
<ode>
Expand Down
Loading