Color: Invariant for channels within `[0, 1]` is not strictly enforced; implicity divison is weird

[EricCousineau-TRI]

Environment

• OS Version: N/A
• Source or binary build? Current master (see source permalink)

Description

Came across this during @jennuine's resolution of https://github.com/osrf/sdformat/issues/193

• Expected behavior:

  • Color channel values are all strictly w/in [0, 1]
  • An error is thrown (rather than weird implicit conversion)

• Actual behavior: Here's what @seancurtis-tri found:

  // Case: Values out of range:
  //  A (alpha) simply gets clamped to the range [0, 1]
  //  For each individual element in R, G, B:
  //    Negative values are set to zero.
  //    Values > 1 are divided by 255

  https://github.com/RobotLocomotion/drake/blob/v0.28.0/multibody/parsing/test/detail_scene_graph_test.cc#L861-L867

This is due to the following code: https://github.com/ignitionrobotics/ign-math/blob/ad0118e689ba531c93df9c332e0c78a5dcd84e6e/src/Color.cc#L498-L511

Steps to reproduce

See code

Output

N/A

\cc @azeey

[EricCousineau-TRI]

Per @jennuine's analysis, this invariant is now enforced in libsdformat XML parsing (https://github.com/osrf/sdformat/pull/519), but due to this issue, it can leak in via DOM mutators.

[jwnimmer-tri]

What's even weirder is that the copy constructor keeps dividing again on each copy, in case it was still over the limit: https://github.com/ignitionrobotics/ign-math/blob/797f67f89c6863b5e02b6e7b24b6c587396cc614/src/Color.cc#L47-L51

Whichever of #283 or #286 merges will fix that part, though, by marked the copy constructor as defaulted.