Skip to content

Commit ad58a21

Browse files
robot-clickhouserobot-clickhouse
committed
Backport #83906 to 25.7: Fix colorSRGBToOKLCH/colorOKLCHToSRGB for mix of const and non-const args
1 parent 75239c1 commit ad58a21

File tree

7 files changed

+74
-50
lines changed

7 files changed

+74
-50
lines changed

‎src/Functions/ColorConversion.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55

66

77
namespace DB
8+
{
9+
10+
namespace ColorConversion
811
{
912
/// Conversion constants for conversion between OKLCH and sRGB
1013
///
@@ -44,3 +47,5 @@ namespace DB
4447
-1.2684380046, 2.6097574011, -0.3413193965,
4548
-0.0041960863, -0.7034186147, 1.7076147010};
4649
}
50+
51+
}

‎src/Functions/colorOKLCHToSRGB.cpp‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ class FunctionColorOKLCHToSRGB : public ITupleFunction
5858
const auto * tuple_type = checkAndGetDataType<DataTypeTuple>(first_arg);
5959
const auto & tuple_inner_types = tuple_type->getElements();
6060

61-
if (tuple_inner_types.size() != channels)
61+
if (tuple_inner_types.size() != ColorConversion::channels)
6262
throw Exception(
6363
ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT,
6464
"First argument of function {} must be a tuple of size {}, a tuple of size {} was provided",
65-
getName(), channels, tuple_inner_types.size());
65+
getName(), ColorConversion::channels, tuple_inner_types.size());
6666

6767
for (const auto & tuple_inner_type : tuple_inner_types)
6868
{
@@ -80,24 +80,28 @@ class FunctionColorOKLCHToSRGB : public ITupleFunction
8080
getName());
8181

8282
auto float64_type = std::make_shared<DataTypeFloat64>();
83-
return std::make_shared<DataTypeTuple>(DataTypes(channels, float64_type));
83+
return std::make_shared<DataTypeTuple>(DataTypes(ColorConversion::channels, float64_type));
8484
}
8585

