Skip to content

Commit

Permalink
Update due to peer review from OCIO
Browse files Browse the repository at this point in the history 
Revise name of Blackbody to PlanckianLocus
Make colorimetry functions on color protected
Fix mistaken ToLinear/FromLinear swap
Add test fixture for tests to use protected functions.

(Internal change: 2334972)
  • Loading branch information
@meshula @pixar-oss
meshula authored and pixar-oss committed Jul 23, 2024
1 parent 9010646 commit a636a70
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 109 deletions.
47 changes: 17 additions & 30 deletions pxr/base/gf/color.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,47 +63,34 @@ GfColor::GfColor(const GfColor &srcColor, const GfColorSpace& dstColorSpace)
_rgb = GfVec3f(dstRGB.r, dstRGB.g, dstRGB.b);
}

// Set the color from a CIEXY coordinate in the chromaticity chart.
void GfColor::SetFromChromaticity(const GfVec2f& xy)
{
NcYxy c = { 1.f, xy[0], xy[1] };
NcRGB rgb = NcYxyToRGB(_colorSpace._data->colorSpace, c);
_rgb = GfVec3f(rgb.r, rgb.g, rgb.b);
}

// Set the color from a NcXYZ coordinate, in the existing color space.
void GfColor::SetFromXYZ(const GfVec3f& xyz)
{
NcXYZ ncxyz = { xyz[0], xyz[1], xyz[2] };
NcRGB dst = NcXYZToRGB(_colorSpace._data->colorSpace, ncxyz);
_rgb = GfVec3f(dst.r, dst.g, dst.b);
}

// Set the color from blackbody temperature in Kelvin,
// converting to the existing color space.
void GfColor::SetFromBlackbodyKelvin(float kelvin, float lumimance)
// Set the color from the Planckian locus (blackbody radiation) temperature
// in Kelvin, in the existing color space.
// Values are computed for temperatures between 1000K and 15000K.
// Note that temperatures below 1900K are out of gamut for Rec709.
void GfColor::SetFromPlanckianLocus(float kelvin, float lumimance)
{
NcYxy c = NcKelvinToYxy(kelvin, lumimance);
NcRGB rgb = NcYxyToRGB(_colorSpace._data->colorSpace, c);
_rgb = GfVec3f(rgb.r, rgb.g, rgb.b);
}

// Get the XYZ coordinate of the color.
GfVec3f GfColor::GetXYZ() const
{
NcRGB src = {_rgb[0], _rgb[1], _rgb[2]};
NcXYZ dst = NcRGBToXYZ(_colorSpace._data->colorSpace, src);
return GfVec3f(dst.x, dst.y, dst.z);
}

