Skip to content

Commit

Permalink
moved color parsing to Param
Browse files Browse the repository at this point in the history 
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
  • Loading branch information
@jennuine
jennuine committed Mar 23, 2021
1 parent 74ab6b4 commit 39c05a5
Show file tree
Hide file tree
Showing 8 changed files with 275 additions and 200 deletions.
90 changes: 8 additions & 82 deletions src/Material.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*
*/
#include <sstream>
#include <string>
#include <optional>
#include <vector>
Expand Down Expand Up @@ -70,18 +69,6 @@ class sdf::Material::Implementation

/// \brief The path to the file where this material was defined.
public: std::string filePath = "";

/// \brief Get a valid color. This checks the color components specified in
/// this->sdf are r,g,b or r,g,b,a (expects 3 or 4 values) and each value is
/// between [0,1]. When only 3 values are present (r,g,b), then alpha is set
/// to 1. If the checks do not pass, the default color 0,0,0,1 is returned.
/// \param[in] _colorType The color type (i.e., ambient, diffuse, specular,
/// or emissive), which is case sensitive and expects a lowercase string
/// \param[out] _errors Parsing errors will be appended to this variable
/// \return A color for _colorType. If no parsing errors occurred then the
/// specified color component from this->sdf is returned, otherwise 0,0,0,1
public: ignition::math::Color GetColor(const std::string &_colorType,
Errors &_errors);
};

/////////////////////////////////////////////////
Expand Down Expand Up @@ -181,13 +168,17 @@ Errors Material::Load(sdf::ElementPtr _sdf)
this->dataPtr->renderOrder = _sdf->Get<float>("render_order",
this->dataPtr->renderOrder).first;

this->dataPtr->ambient = this->dataPtr->GetColor("ambient", errors);
this->dataPtr->ambient = _sdf->Get<ignition::math::Color>("ambient",
this->dataPtr->ambient).first;

this->dataPtr->diffuse = this->dataPtr->GetColor("diffuse", errors);
this->dataPtr->diffuse = _sdf->Get<ignition::math::Color>("diffuse",
this->dataPtr->diffuse).first;

this->dataPtr->specular = this->dataPtr->GetColor("specular", errors);
this->dataPtr->specular = _sdf->Get<ignition::math::Color>("specular",
this->dataPtr->specular).first;

this->dataPtr->emissive = this->dataPtr->GetColor("emissive", errors);
this->dataPtr->emissive = _sdf->Get<ignition::math::Color>("emissive",
this->dataPtr->emissive).first;

this->dataPtr->lighting = _sdf->Get<bool>("lighting",
this->dataPtr->lighting).first;
Expand Down Expand Up @@ -367,68 +358,3 @@ void Material::SetFilePath(const std::string &_filePath)
{
this->dataPtr->filePath = _filePath;
}

//////////////////////////////////////////////////
ignition::math::Color Material::Implementation::GetColor(
const std::string &_colorType, Errors &_errors)
{
ignition::math::Color color(0, 0, 0, 1);

if (!this->sdf->HasElement(_colorType))
return color;

std::string colorStr = this->sdf->GetElement(_colorType)
->GetValue()
->GetAsString();
if (colorStr.empty())
{
_errors.push_back({ErrorCode::ELEMENT_INVALID, "No values found inside <" +
_colorType + "> element. Using default values 0 0 0 1."});
return color;
}

std::stringstream ss(colorStr);
std::string token;
std::vector<float> colors;
float c; // r,g,b,a values
bool isValidColor = true;
while (ss >> token)
{
try
{
c = std::stof(token);
colors.push_back(c);
}
catch(const std::exception &/*e*/)
{
std::cerr << "Error converting color value: can not convert ["
<< token << "] to a float" << std::endl;
isValidColor = false;
break;
}

if (c < 0.0f || c > 1.0f)
{
isValidColor = false;
break;
}
}

size_t colorSize = colors.size();
if (colorSize == 3u)
colors.push_back(1.0f);
else if (colorSize != 4u)
isValidColor = false;

if (!isValidColor)
{
_errors.push_back({ErrorCode::ELEMENT_INVALID, "The value <" + _colorType +
">" + colorStr + "</" + _colorType + "> is invalid. " +
"Using default values 0 0 0 1 for <" + _colorType + "> element."});
return color;
}

color.Set(colors[0], colors[1], colors[2], colors[3]);

return color;
}
101 changes: 0 additions & 101 deletions src/Material_TEST.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,104 +289,3 @@ TEST(DOMMaterial, InvalidSdf)
sdf::Errors errors = material.Load(elem);
EXPECT_EQ(sdf::ErrorCode::ELEMENT_INCORRECT_TYPE, errors[0].Code());
}

