Skip to content

Commit 9dabe5b

Browse files
committed
- fix hsv_to_rgb conversion
- add COLOR_TEST_MODE option - use rgb color == for HSVColor value comparison, that now works, since both conversions are correct
1 parent d7c645d commit 9dabe5b

File tree

4 files changed

+193
-129
lines changed

4 files changed

+193
-129
lines changed

src/helper/color.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ namespace {
5353
// and modified and sped up, by optimizing it manually
5454
[[nodiscard]] HSVColor Color::to_hsv_color() const {
5555
using FloatType = double; //for more precision use "long double" here
56+
5657
constexpr auto max_d = static_cast<FloatType>(0xFF);
5758
const auto r_d = static_cast<FloatType>(r) / max_d;
5859
const auto g_d = static_cast<FloatType>(g) / max_d;

src/helper/color.hpp

Lines changed: 104 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55
#include "helper/utils.hpp"
66

77
#include <algorithm>
8+
#include <cmath>
89
#include <fmt/format.h>
910
#include <ostream>
1011
#include <string>
1112

12-
1313
struct Color;
1414

1515
struct HSVColor {
@@ -56,6 +56,50 @@ struct HSVColor {
5656
std::ostream& operator<<(std::ostream& os) const;
5757
};
5858

59+
namespace {
60+
//TODO: if everything (also libc++ ) supports c++23 , the std functions are constexpr, so we can use them
61+
template<typename T>
62+
constexpr T fmod_constexpr(T value, T divisor) {
63+
if (not utils::is_constant_evaluated()) {
64+
return std::fmod(value, divisor);
65+
}
66+
67+
return value - static_cast<T>(static_cast<u64>(value / divisor)) * divisor;
68+
}
69+
70+
template<typename T>
71+
constexpr T fabs_constexpr(T value) {
72+
if (not utils::is_constant_evaluated()) {
73+
return std::fabs(value);
74+
}
75+
76+
if (value == static_cast<T>(-0.0)) {
77+
return static_cast<T>(0.0);
78+
}
79+
80+
if (value < static_cast<T>(0.0)) {
81+
return -value;
82+
}
83+
84+
return value;
85+
}
86+
87+
template<typename T>
88+
constexpr T round_constexpr(T value) {
89+
if (not utils::is_constant_evaluated()) {
90+
return std::round(value);
91+
}
92+
93+
T truncated = static_cast<T>(static_cast<i64>(value));
94+
95+
if (value - truncated >= 0.5) {
96+
truncated += static_cast<T>(1.0);
97+
}
98+
99+
return truncated;
100+
}
101+
} // namespace
102+
59103
struct Color {
60104
u8 r;
61105
u8 g;
@@ -79,82 +123,85 @@ struct Color {
79123

80124
constexpr Color(const HSVColor& color) {
81125

82-
const auto set_color = [&color, this](double r, double g, double b) {
83-
this->r = static_cast<u8>(std::clamp(r, 0.0, 1.0) * static_cast<double>(0xFF));
84-
this->g = static_cast<u8>(std::clamp(g, 0.0, 1.0) * static_cast<double>(0xFF));
85-
this->b = static_cast<u8>(std::clamp(b, 0.0, 1.0) * static_cast<double>(0xFF));
86-
this->a = color.a;
87-
};
126+
using FloatType = double; //for more precision use "long double" here
88127

89-
// taken from https://stackoverflow.com/questions/3018313/algorithm-to-convert-rgb-to-hsv-and-hsv-to-rgb-in-range-0-255-for-both
90-
double hh{};
91-
double p{};
92-
double q{};
93-
double t{};
94-
double ff{};
95-
96-
long i{};
97-
98-
double d_r{};
99-
double d_g{};
100-
double d_b{};
101-
102-
if (color.s <= 0.0) { // < is bogus, just shuts up warnings
103-
d_r = color.v;
104-
d_g = color.v;
105-
d_b = color.v;
106-
set_color(d_r, d_g, d_b);
107-
return;
108-
}
128+
// taken from https://scratch.mit.edu/discuss/topic/694772/
129+
130+
const auto h = static_cast<FloatType>(color.h);
131+
const auto s = static_cast<FloatType>(color.s);
132+
const auto v = static_cast<FloatType>(color.v);
109133

110-
hh = color.h;
111-
if (hh >= 360.0) {
112-
hh = 0.0;
134+
FloatType chroma = v * s;
135+
136+
FloatType hue = h;
137+
if (hue >= static_cast<FloatType>(360.0)) {
138+
hue = static_cast<FloatType>(0.0);
113139
}
114140

115-
hh /= 60.0;
116-
i = static_cast<long>(hh);
117-
ff = hh - static_cast<double>(i);
118141

119-
p = color.v * (1.0 - color.s);
120-
q = color.v * (1.0 - (color.s * ff));
121-
t = color.v * (1.0 - (color.s * (1.0 - ff)));
142+
FloatType x = chroma
143+
* (static_cast<FloatType>(1.0)
144+
- fabs_constexpr(
145+
fmod_constexpr(hue / static_cast<FloatType>(60.0), static_cast<FloatType>(2.0))
146+
- static_cast<FloatType>(1.0)
147+
));
148+
149+
u64 index = static_cast<u64>(hue / static_cast<FloatType>(60.0));
122150

123-
switch (i) {
151+
FloatType d_r{};
152+
FloatType d_g{};
153+
FloatType d_b{};
154+
155+
switch (index) {
124156
case 0:
125-
d_r = color.v;
126-
d_g = t;
127-
d_b = p;
157+
d_r = chroma;
158+
d_g = x;
159+
d_b = static_cast<FloatType>(0.0);
128160
break;
129161
case 1:
130-
d_r = q;
131-
d_g = color.v;
132-
d_b = p;
162+
d_r = x;
163+
d_g = chroma;
164+
d_b = static_cast<FloatType>(0.0);
133165
break;
134166
case 2:
135-
d_r = p;
136-
d_g = color.v;
137-
d_b = t;
167+
d_r = static_cast<FloatType>(0.0);
168+
d_g = chroma;
169+
d_b = x;
138170
break;
139171
case 3:
140-
d_r = p;
141-
d_g = q;
142-
d_b = color.v;
172+
d_r = static_cast<FloatType>(0.0);
173+
d_g = x;
174+
d_b = chroma;
143175
break;
144176
case 4:
145-
d_r = t;
146-
d_g = p;
147-
d_b = color.v;
177+
d_r = x;
178+
d_g = static_cast<FloatType>(0.0);
179+
d_b = chroma;
148180
break;
149181
case 5:
150-
default:
151-
d_r = color.v;
152-
d_g = p;
153-
d_b = q;
182+
d_r = chroma;
183+
d_g = static_cast<FloatType>(0.0);
184+
d_b = x;
154185
break;
186+
default:
187+
utils::unreachable();
155188
}
156189

157-
set_color(d_r, d_g, d_b);
190+
191+
FloatType m = v - chroma;
192+
193+
const auto finish_value = [m](FloatType value) -> u8 {
194+
const auto result =
195+
std::clamp<FloatType>(value + m, static_cast<FloatType>(0.0), static_cast<FloatType>(1.0))
196+
* static_cast<FloatType>(0xFF);
197+
198+
return static_cast<u8>(round_constexpr(result));
199+
};
200+
201+
this->r = finish_value(d_r);
202+
this->g = finish_value(d_g);
203+
this->b = finish_value(d_b);
204+
this->a = color.a;
158205
}
159206

160207
[[nodiscard]] constexpr bool operator==(const Color& other) const {

0 commit comments

Comments
 (0)