// Get the CIEXY coordinate of the color in the chromaticity chart.
// Get the CIEXY coordinate of the color in the chromaticity chart,
// For use in testing.
GF_API
GfVec2f GfColor::GetChromaticity() const
{
GfVec2f GfColor::_GetChromaticity() const {
NcRGB src = {_rgb[0], _rgb[1], _rgb[2]};
NcXYZ rgb = NcRGBToXYZ(_colorSpace._data->colorSpace, src);
NcYxy chroma = NcXYZToYxy(rgb);
return GfVec2f(chroma.x, chroma.y);
}

// Set the color from a CIEXY coordinate in the chromaticity chart.
// For use in testing.
GF_API
void GfColor::_SetFromChromaticity(const GfVec2f& xy) {
NcYxy c = { 1.f, xy[0], xy[1] };
NcRGB rgb = NcYxyToRGB(_colorSpace._data->colorSpace, c);
_rgb = GfVec3f(rgb.r, rgb.g, rgb.b);
}

PXR_NAMESPACE_CLOSE_SCOPE
42 changes: 15 additions & 27 deletions pxr/base/gf/color.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,26 +61,14 @@ class GfColor {
GF_API
GfColor(const GfColor &color, const GfColorSpace& colorSpace);

/// Set the color from an xy coordinate in the chromaticity chart,
/// in the existing color space. Note the xy is conventionally annotated
/// as lower case coordinates.
/// \param xy The xy coordinate.
GF_API
void SetFromChromaticity(const GfVec2f& xy);

/// Set the color from an XYZ coordinate, in the existing color space.
/// Note that XYZ is conventionally annotated as upper case coordinates.
/// \param XYZ The XYZ coordinate.
GF_API
void SetFromXYZ(const GfVec3f& XYZ);

/// Set the color from blackbody temperature in Kelvin, in the existing color space.
/// Set the color from the Planckian locus (blackbody radiation) temperature
/// in Kelvin, in the existing color space.
/// Values are computed for temperatures between 1000K and 15000K.
/// Note that temperatures below 1900K are out of gamut for Rec709.
/// \param kelvin The blackbody temperature in Kelvin.
/// \param kelvin The temperature in Kelvin.
/// \param luminance The desired luminance.
GF_API
void SetFromBlackbodyKelvin(float kelvin, float luminance);
void SetFromPlanckianLocus(float kelvin, float luminance);

/// Get the RGB tuple.
/// \return The RGB tuple.
Expand All @@ -90,16 +78,6 @@ class GfColor {
/// \return The color space.
GfColorSpace GetColorSpace() const { return _colorSpace; }

/// Get the XYZ coordinate of the color.
/// \return The XYZ coordinate.
GF_API
GfVec3f GetXYZ() const;

/// Get the chromaticity of the color.
/// \return The chromaticity.
GF_API
GfVec2f GetChromaticity() const;

/// Equality operator.
/// \param rh The right-hand side color.
/// \return True if the colors are equal, false otherwise.
Expand All @@ -112,9 +90,19 @@ class GfColor {
/// \return True if the colors are not equal, false otherwise.
bool operator !=(const GfColor &rh) const { return !(*this == rh); }

private:
protected:
GfColorSpace _colorSpace; ///< The color space.
GfVec3f _rgb; ///< The RGB tuple.

// Get the CIEXY coordinate of the color in the chromaticity chart,
// For use in testing.
GF_API
GfVec2f _GetChromaticity() const;

// Set the color from a CIEXY coordinate in the chromaticity chart.
// For use in testing.
GF_API
void _SetFromChromaticity(const GfVec2f& xy);
};


Expand Down
4 changes: 2 additions & 2 deletions pxr/base/gf/nc/nanocolor.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ struct NcColorSpace {

static void _NcInitColorSpace(NcColorSpace* cs);

static float nc_ToLinear(const NcColorSpace* cs, float t) {
static float nc_FromLinear(const NcColorSpace* cs, float t) {
const float gamma = cs->desc.gamma;
if (t < cs->K0 / cs->phi)
return t * cs->phi;
const float a = cs->desc.linearBias;
return (1.f + a) * powf(t, 1.f / gamma) - a;
}

static float nc_FromLinear(const NcColorSpace* cs, float t) {
static float nc_ToLinear(const NcColorSpace* cs, float t) {
const float gamma = cs->desc.gamma;
if (t < cs->K0)
return t / cs->phi;
Expand Down
5 changes: 3 additions & 2 deletions pxr/base/gf/nc/nanocolor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ typedef struct {
} NcRGB;

// NcM33f is a 3x3 matrix of floats used for color space conversions.
// It's stored in column major order, such that multiplying an NcRGB
// by an NcM33f will yield another NcRGB transformed by that matrix.
// It's stored in row major order, such that posting multiplying an NcRGB
// as a column vector by an NcM33f will yield another NcRGB column
// transformed by that matrix.
typedef struct {
float m[9];
} NcM33f;
Expand Down
129 changes: 81 additions & 48 deletions pxr/base/gf/testenv/testGfColor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,36 @@
#include "pxr/base/gf/color.h"
#include "pxr/base/gf/colorSpace.h"
#include "pxr/base/tf/diagnostic.h"
#include "../nc/nanocolor.h"
#include "../colorSpace_data.h"

PXR_NAMESPACE_USING_DIRECTIVE

bool ColorApproxEq(const GfColor& c1, const GfColor& c2)
class GfColorTest : public GfColor {
public:
// wrap the constructors
GfColorTest() : GfColor() {}
GfColorTest(const GfColorSpace& colorSpace) : GfColor(colorSpace) {}
GfColorTest(const GfVec3f &rgb, const GfColorSpace& colorSpace)
: GfColor(rgb, colorSpace) {}
GfColorTest(const GfColor& srcColor, const GfColorSpace& dstColorSpace)
: GfColor(srcColor, dstColorSpace) {}
GfColorTest(const GfColor& srcColor) : GfColor(srcColor) {}

// Get the CIEXY coordinate of the color in the chromaticity chart.
GfVec2f GetChromaticity() const {
return _GetChromaticity();
}

// Set the color from a CIEXY coordinate in the chromaticity chart.
void SetFromChromaticity(const GfVec2f& xy) {
_SetFromChromaticity(xy);
}

};


bool ColorApproxEq(const GfColorTest& c1, const GfColorTest& c2)
{
return GfIsClose(c1.GetRGB(), c2.GetRGB(), 1e-5f);
}
Expand Down Expand Up @@ -57,7 +83,9 @@ main(int argc, char *argv[])
GfColor mauveLinear(GfVec3f(0.5f, 0.25f, 0.125f), csLinearRec709);
GfColor mauveGamma(mauveLinear, csG22Rec709);

GfVec2f wpD65xy = GfColor(GfVec3f(1.0f, 1.0f, 1.0f), csLinearRec709).GetChromaticity();
GfVec2f cr_baseline_linear = GfColorTest(mauveLinear).GetChromaticity();
GfVec2f cr_baseline_curve = GfColorTest(mauveGamma).GetChromaticity();
GfVec2f wpD65xy = GfColorTest(GfVec3f(1.0f, 1.0f, 1.0f), csLinearRec709).GetChromaticity();

GfVec2f ap0Primaries[3] = {
{ 0.7347, 0.2653 },
Expand All @@ -81,6 +109,13 @@ main(int argc, char *argv[])
TF_AXIOM(c.GetColorSpace() == csLinearRec709);
TF_AXIOM(c.GetRGB() == GfVec3f(0, 0, 0));
}
// test copy construction
{
GfColor c(GfVec3f(0.5f, 0.5f, 0.5f), csSRGB);
GfColor c2(c);
TF_AXIOM(c2.GetColorSpace() == csSRGB);
TF_AXIOM(c2.GetRGB() == GfVec3f(0.5f, 0.5f, 0.5f));
}
// test construction with color space
{
GfColor c(csSRGB);
Expand Down Expand Up @@ -110,14 +145,12 @@ main(int argc, char *argv[])
// test CIE XY equality, and thus also GetChromaticity
{
// MauveLinear and Gamma are both have a D65 white point.
GfColor col_SRGB(mauveLinear, csSRGB);
GfColor col_ap0(col_SRGB, csAp0); // different white point
GfColor col_SRGBP3(col_ap0, csSRGBP3); // adapt to d65 for comparison
GfColor col_SRGB_2(col_ap0, csSRGB);
GfColor col_SRGB_3(col_SRGBP3, csSRGB);

GfVec2f cr_baseline_linear = mauveLinear.GetChromaticity();
GfVec2f cr_baseline_curve = mauveGamma.GetChromaticity();
GfColorTest col_SRGB(mauveLinear, csSRGB);
GfColorTest col_ap0(col_SRGB, csAp0); // different white point
GfColorTest col_SRGBP3(col_ap0, csSRGBP3); // adapt to d65 for comparison
GfColorTest col_SRGB_2(col_ap0, csSRGB);
GfColorTest col_SRGB_3(col_SRGBP3, csSRGB);

GfVec2f cr_SRGB = col_SRGB.GetChromaticity();
GfVec2f cr_SRGB_2 = col_SRGB_2.GetChromaticity();
GfVec2f cr_SRGB_3 = col_SRGB_3.GetChromaticity();
Expand All @@ -132,31 +165,31 @@ main(int argc, char *argv[])
{
//print out all the values as we go and report to Rick

GfColor colG22Rec709(mauveLinear, csG22Rec709);
GfColorTest colG22Rec709(mauveLinear, csG22Rec709);
TF_AXIOM(GfIsClose(colG22Rec709, mauveGamma, 1e-5f));
GfColor colLinRec709(colG22Rec709, csLinearRec709);
GfColorTest colLinRec709(colG22Rec709, csLinearRec709);
TF_AXIOM(GfIsClose(colLinRec709, mauveLinear, 1e-5f));

// verify assignment didn't mutate cs
TF_AXIOM(colG22Rec709.GetColorSpace() == csG22Rec709);

TF_AXIOM(colLinRec709.GetColorSpace() == csLinearRec709);
GfColor colSRGB_2(colLinRec709, csSRGB);
GfColorTest colSRGB_2(colLinRec709, csSRGB);
GfVec2f xy1 = colG22Rec709.GetChromaticity();
GfVec2f xy2 = colSRGB_2.GetChromaticity();
TF_AXIOM(GfIsClose(xy1, xy2, 1e-5f));
GfColor colAp0(colSRGB_2, csAp0);
GfColorTest colAp0(colSRGB_2, csAp0);
GfVec2f xy3 = colAp0.GetChromaticity();
TF_AXIOM(GfIsClose(xy1, xy3, 3e-2f));
GfColor colSRGB_3(colAp0, csSRGB);
GfColorTest colSRGB_3(colAp0, csSRGB);
GfVec2f xy4 = colAp0.GetChromaticity();
TF_AXIOM(GfIsClose(xy1, xy4, 3e-2f));
GfColor col_SRGBP3(colSRGB_3, csSRGBP3);
GfColorTest col_SRGBP3(colSRGB_3, csSRGBP3);
GfVec2f xy5 = col_SRGBP3.GetChromaticity();
TF_AXIOM(GfIsClose(xy1, xy5, 3e-2f));

// all the way back to rec709
GfColor colLinRec709_2(col_SRGBP3, csLinearRec709);
GfColorTest colLinRec709_2(col_SRGBP3, csLinearRec709);
TF_AXIOM(GfIsClose(colLinRec709_2, colLinRec709, 1e-5f));
}
// test move constructor
Expand Down Expand Up @@ -194,42 +227,42 @@ main(int argc, char *argv[])
// the chromaticity of D65, even though spectrally, they are unrelated.
// nb. 6504 is the CCT match to D65
{
GfColor c;
c.SetFromBlackbodyKelvin(6504, 1.0f);
GfColorTest c;
c.SetFromPlanckianLocus(6504, 1.0f);
GfVec2f xy = c.GetChromaticity();
TF_AXIOM(GfIsClose(xy, wpD65xy, 1e-2f));
}
// test that primaries correspond to unit vectors in their color space
{
GfColor c1(csAp0);
GfColorTest c1(csAp0);
c1.SetFromChromaticity(ap0Primaries[0]);
GfColor c2(csAp0);
GfColorTest c2(csAp0);
c2.SetFromChromaticity(ap0Primaries[1]);
GfColor c3(csAp0);
GfColorTest c3(csAp0);
c3.SetFromChromaticity(ap0Primaries[2]);
TF_AXIOM(GfIsClose(c1, GfColor(GfVec3f(1, 0, 0), csAp0), 1e-5f));
TF_AXIOM(GfIsClose(c2, GfColor(GfVec3f(0, 1, 0), csAp0), 1e-5f));
TF_AXIOM(GfIsClose(c3, GfColor(GfVec3f(0, 0, 1), csAp0), 1e-5f));
TF_AXIOM(GfIsClose(c1, GfColorTest(GfVec3f(1, 0, 0), csAp0), 1e-5f));
TF_AXIOM(GfIsClose(c2, GfColorTest(GfVec3f(0, 1, 0), csAp0), 1e-5f));
TF_AXIOM(GfIsClose(c3, GfColorTest(GfVec3f(0, 0, 1), csAp0), 1e-5f));

GfColor c4(csLinearRec2020);
GfColorTest c4(csLinearRec2020);
c4.SetFromChromaticity(rec2020Primaries[0]);
GfColor c5(csLinearRec2020);
GfColorTest c5(csLinearRec2020);
c5.SetFromChromaticity(rec2020Primaries[1]);
GfColor c6(csLinearRec2020);
GfColorTest c6(csLinearRec2020);
c6.SetFromChromaticity(rec2020Primaries[2]);
TF_AXIOM(GfIsClose(c4, GfColor(GfVec3f(1, 0, 0), csLinearRec2020), 1e-5f));
TF_AXIOM(GfIsClose(c5, GfColor(GfVec3f(0, 1, 0), csLinearRec2020), 1e-5f));
TF_AXIOM(GfIsClose(c6, GfColor(GfVec3f(0, 0, 1), csLinearRec2020), 1e-5f));
TF_AXIOM(GfIsClose(c4, GfColorTest(GfVec3f(1, 0, 0), csLinearRec2020), 1e-5f));
TF_AXIOM(GfIsClose(c5, GfColorTest(GfVec3f(0, 1, 0), csLinearRec2020), 1e-5f));
TF_AXIOM(GfIsClose(c6, GfColorTest(GfVec3f(0, 0, 1), csLinearRec2020), 1e-5f));

GfColor c7(csLinearRec709);
GfColorTest c7(csLinearRec709);
c7.SetFromChromaticity(rec709Primaries[0]);
GfColor c8(csLinearRec709);
GfColorTest c8(csLinearRec709);
c8.SetFromChromaticity(rec709Primaries[1]);
GfColor c9(csLinearRec709);
GfColorTest c9(csLinearRec709);
c9.SetFromChromaticity(rec709Primaries[2]);
TF_AXIOM(GfIsClose(c7, GfColor(GfVec3f(1, 0, 0), csLinearRec709), 1e-5f));
TF_AXIOM(GfIsClose(c8, GfColor(GfVec3f(0, 1, 0), csLinearRec709), 1e-5f));
TF_AXIOM(GfIsClose(c9, GfColor(GfVec3f(0, 0, 1), csLinearRec709), 1e-5f));
TF_AXIOM(GfIsClose(c7, GfColorTest(GfVec3f(1, 0, 0), csLinearRec709), 1e-5f));
TF_AXIOM(GfIsClose(c8, GfColorTest(GfVec3f(0, 1, 0), csLinearRec709), 1e-5f));
TF_AXIOM(GfIsClose(c9, GfColorTest(GfVec3f(0, 0, 1), csLinearRec709), 1e-5f));
}

// permute the rec709 primaries through rec2020 and ap0
Expand All @@ -239,15 +272,15 @@ main(int argc, char *argv[])
// space's primaries
{
// Create converted colors
GfColor red709(GfVec3f(1.0f, 0.0f, 0.0f), csLinearRec709);
GfColor green709(GfVec3f(0.0f, 1.0f, 0.0f), csLinearRec709);
GfColor blue709(GfVec3f(0.0f, 0.0f, 1.0f), csLinearRec709);
GfColor red2020(GfVec3f(1.0f, 0.0f, 0.0f), csLinearRec2020);
GfColor green2020(GfVec3f(0.0f, 1.0f, 0.0f), csLinearRec2020);
GfColor blue2020(GfVec3f(0.0f, 0.0f, 1.0f), csLinearRec2020);
GfColor redAp0(GfVec3f(1.0f, 0.0f, 0.0f), csAp0);
GfColor greenAp0(GfVec3f(0.0f, 1.0f, 0.0f), csAp0);
GfColor blueAp0(GfVec3f(0.0f, 0.0f, 1.0f), csAp0);
GfColorTest red709(GfVec3f(1.0f, 0.0f, 0.0f), csLinearRec709);
GfColorTest green709(GfVec3f(0.0f, 1.0f, 0.0f), csLinearRec709);
GfColorTest blue709(GfVec3f(0.0f, 0.0f, 1.0f), csLinearRec709);
GfColorTest red2020(GfVec3f(1.0f, 0.0f, 0.0f), csLinearRec2020);
GfColorTest green2020(GfVec3f(0.0f, 1.0f, 0.0f), csLinearRec2020);
GfColorTest blue2020(GfVec3f(0.0f, 0.0f, 1.0f), csLinearRec2020);
GfColorTest redAp0(GfVec3f(1.0f, 0.0f, 0.0f), csAp0);
GfColorTest greenAp0(GfVec3f(0.0f, 1.0f, 0.0f), csAp0);
GfColorTest blueAp0(GfVec3f(0.0f, 0.0f, 1.0f), csAp0);

// Verify that converted 709 colors are within rec2020 gamut
TF_AXIOM(PointInTriangle(red709.GetChromaticity(),
Expand Down Expand Up @@ -316,8 +349,8 @@ main(int argc, char *argv[])
// values between 1000 and 2000, they are slightly divergent from
// canonical values.
for (int kelvin = 1000; kelvin <= 15000; kelvin += 1000) {
GfColor c(csIdentity);
c.SetFromBlackbodyKelvin(kelvin, 1.0f);
GfColorTest c(csIdentity);
c.SetFromPlanckianLocus(kelvin, 1.0f);
GfVec2f xy = c.GetChromaticity();
int index = (kelvin - 1000) / 1000;
GfVec2f known = tableOfKnownValues[index];
Expand Down

0 comments on commit a636a70

Please sign in to comment.