void PrintErrors(sdf::Errors &_errors)
{
for (const auto &e : _errors)
std::cout << e << std::endl;
}

/////////////////////////////////////////////////
TEST(DOMMaterial, Colors)
{
sdf::ElementPtr elem(new sdf::Element());
elem->SetName("material");

// invalid diffuse (empty)
sdf::ElementPtr elemDiffuse(new sdf::Element());
elemDiffuse->SetName("diffuse");
elemDiffuse->AddValue("string", "", false, "description");
elem->InsertElement(elemDiffuse);

sdf::Material material;
sdf::Errors errors = material.Load(elem);
PrintErrors(errors);

ASSERT_EQ(errors.size(), 1u);
EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::ELEMENT_INVALID);
EXPECT_EQ(material.Ambient(), ignition::math::Color(0, 0, 0, 1));
EXPECT_EQ(material.Diffuse(), ignition::math::Color(0, 0, 0, 1));
EXPECT_EQ(material.Specular(), ignition::math::Color(0, 0, 0, 1));
EXPECT_EQ(material.Emissive(), ignition::math::Color(0, 0, 0, 1));

// another invalid diffuse (value is > 1)
elemDiffuse->Set<std::string>("1.5 0 0 1");
errors.clear();
errors = material.Load(elem);
PrintErrors(errors);

ASSERT_EQ(errors.size(), 1u);
EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::ELEMENT_INVALID);
EXPECT_EQ(material.Diffuse(), ignition::math::Color(0, 0, 0, 1));

// another invalid diffuse (only 2 values)
elemDiffuse->Set<std::string>("0.1 0.2");
errors.clear();
errors = material.Load(elem);
PrintErrors(errors);

ASSERT_EQ(errors.size(), 1u);
EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::ELEMENT_INVALID);
EXPECT_EQ(material.Diffuse(), ignition::math::Color(0, 0, 0, 1));

// invalid ambient
sdf::ElementPtr elemAmbient(new sdf::Element());
elemAmbient->AddValue("string", "", false, "description");
elemAmbient->SetName("ambient");
elemAmbient->Set<std::string>("0.1 0.2 test");
elem->InsertElement(elemAmbient);

// invalid specular
sdf::ElementPtr elemSpecular(new sdf::Element());
elemSpecular->AddValue("string", "", false, "description");
elemSpecular->SetName("specular");
elemSpecular->Set<std::string>("0.1 0.2 0.3 0.4 0.5");
elem->InsertElement(elemSpecular);

// invalid emissive
sdf::ElementPtr elemEmissive(new sdf::Element());
elemEmissive->AddValue("string", "", false, "description");
elemEmissive->SetName("emissive");
elemEmissive->Set<std::string>("-0.1 0.2 0.3 0.4");
elem->InsertElement(elemEmissive);

errors.clear();
errors = material.Load(elem);
PrintErrors(errors);

EXPECT_EQ(errors.size(), 4u);
for (const auto &e : errors)
EXPECT_EQ(e.Code(), sdf::ErrorCode::ELEMENT_INVALID);

EXPECT_EQ(material.Ambient(), ignition::math::Color(0, 0, 0, 1));
EXPECT_EQ(material.Diffuse(), ignition::math::Color(0, 0, 0, 1));
EXPECT_EQ(material.Specular(), ignition::math::Color(0, 0, 0, 1));
EXPECT_EQ(material.Emissive(), ignition::math::Color(0, 0, 0, 1));

// valid diffuse, ambient, specular, emissive
elemDiffuse->Set<std::string>("0 0.1 0.2");
elemAmbient->Set<std::string>("0.3 0.4 0.55 1");
elemSpecular->Set<std::string>("0 0.1 0.2 0.3");
elemEmissive->Set<std::string>("0.12 0.23 0.34 0.56");

errors.clear();
errors = material.Load(elem);
PrintErrors(errors);

EXPECT_EQ(errors.size(), 0u);
EXPECT_EQ(material.Diffuse(), ignition::math::Color(0, 0.1f, 0.2f, 1));
EXPECT_EQ(material.Ambient(), ignition::math::Color(0.3f, 0.4f, 0.55f, 1));
EXPECT_EQ(material.Specular(), ignition::math::Color(0, 0.1f, 0.2f, 0.3f));
EXPECT_EQ(material.Emissive(),
ignition::math::Color(0.12f, 0.23f, 0.34f, 0.56f));
}
76 changes: 60 additions & 16 deletions src/Param.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <locale>
#include <sstream>
#include <string>
#include <vector>

