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

Using Element::get<Color> has surprising defaults #193

Closed
osrf-migration opened this issue Jul 2, 2018 · 2 comments · Fixed by #519
Closed

Using Element::get<Color> has surprising defaults #193

osrf-migration opened this issue Jul 2, 2018 · 2 comments · Fixed by #519
Assignees
Labels
bug Something isn't working major

Comments

@osrf-migration
Copy link

Original report (archived issue) by SeanCurtis-TRI (Bitbucket: SeanCurtis-TRI).


When parsing a <material> tag for (e.g.,) diffuse color information, one can do the following (assuming that material_element is an sdf::Element):

Color default_color(0.25, 0.5, 0.75, 0.8);
material_element.Get<Color>("diffuse", default_color);

The specification suggests that the corresponding tag should look like:
<diffuse>r g b a</diffuse>

However, the following tags produce the following colors:
<diffuse>1</diffuse --> 1 0 0 1
<diffuse>1 0.25</diffuse> --> 1 0.25 0 1
<diffuse>1 0.25 0.5</diffuse> --> 1 0.25 0.5 1
<diffuse>1 0.25 0.5 0.75 2</diffuse> --> 1 0.25 0.5 0.75

The rules I infer are:

  1. If a is missing, use the default value 1.
  2. if r, g, or b are missing, use the default value 0
  3. If there are two many values, simply truncate.

I think this is incredibly counter-intuitive and not strictly helpful. One can make solid arguments that supplying rgb (in place of rgba) is a reasonable overload. But all of the rest seem kind of insane.

@osrf-migration
Copy link
Author

Original comment by SeanCurtis-TRI (Bitbucket: SeanCurtis-TRI).


Also, out-of-range values get treated....strangely.

Alpha seems to simply get clamped to the range [0, 1]
R, G, & B get a different treatment. They are clamped to zero if negative and set to i/255 if larger than 1 (with no guarantees that the resultant value is still in the range [0, 1]).

@osrf-migration osrf-migration added major bug Something isn't working labels Apr 11, 2020
@EricCousineau-TRI
Copy link
Collaborator

Still awkward in RobotLocomotion/drake#14705
\cc @SeanCurtis-TRI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working major
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants