Skip to content

Commit 80fe4c1

Browse files
llvm-beanzllvm-beanz
authored
[NFC] Refactor and cleanup of color handling (#21)
This updates the Image encoder to use the same function for color conversion that will be used elsewhere and is included in the Color header.
1 parent 113bde8 commit 80fe4c1

File tree

4 files changed

+92
-76
lines changed

4 files changed

+92
-76
lines changed

‎include/Image/Color.h

Lines changed: 63 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,36 +12,86 @@
1212
#ifndef HLSLTEST_IMAGE_COLOR_H
1313
#define HLSLTEST_IMAGE_COLOR_H
1414

15+
#include <algorithm>
16+
#include <tuple>
17+
#include <type_traits>
18+
1519
namespace hlsltest {
1620

17-
class Color {
21+
namespace ColorUtils {
22+
template <typename T>
23+
std::enable_if_t<std::is_integral_v<T>, double> toInt(double Val) {
24+
constexpr double Max = static_cast<double>(std::numeric_limits<T>::max());
25+
constexpr double Base = Max + 1.0;
26+
double Conv = std::clamp(floor(Val * Base), 0.0, Max);
27+
return static_cast<T>(Conv);
28+
}
29+
30+
template <typename T>
31+
std::enable_if_t<std::is_arithmetic_v<T>, double> toDouble(T Val) {
32+
if constexpr (std::is_floating_point_v<T>)
33+
return Val;
34+
return static_cast<double>(Val) /
35+
static_cast<double>(std::numeric_limits<T>::max());
36+
}
37+
38+
template <typename NewTy, typename OldTy> NewTy convertColor(OldTy Val) {
39+
if constexpr (std::is_same_v<NewTy, OldTy>)
40+
return Val;
41+
double Dbl = toDouble(Val);
42+
if constexpr (std::is_integral_v<NewTy>)
43+
return toInt<NewTy>(Dbl);
44+
return static_cast<NewTy>(Dbl);
45+
}
46+
} // namespace ColorUtils
47+
48+
enum class ColorSpace { RGB, XYZ, LAB };
49+
50+
template <typename T> class ColorBase {
1851
public:
19-
enum Space {
20-
RGB,
21-
XYZ,
22-
LAB
23-
};
52+
T R, G, B;
2453

25-
double R, G, B;
54+
ColorSpace Space;
2655

27-
Space ColorSpace;
56+
constexpr ColorBase(T Rx, T Gy, T Bz, ColorSpace CS = ColorSpace::RGB)
57+
: R(Rx), G(Gy), B(Bz), Space(CS) {}
58+
ColorBase() = delete;
59+
ColorBase(const ColorBase &) = default;
60+
ColorBase(ColorBase &&) = default;
61+
ColorBase &operator=(const ColorBase &) = default;
62+
ColorBase &operator=(ColorBase &&) = default;
2863

29-
constexpr Color(double Rx, double Gy, double Bz, Space CS = RGB)
30-
: R(Rx), G(Gy), B(Bz), ColorSpace(CS) {}
64+
template <typename NewTy> ColorBase<NewTy> getAs() {
65+
ColorBase<NewTy> Val = {ColorUtils::convertColor<NewTy>(R),
66+
ColorUtils::convertColor<NewTy>(G),
67+
ColorUtils::convertColor<NewTy>(B), Space};
68+
return Val;
69+
}
70+
71+
bool operator==(const ColorBase &RHS) const {
72+
return std::tie(R, G, B, Space) == std::tie(RHS.R, RHS.G, RHS.B, RHS.Space);
73+
}
74+
};
75+
76+
class Color : public ColorBase<double> {
77+
public:
78+
constexpr Color(double Rx, double Gy, double Bz,
79+
ColorSpace CS = ColorSpace::RGB)
80+
: ColorBase(Rx, Gy, Bz, CS) {}
3181
Color() = delete;
3282
Color(const Color &) = default;
3383
Color(Color &&) = default;
3484
Color &operator=(const Color &) = default;
3585
Color &operator=(Color &&) = default;
3686

37-
Color translateSpace(Space NewCS) {
38-
if (NewCS == ColorSpace)
87+
Color translateSpace(ColorSpace NewCS) {
88+
if (NewCS == Space)
3989
return *this;
4090
return translateSpaceImpl(NewCS);
4191
}
4292

4393
private:
44-
Color translateSpaceImpl(Space NewCS);
94+
Color translateSpaceImpl(ColorSpace NewCS);
4595
};
4696

4797
} // namespace hlsltest

‎lib/Image/Color.cpp

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@
1515

1616
using namespace hlsltest;
1717

18-
constexpr Color D65WhitePoint = Color(95.047, 100.000, 108.883, Color::XYZ);
18+
constexpr Color D65WhitePoint =
19+
Color(95.047, 100.000, 108.883, ColorSpace::XYZ);
1920

20-
static Color multiply(const Color LHS, double Mat[9], Color::Space NewSpace) {
21+
static Color multiply(const Color LHS, double Mat[9], ColorSpace NewSpace) {
2122
double X, Y, Z;
2223
X = (LHS.R * Mat[0]) + (LHS.G * Mat[1]) + (LHS.B * Mat[2]);
2324
Y = (LHS.R * Mat[3]) + (LHS.G * Mat[4]) + (LHS.B * Mat[5]);
@@ -30,15 +31,15 @@ static Color RGBToXYZ(const Color Old) {
3031
// Source: http://brucelindbloom.com/index.html?Eqn_RGB_XYZ_Matrix.html
3132
double Mat[] = {0.4124564, 0.3575761, 0.1804375, 0.2126729, 0.7151522,
3233
0.0721750, 0.0193339, 0.1191920, 0.9503041};
33-
return multiply(Old, Mat, Color::XYZ);
34+
return multiply(Old, Mat, ColorSpace::XYZ);
3435
}
3536

3637
static Color XYZToRGB(const Color Old) {
3738
// Matrix assumes D65 white point.
3839
// Source: http://brucelindbloom.com/index.html?Eqn_RGB_XYZ_Matrix.html
3940
double Mat[] = {3.2404542, -1.5371385, -0.4985314, -0.9692660, 1.8760108,
4041
0.0415560, 0.0556434, -0.2040259, 1.0572252};
41-
return multiply(Old, Mat, Color::RGB);
42+
return multiply(Old, Mat, ColorSpace::RGB);
4243
}
4344

4445
static double convertXYZ(double Val) {
@@ -55,7 +56,7 @@ static Color XYZToLAB(const Color Old) {
5556
double L = fmax(0.0, 116.0 * Y - 16.0);
5657
double A = 500 * (X - Y);
5758
double B = 200 * (Y - Z);
58-
return Color(L, A, B, Color::LAB);
59+
return Color(L, A, B, ColorSpace::LAB);
5960
}
6061

6162
static double convertLAB(double Val) {
@@ -77,19 +78,21 @@ static Color LABToXYZ(const Color Old) {
7778
Y = convertLAB(Y) * D65WhitePoint.G;
7879
Z = convertLAB(Z) * D65WhitePoint.B;
7980

80-
return Color(X, Y, Z, Color::XYZ);
81+
return Color(X, Y, Z, ColorSpace::XYZ);
8182
}
8283

83-
Color Color::translateSpaceImpl(Space NewCS) {
84+
Color Color::translateSpaceImpl(ColorSpace NewCS) {
85+
if (Space == NewCS)
86+
return *this;
8487
Color Tmp = *this;
85-
if (ColorSpace == Color::RGB)
88+
if (Space == ColorSpace::RGB)
8689
Tmp = RGBToXYZ(*this);
87-
else if (ColorSpace == Color::LAB)
90+
else if (Space == ColorSpace::LAB)
8891
Tmp = LABToXYZ(*this);
8992
// Tmp is now in XYZ space.
90-
if (NewCS == Color::RGB)
93+
if (NewCS == ColorSpace::RGB)
9194
return XYZToRGB(Tmp);
92-
if (NewCS == Color::LAB)
95+
if (NewCS == ColorSpace::LAB)
9396
return XYZToLAB(Tmp);
9497
return Tmp;
9598
}

‎lib/Image/Image.cpp

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//===----------------------------------------------------------------------===//
1111

1212
#include "Image/Image.h"
13+
#include "Image/Color.h"
1314

1415
#include "llvm/ADT/ScopeExit.h"
1516
#include "llvm/ADT/StringRef.h"
@@ -30,15 +31,8 @@ void TranslatePixelData(Image &Dst, ImageRef Src) {
3031
const SrcType *SrcPtr = reinterpret_cast<const SrcType *>(Src.data());
3132
for (uint64_t I = 0; I < Pixels; ++I) {
3233
for (uint32_t J = 0; J < CopiedChannels; ++J, ++SrcPtr, ++DstPtr) {
33-
double Tmp = static_cast<double>(*SrcPtr);
34-
// If the source type is not a float, normalize it between 0 & 1.
35-
if constexpr (!std::is_floating_point<SrcType>())
36-
Tmp /= std::numeric_limits<SrcType>::max();
37-
// If the destination type is not a float, scale it into the integer type.
38-
if constexpr (!std::is_floating_point<DstType>())
39-
Tmp *= std::numeric_limits<DstType>::max();
40-
41-
*DstPtr = static_cast<DstType>(Tmp);
34+
*DstPtr = ColorUtils::convertColor<DstType>(*SrcPtr);
35+
4236
if constexpr (sizeof(DstType) > 1 && !llvm::sys::IsBigEndianHost)
4337
llvm::sys::swapByteOrder(*DstPtr);
4438
}

‎unittests/Image/ColorTests.cpp

Lines changed: 12 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -19,65 +19,34 @@
1919

2020
using namespace hlsltest;
2121

22-
template <typename IntegerTy, typename FloatTy>
23-
IntegerTy NormalizeToInteger(FloatTy &Val) {
24-
constexpr FloatTy Base =
25-
static_cast<FloatTy>(std::numeric_limits<IntegerTy>::max()) + 1.0;
26-
FloatTy Conv = std::clamp(floor(Val * Base), static_cast<FloatTy>(0.0),
27-
static_cast<FloatTy>(255.0));
28-
return static_cast<IntegerTy>(Conv);
29-
}
30-
3122
TEST(ColorTests, RoundTrip) {
3223
{
3324
Color RGB = Color(1.0, 0.5, 0.0);
34-
Color XYZ = RGB.translateSpace(Color::XYZ);
35-
Color LAB = RGB.translateSpace(Color::LAB);
25+
Color XYZ = RGB.translateSpace(ColorSpace::XYZ);
26+
Color LAB = RGB.translateSpace(ColorSpace::LAB);
3627

37-
Color XYZPrime = LAB.translateSpace(Color::XYZ);
38-
Color RGBPrime = LAB.translateSpace(Color::RGB);
28+
Color XYZPrime = LAB.translateSpace(ColorSpace::XYZ);
29+
Color RGBPrime = LAB.translateSpace(ColorSpace::RGB);
3930

4031
// The floating point rounding errors across these transformations are
4132
// significant, so rather than comparing the float values we normalize them
4233
// back into integer color values to compare.
43-
EXPECT_EQ(NormalizeToInteger<uint16_t>(XYZ.R),
44-
NormalizeToInteger<uint16_t>(XYZPrime.R));
45-
EXPECT_EQ(NormalizeToInteger<uint16_t>(XYZ.G),
46-
NormalizeToInteger<uint16_t>(XYZPrime.G));
47-
EXPECT_EQ(NormalizeToInteger<uint16_t>(XYZ.B),
48-
NormalizeToInteger<uint16_t>(XYZPrime.B));
49-
50-
EXPECT_EQ(NormalizeToInteger<uint16_t>(RGB.R),
51-
NormalizeToInteger<uint16_t>(RGBPrime.R));
52-
EXPECT_EQ(NormalizeToInteger<uint16_t>(RGB.G),
53-
NormalizeToInteger<uint16_t>(RGBPrime.G));
54-
EXPECT_EQ(NormalizeToInteger<uint16_t>(RGB.B),
55-
NormalizeToInteger<uint16_t>(RGBPrime.B));
34+
EXPECT_EQ(XYZ.getAs<uint16_t>(), XYZPrime.getAs<uint16_t>());
35+
EXPECT_EQ(RGB.getAs<uint16_t>(), RGBPrime.getAs<uint16_t>());
5636
}
5737

5838
{
5939
Color RGB = Color(0.125, 0.896, 0.652);
60-
Color XYZ = RGB.translateSpace(Color::XYZ);
61-
Color LAB = RGB.translateSpace(Color::LAB);
40+
Color XYZ = RGB.translateSpace(ColorSpace::XYZ);
41+
Color LAB = RGB.translateSpace(ColorSpace::LAB);
6242

63-
Color XYZPrime = LAB.translateSpace(Color::XYZ);
64-
Color RGBPrime = LAB.translateSpace(Color::RGB);
43+
Color XYZPrime = LAB.translateSpace(ColorSpace::XYZ);
44+
Color RGBPrime = LAB.translateSpace(ColorSpace::RGB);
6545

6646
// The floating point rounding errors across these transformations are
6747
// significant, so rather than comparing the float values we normalize them
6848
// back into integer color values to compare.
69-
EXPECT_EQ(NormalizeToInteger<uint16_t>(XYZ.R),
70-
NormalizeToInteger<uint16_t>(XYZPrime.R));
71-
EXPECT_EQ(NormalizeToInteger<uint16_t>(XYZ.G),
72-
NormalizeToInteger<uint16_t>(XYZPrime.G));
73-
EXPECT_EQ(NormalizeToInteger<uint16_t>(XYZ.B),
74-
NormalizeToInteger<uint16_t>(XYZPrime.B));
75-
76-
EXPECT_EQ(NormalizeToInteger<uint16_t>(RGB.R),
77-
NormalizeToInteger<uint16_t>(RGBPrime.R));
78-
EXPECT_EQ(NormalizeToInteger<uint16_t>(RGB.G),
79-
NormalizeToInteger<uint16_t>(RGBPrime.G));
80-
EXPECT_EQ(NormalizeToInteger<uint16_t>(RGB.B),
81-
NormalizeToInteger<uint16_t>(RGBPrime.B));
49+
EXPECT_EQ(XYZ.getAs<uint16_t>(), XYZPrime.getAs<uint16_t>());
50+
EXPECT_EQ(RGB.getAs<uint16_t>(), RGBPrime.getAs<uint16_t>());
8251
}
8352
}

0 commit comments

Comments
 (0)