#include <locale.h>
#include <math.h>
Expand Down Expand Up @@ -359,6 +360,63 @@ bool ParseUsingStringStream(const std::string &_input, const std::string &_key,
return true;
}

//////////////////////////////////////////////////
/// \brief Helper function for Param::ValueFromString for parsing colors
/// This checks the color components specified in _input are rgb or rgba
/// (expects 3 or 4 values) and each value is between [0,1]. When only 3 values
/// are present (rgb), then alpha is set to 1.
/// \param[in] _input Input string.
/// \param[in] _key Key of the parameter, used for error message.
/// \param[out] _value This will be set with the parsed value.
/// \return True if parsing colors succeeded.
bool ParseColorUsingStringStream(const std::string &_input,
const std::string &_key, ParamPrivate::ParamVariant &_value)
{
StringStreamClassicLocale ss(_input);
std::string token;
std::vector<float> colors;
float c; // r,g,b,a values
bool isValidColor = true;
while (ss >> token)
{
try
{
c = std::stof(token);
colors.push_back(c);
}
catch(const std::exception &/*e*/)
{
sdferr << "Error converting color value [" << token << "] to a float\n";
isValidColor = false;
break;
}

if (c < 0.0f || c > 1.0f)
{
isValidColor = false;
break;
}
}

size_t colorSize = colors.size();
if (isValidColor && colorSize == 3u)
colors.push_back(1.0f);
else if (colorSize != 4u)
isValidColor = false;

if (isValidColor)
{
_value = ignition::math::Color(colors[0], colors[1], colors[2], colors[3]);
}
else
{
sdferr << "The value <" << _key <<
">" << _input << "</" << _key << "> is invalid.\n";
}

return isValidColor;
}

//////////////////////////////////////////////////
bool Param::ValueFromString(const std::string &_value)
{
Expand Down Expand Up @@ -448,22 +506,8 @@ bool Param::ValueFromString(const std::string &_value)
else if (this->dataPtr->typeName == "ignition::math::Color" ||
this->dataPtr->typeName == "color")
{
// The insertion operator (>>) expects 4 values, but the last value (the
// alpha) is optional. We first try to parse assuming the alpha is
// specified. If that fails, we append the default value of alpha to the
// string and try to parse again.
bool result = ParseUsingStringStream<ignition::math::Color>(
tmp, this->dataPtr->key, this->dataPtr->value);

if (!result)
{
ignition::math::Color colortmp;
return ParseUsingStringStream<ignition::math::Color>(
tmp + " " + std::to_string(colortmp.A()), this->dataPtr->key,
this->dataPtr->value);
}
else
return true;
return ParseColorUsingStringStream(
tmp, this->dataPtr->key, this->dataPtr->value);
}
else if (this->dataPtr->typeName == "ignition::math::Vector2i" ||
this->dataPtr->typeName == "vector2i")
Expand Down
9 changes: 8 additions & 1 deletion src/Param_TEST.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,13 @@ TEST(Param, InvalidConstructor)
sdf::AssertionInternalError);
}

////////////////////////////////////////////////////
TEST(Param, InvalidColorConstructor)
{
ASSERT_THROW(sdf::Param("key", "color", "8 20 67 23", false, "description"),
sdf::AssertionInternalError);
}

////////////////////////////////////////////////////
TEST(Param, SetDescription)
{
Expand Down Expand Up @@ -392,7 +399,7 @@ TEST(Param, GetAny)
sdf::Param timeParam("key", "time", "8 20", false, "description");
EXPECT_TRUE(timeParam.GetAny(anyValue));

sdf::Param colorParam("key", "color", "8 20 67 23", false, "description");
sdf::Param colorParam("key", "color", "0 0.1 0.2 0.3", false, "description");
EXPECT_TRUE(colorParam.GetAny(anyValue));

sdf::Param vector3Param("key", "vector3", "8.1 20.24 67.7", false,
Expand Down
1 change: 1 addition & 0 deletions test/integration/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ set(tests
locale_fix.cc
locale_fix_cxx.cc
material_pbr.cc
material.cc
model_dom.cc
model_versions.cc
nested_model.cc
Expand Down
Loading

0 comments on commit 39c05a5

Please sign in to comment.