8686
ColumnPtr executeImpl(const ColumnsWithTypeAndName & arguments, const DataTypePtr &, size_t input_rows_count) const override
8787
{
8888
auto float64_type = std::make_shared<DataTypeFloat64>();
89-
auto tuple_f64_type = std::make_shared<DataTypeTuple>(DataTypes(channels, float64_type));
89+
auto tuple_f64_type = std::make_shared<DataTypeTuple>(DataTypes(ColorConversion::channels, float64_type));
9090

9191
auto tuple_f64_arg = castColumn(arguments[0], tuple_f64_type);
9292
auto rgb_cols = getTupleElements(*tuple_f64_arg);
9393

9494
ColumnPtr gamma;
9595
if (arguments.size() == 2)
96-
gamma = castColumn(arguments[1], float64_type);
96+
gamma = castColumn(arguments[1], float64_type)->convertToFullColumnIfConst();
9797

98-
const auto & lightness_data = assert_cast<const ColumnFloat64 &>(*rgb_cols[0]).getData();
99-
const auto & chroma_data = assert_cast<const ColumnFloat64 &>(*rgb_cols[1]).getData();
100-
const auto & hue_data = assert_cast<const ColumnFloat64 &>(*rgb_cols[2]).getData();
98+
ColumnPtr lightness_column = rgb_cols[0]->convertToFullColumnIfConst();
99+
ColumnPtr chroma_column = rgb_cols[1]->convertToFullColumnIfConst();
100+
ColumnPtr hue_column = rgb_cols[2]->convertToFullColumnIfConst();
101+
102+
const auto & lightness_data = assert_cast<const ColumnFloat64 &>(*lightness_column).getData();
103+
const auto & chroma_data = assert_cast<const ColumnFloat64 &>(*chroma_column).getData();
104+
const auto & hue_data = assert_cast<const ColumnFloat64 &>(*hue_column).getData();
101105
const auto * gamma_data = gamma ? &assert_cast<const ColumnFloat64 &>(*gamma).getData() : nullptr;
102106

103107
auto col_red = ColumnFloat64::create();
@@ -114,9 +118,9 @@ class FunctionColorOKLCHToSRGB : public ITupleFunction
114118

115119
for (size_t row = 0; row < input_rows_count; ++row)
116120
{
117-
Color lch_data{lightness_data[row], chroma_data[row], hue_data[row]};
118-
Float64 gamma_cur = gamma_data ? (*gamma_data)[row] : default_gamma;
119-
Color res = convertOklchToSrgb(lch_data, gamma_cur);
121+
ColorConversion::Color lch_data{lightness_data[row], chroma_data[row], hue_data[row]};
122+
Float64 gamma_cur = gamma_data ? (*gamma_data)[row] : ColorConversion::default_gamma;
123+
ColorConversion::Color res = convertOklchToSrgb(lch_data, gamma_cur);
120124
red_data.push_back(res[0]);
121125
green_data.push_back(res[1]);
122126
blue_data.push_back(res[2]);
@@ -127,36 +131,36 @@ class FunctionColorOKLCHToSRGB : public ITupleFunction
127131

128132
private:
129133
/// OKLCH -> sRGB. Follows the step-by-step pipeline described in Ottosson’s article. See ColorConversion.h
130-
Color convertOklchToSrgb(const Color & oklch, Float64 gamma) const
134+
ColorConversion::Color convertOklchToSrgb(const ColorConversion::Color & oklch, Float64 gamma) const
131135
{
132136
Float64 chroma = oklch[1];
133-
Float64 hue_rad = oklch[2] * deg2rad;
137+
Float64 hue_rad = oklch[2] * ColorConversion::deg2rad;
134138

135-
Color oklab = oklch;
139+
ColorConversion::Color oklab = oklch;
136140

137141
oklab[1] = chroma * std::cos(hue_rad);
138142
oklab[2] = chroma * std::sin(hue_rad);
139143

140-
Color lms{};
141-
for (size_t i = 0; i < channels; ++i)
144+
ColorConversion::Color lms{};
145+
for (size_t i = 0; i < ColorConversion::channels; ++i)
142146
{
143-
for (size_t channel = 0; channel < channels; ++channel)
144-
lms[i] = std::fma(oklab[channel], oklab_to_lms_base[(3 * i) + channel], lms[i]);
147+
for (size_t channel = 0; channel < ColorConversion::channels; ++channel)
148+
lms[i] = std::fma(oklab[channel], ColorConversion::oklab_to_lms_base[(3 * i) + channel], lms[i]);
145149
lms[i] = lms[i] * lms[i] * lms[i];
146150
}
147151

148-
Color rgb{};
149-
for (size_t i = 0; i < channels; ++i)
152+
ColorConversion::Color rgb{};
153+
for (size_t i = 0; i < ColorConversion::channels; ++i)
150154
{
151-
for (size_t channel = 0; channel < channels; ++channel)
152-
rgb[i] = std::fma(lms[channel], lms_to_linear_base[(3 * i) + channel], rgb[i]);
155+
for (size_t channel = 0; channel < ColorConversion::channels; ++channel)
156+
rgb[i] = std::fma(lms[channel], ColorConversion::lms_to_linear_base[(3 * i) + channel], rgb[i]);
153157
}
154158

155159
if (gamma == 0)
156-
gamma = gamma_fallback;
160+
gamma = ColorConversion::gamma_fallback;
157161

158162
Float64 power = 1 / gamma;
159-
for (size_t i = 0; i < channels; ++i)
163+
for (size_t i = 0; i < ColorConversion::channels; ++i)
160164
{
161165
rgb[i] = std::clamp(rgb[i], 0.0, 1.0);
162166
rgb[i] = std::pow(rgb[i], power) * 255.0;

‎src/Functions/colorSRGBToOKLCH.cpp‎

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,11 @@ class FunctionColorSRGBToOKLCH : public ITupleFunction
5959
const auto * tuple_type = checkAndGetDataType<DataTypeTuple>(first_arg);
6060
const auto & tuple_inner_types = tuple_type->getElements();
6161

62-
if (tuple_inner_types.size() != channels)
62+
if (tuple_inner_types.size() != ColorConversion::channels)
6363
throw Exception(
6464
ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT,
6565
"First argument of function {} must be a tuple of size {}, a tuple of size {} was provided",
66-
getName(), channels, tuple_inner_types.size());
66+
getName(), ColorConversion::channels, tuple_inner_types.size());
6767

6868
for (const auto & tuple_inner_type : tuple_inner_types)
6969
{
@@ -81,24 +81,28 @@ class FunctionColorSRGBToOKLCH : public ITupleFunction
8181
getName());
8282

8383
auto float64_type = std::make_shared<DataTypeFloat64>();
84-
return std::make_shared<DataTypeTuple>(DataTypes(channels, float64_type));
84+
return std::make_shared<DataTypeTuple>(DataTypes(ColorConversion::channels, float64_type));
8585
}
8686

8787
ColumnPtr executeImpl(const ColumnsWithTypeAndName & arguments, const DataTypePtr &, size_t input_rows_count) const override
8888
{
8989
auto float64_type = std::make_shared<DataTypeFloat64>();
90-
auto tuple_f64_ptr = std::make_shared<DataTypeTuple>(DataTypes(channels, float64_type));
90+
auto tuple_f64_ptr = std::make_shared<DataTypeTuple>(DataTypes(ColorConversion::channels, float64_type));
9191

9292
auto tuple_f64_arg = castColumn(arguments[0], tuple_f64_ptr);
9393
auto rgb_cols = getTupleElements(*tuple_f64_arg);
9494

9595
ColumnPtr gamma;
9696
if (arguments.size() == 2)
97-
gamma = castColumn(arguments[1], float64_type);
97+
gamma = castColumn(arguments[1], float64_type)->convertToFullColumnIfConst();
9898

99-
const auto & red_data = assert_cast<const ColumnFloat64 &>(*rgb_cols[0]).getData();
100-
const auto & green_data = assert_cast<const ColumnFloat64 &>(*rgb_cols[1]).getData();
101-
const auto & blue_data = assert_cast<const ColumnFloat64 &>(*rgb_cols[2]).getData();
99+
ColumnPtr red_column = rgb_cols[0]->convertToFullColumnIfConst();
100+
ColumnPtr green_column = rgb_cols[1]->convertToFullColumnIfConst();
101+
ColumnPtr blue_column = rgb_cols[2]->convertToFullColumnIfConst();
102+
103+
const auto & red_data = assert_cast<const ColumnFloat64 &>(*red_column).getData();
104+
const auto & green_data = assert_cast<const ColumnFloat64 &>(*green_column).getData();
105+
const auto & blue_data = assert_cast<const ColumnFloat64 &>(*blue_column).getData();
102106
const auto * gamma_data = gamma ? &assert_cast<const ColumnFloat64 &>(*gamma).getData() : nullptr;
103107

104108
auto col_lightness = ColumnFloat64::create();
@@ -115,9 +119,9 @@ class FunctionColorSRGBToOKLCH : public ITupleFunction
115119

116120
for (size_t row = 0; row < input_rows_count; ++row)
117121
{
118-
Color rgb_data{red_data[row], green_data[row], blue_data[row]};
119-
Float64 gamma_cur = gamma_data ? (*gamma_data)[row] : default_gamma;
120-
Color res = convertSrgbToOklch(rgb_data, gamma_cur);
122+
ColorConversion::Color rgb_data{red_data[row], green_data[row], blue_data[row]};
123+
Float64 gamma_cur = gamma_data ? (*gamma_data)[row] : ColorConversion::default_gamma;
124+
ColorConversion::Color res = convertSrgbToOklch(rgb_data, gamma_cur);
121125
lightness_data.push_back(res[0]);
122126
chroma_data.push_back(res[1]);
123127
hue_data.push_back(res[2]);
@@ -128,36 +132,36 @@ class FunctionColorSRGBToOKLCH : public ITupleFunction
128132

129133
private:
130134
/// sRGB -> OKLCH. Follows the step-by-step pipeline described in Ottosson’s article, see ColorConversion.h
131-
Color convertSrgbToOklch(const Color & rgb, Float64 gamma) const
135+
ColorConversion::Color convertSrgbToOklch(const ColorConversion::Color & rgb, Float64 gamma) const
132136
{
133-
Color rgb_lin;
134-
for (size_t i = 0; i < channels; ++i)
137+
ColorConversion::Color rgb_lin;
138+
for (size_t i = 0; i < ColorConversion::channels; ++i)
135139
rgb_lin[i] = std::pow(rgb[i] / 255.0, gamma);
136140

137-
Color lms{};
138-
for (size_t i = 0; i < channels; ++i)
141+
ColorConversion::Color lms{};
142+
for (size_t i = 0; i < ColorConversion::channels; ++i)
139143
{
140-
for (size_t channel = 0; channel < channels; ++channel)
141-
lms[i] = std::fma(rgb_lin[channel], linear_to_lms_base[(3 * i) + channel], lms[i]);
144+
for (size_t channel = 0; channel < ColorConversion::channels; ++channel)
145+
lms[i] = std::fma(rgb_lin[channel], ColorConversion::linear_to_lms_base[(3 * i) + channel], lms[i]);
142146
lms[i] = std::cbrt(lms[i]);
143147
}
144148

145-
Color oklab{};
146-
for (size_t i = 0; i < channels; ++i)
149+
ColorConversion::Color oklab{};
150+
for (size_t i = 0; i < ColorConversion::channels; ++i)
147151
{
148-
for (size_t channel = 0; channel < channels; ++channel)
149-
oklab[i] = std::fma(lms[channel], lms_to_oklab_base[(3 * i) + channel], oklab[i]);
152+
for (size_t channel = 0; channel < ColorConversion::channels; ++channel)
153+
oklab[i] = std::fma(lms[channel], ColorConversion::lms_to_oklab_base[(3 * i) + channel], oklab[i]);
150154
}
151155

152-
Color oklch = oklab;
156+
ColorConversion::Color oklch = oklab;
153157

154158
Float64 a = oklab[1];
155159
Float64 b = oklab[2];
156160

157161
oklch[1] = std::sqrt(a * a + b * b);
158-
if (oklch[1] >= epsilon)
162+
if (oklch[1] >= ColorConversion::epsilon)
159163
{
160-
Float64 hue_degrees = std::atan2(b, a) * rad2deg;
164+
Float64 hue_degrees = std::atan2(b, a) * ColorConversion::rad2deg;
161165
oklch[2] = std::fmod(hue_degrees + 360.0, 360.0);
162166
}
163167
else

‎tests/queries/0_stateless/03561_colorSRGBToOKLCH.reference‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,5 @@
2121
(nan,0,0)
2222
(0,0,0)
2323
(0.350124,0.242612,264.052021)
24+
--- Fuzzing
25+
(0.794737,0,0)

‎tests/queries/0_stateless/03561_colorSRGBToOKLCH.sql‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,8 @@ SELECT tuple(round(t.1, 6), round(t.2, 6), round(t.3, 6));
4747
WITH colorSRGBToOKLCH((53, 134, 180), 1000) AS t
4848
SELECT tuple(round(t.1, 6), round(t.2, 6), round(t.3, 6));
4949
WITH colorSRGBToOKLCH((1e-3, 1e-6, 180)) AS t
50-
SELECT tuple(round(t.1, 6), round(t.2, 6), round(t.3, 6));
50+
SELECT tuple(round(t.1, 6), round(t.2, 6), round(t.3, 6));
51+
52+
SELECT '--- Fuzzing';
53+
WITH colorSRGBToOKLCH((128, 128, 128), materialize(1.)) AS t
54+
SELECT tuple(round(t.1, 6), round(t.2, 6), round(t.3, 6));

‎tests/queries/0_stateless/03562_colorOKLCHToSRGB.reference‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,5 @@
2020
(255.851847,255.365917,255.200142)
2121
(254.150989,254.634608,254.800015)
2222
(0,255,255)
23+
--- Fuzzing
24+
(0,255,255)

‎tests/queries/0_stateless/03562_colorOKLCHToSRGB.sql‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,6 @@ SELECT tuple(round(t.1, 6), round(t.2, 6), round(t.3, 6));
4848
WITH colorOKLCHToSRGB((1e3, 1e6, 180)) as t
4949
SELECT tuple(round(t.1, 6), round(t.2, 6), round(t.3, 6));
5050

51+
SELECT '--- Fuzzing';
52+
WITH colorOKLCHToSRGB((1e3, 1e6, 180), materialize(0.)) as t
53+
SELECT tuple(round(t.1, 6), round(t.2, 6), round(t.3, 6));

0 commit comments

Comments
 (0)