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

Prevent default-constructed variants from holding a type #371

Merged
merged 1 commit into from
Aug 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions Migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ release will remove the deprecated code.
+ Added `Scene::SetCameraPassCountPerGpuFlush`. Setting this value to 0 forces legacy behavior which eases porting.
+ Systems that rely on Graphics components like particle FXs and postprocessing are explicitly affected by Scene's Pre/PostRender. Once `Scene::PostRender` is called, the particle FXs' simulation is moved forward, as well as time values sent to postprocessing shaders. In previous ign-rendering versions each `Camera::Render` call would move the particle simulation forward, which could cause subtle bugs or inconsistencies when Cameras were rendering the same frame from different angles. Setting SetCameraPassCountPerGpuFlush to 0 will also cause these subtle bugs to reappear.

1. **Visual.hh** and **Node.hh**
+ `*UserData` methods and the `Variant` type alias have been moved from the `Visual` class to the `Node` class.
`Node::UserData` now returns no data for keys that don't exist (prior to Rendering 6.x, if
`Visual::UserData` was called with a key that doesn't exist, an `int` was returned by default).
adlarkin marked this conversation as resolved.
Show resolved Hide resolved

## Ignition Rendering 4.0 to 4.1

## ABI break
Expand Down
25 changes: 17 additions & 8 deletions include/ignition/rendering/Node.hh
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,16 @@ namespace ignition
{
inline namespace IGNITION_RENDERING_VERSION_NAMESPACE {
//
/// \brief Alias for a variant that can hold various types of data.
/// The first type of the variant is std::monostate in order to prevent
/// default-constructed variants from holding a type (a default-constructed
/// variant is returned when a user calls Node::UserData with a key that
/// doesn't exist for the node. In this case, since the key doesn't
/// exist, the variant that is returned shouldn't hold any types - an
/// "empty variant" should be returned for keys that don't exist)
using Variant =
std::variant<int, float, double, std::string, bool, unsigned int>;
std::variant<std::monostate, int, float, double, std::string, bool,
unsigned int>;

/// \class Node Node.hh ignition/rendering/Node.hh
/// \brief Represents a single posable node in the scene graph
Expand Down Expand Up @@ -228,12 +236,12 @@ namespace ignition
/// \param[in] _scale Scalars to alter the current scale
public: virtual void Scale(const math::Vector3d &_scale) = 0;

/// \brief Determine if this visual inherits scale from this parent
/// \return True if this visual inherits scale from this parent
/// \brief Determine if this node inherits scale from this parent
/// \return True if this node inherits scale from this parent
public: virtual bool InheritScale() const = 0;

/// \brief Specify if this visual inherits scale from its parent
/// \param[in] _inherit True if this visual inherits scale from its parent
/// \brief Specify if this node inherits scale from its parent
/// \param[in] _inherit True if this node inherits scale from its parent
public: virtual void SetInheritScale(bool _inherit) = 0;

/// \brief Get number of child nodes
Expand Down Expand Up @@ -309,15 +317,16 @@ namespace ignition
/// This detaches all the child nodes but does not destroy them
public: virtual void RemoveChildren() = 0;

/// \brief Store any custom data associated with this visual
/// \brief Store any custom data associated with this node
/// \param[in] _key Unique key
/// \param[in] _value Value in any type
public: virtual void SetUserData(
const std::string &_key, Variant _value) = 0;

/// \brief Get custom data stored in this visual
/// \brief Get custom data stored in this node
/// \param[in] _key Unique key
/// \return Value in any type
/// \return Value in any type. If _key does not exist for the node, an
/// empty variant is returned (i.e., no data).
public: virtual Variant UserData(const std::string &_key) const = 0;
};
}
Expand Down
23 changes: 23 additions & 0 deletions src/Visual_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,29 @@ void VisualTest::UserData(const std::string &_renderEngine)
value = visual->UserData(stringKey);
EXPECT_EQ(stringValue, std::get<std::string>(value));

// bool
std::string boolKey = "bool";
bool boolValue = true;
visual->SetUserData(boolKey, boolValue);
value = visual->UserData(boolKey);
EXPECT_EQ(boolValue, std::get<bool>(value));

// unsigned int
std::string unsignedIntKey = "unsignedInt";
unsigned int unsignedIntValue = 5u;
visual->SetUserData(unsignedIntKey, unsignedIntValue);
value = visual->UserData(unsignedIntKey);
EXPECT_EQ(unsignedIntValue, std::get<unsigned int>(value));

// test a key that does not exist (should return no data)
value = visual->UserData("invalidKey");
EXPECT_FALSE(std::holds_alternative<int>(value));
EXPECT_FALSE(std::holds_alternative<float>(value));
EXPECT_FALSE(std::holds_alternative<double>(value));
EXPECT_FALSE(std::holds_alternative<std::string>(value));
EXPECT_FALSE(std::holds_alternative<bool>(value));
EXPECT_FALSE(std::holds_alternative<unsigned int>(value));

adlarkin marked this conversation as resolved.
Show resolved Hide resolved
// test invalid access
EXPECT_THROW(
{
Expand Down