Skip to content

Commit 4a6c058

Browse files
KevinJWKevinJW
committed
Extend tables to have an additonal entry to allow for boundary condition to not over flow table bounds during lookup
All tables are now 1 entry larger and should have valid data wrapped into the extra entries Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com>
1 parent 8b274ae commit 4a6c058

File tree

3 files changed

+26
-18
lines changed

3 files changed

+26
-18
lines changed

‎src/OpenColorIO/ops/fixedfunction/ACES2/Common.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,11 @@ inline float from_radians(const float v) { return wrap_to_hue_limit(v); };
4747

4848
struct TableBase
4949
{
50-
static constexpr unsigned int _TABLE_ADDITION_ENTRIES = 2;
51-
static constexpr unsigned int base_index = 1;
50+
static constexpr unsigned int _TABLE_ADDITION_LOWER_ENTRIES = 1;
51+
static constexpr unsigned int _TABLE_ADDITION_UPPER_ENTRIES = 2;
52+
static constexpr unsigned int base_index = _TABLE_ADDITION_LOWER_ENTRIES;
5253
static constexpr unsigned int nominal_size = 360;
53-
static constexpr unsigned int total_size = nominal_size + _TABLE_ADDITION_ENTRIES;
54+
static constexpr unsigned int total_size = nominal_size + _TABLE_ADDITION_LOWER_ENTRIES + _TABLE_ADDITION_UPPER_ENTRIES;
5455

5556
static constexpr unsigned int lower_wrap_index = 0;
5657
static constexpr unsigned int upper_wrap_index = base_index + nominal_size;

‎src/OpenColorIO/ops/fixedfunction/ACES2/Transform.cpp‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -730,8 +730,9 @@ void build_hue_table(Table1D &hue_table, const std::array<float, max_sorted_corn
730730
// BUG: could break if we are unlucky with samples all being used up by this point
731731
build_hue_sample_interval(hue_table.nominal_size - total_samples, sorted_hues[i - 1], hue_limit, hue_table, total_samples + 1);
732732

733-
hue_table[hue_table.lower_wrap_index] = hue_table[hue_table.last_nominal_index] - hue_limit;
734-
hue_table[hue_table.upper_wrap_index] = hue_table[hue_table.first_nominal_index] + hue_limit;
733+
hue_table[hue_table.lower_wrap_index] = hue_table[hue_table.last_nominal_index] - hue_limit;
734+
hue_table[hue_table.upper_wrap_index] = hue_table[hue_table.first_nominal_index] + hue_limit;
735+
hue_table[hue_table.upper_wrap_index + 1] = hue_table[hue_table.first_nominal_index + 1] + hue_limit;
735736
}
736737

737738
std::array<float, 2> find_display_cusp_for_hue(float hue, const std::array<f3, totalCornerCount>& RGB_corners, const std::array<f3, totalCornerCount>& JMh_corners,
@@ -820,12 +821,15 @@ Table3D build_cusp_table(const Table1D& hue_table, const std::array<f3, totalCor
820821
}
821822

822823
// Copy extra entries to ease the code to handle hues wrapping around
823-
output_table[output_table.lower_wrap_index][0] = output_table[output_table.last_nominal_index][0];
824-
output_table[output_table.lower_wrap_index][1] = output_table[output_table.last_nominal_index][1];
825-
output_table[output_table.lower_wrap_index][2] = hue_table[hue_table.lower_wrap_index];
826-
output_table[output_table.upper_wrap_index][0] = output_table[output_table.first_nominal_index][0];
827-
output_table[output_table.upper_wrap_index][1] = output_table[output_table.first_nominal_index][1];
828-
output_table[output_table.upper_wrap_index][2] = hue_table[hue_table.upper_wrap_index];
824+
output_table[output_table.lower_wrap_index][0] = output_table[output_table.last_nominal_index][0];
825+
output_table[output_table.lower_wrap_index][1] = output_table[output_table.last_nominal_index][1];
826+
output_table[output_table.lower_wrap_index][2] = hue_table[hue_table.lower_wrap_index];
827+
output_table[output_table.upper_wrap_index][0] = output_table[output_table.first_nominal_index][0];
828+
output_table[output_table.upper_wrap_index][1] = output_table[output_table.first_nominal_index][1];
829+
output_table[output_table.upper_wrap_index][2] = hue_table[hue_table.upper_wrap_index];
830+
output_table[output_table.upper_wrap_index + 1][0] = output_table[output_table.first_nominal_index + 1][0];
831+
output_table[output_table.upper_wrap_index + 1][1] = output_table[output_table.first_nominal_index + 1][1];
832+
output_table[output_table.upper_wrap_index + 1][2] = hue_table[hue_table.upper_wrap_index + 1];
829833
return output_table;
830834
}
831835

@@ -898,8 +902,9 @@ Table1D make_reach_m_table(const JMhParams &params, const float limit_J_max)
898902

899903
gamutReachTable[i + gamutReachTable.base_index] = high;
900904
}
901-
gamutReachTable[gamutReachTable.lower_wrap_index] = gamutReachTable[gamutReachTable.last_nominal_index];
902-
gamutReachTable[gamutReachTable.upper_wrap_index] = gamutReachTable[gamutReachTable.first_nominal_index];
905+
gamutReachTable[gamutReachTable.lower_wrap_index] = gamutReachTable[gamutReachTable.last_nominal_index];
906+
gamutReachTable[gamutReachTable.upper_wrap_index] = gamutReachTable[gamutReachTable.first_nominal_index];
907+
gamutReachTable[gamutReachTable.upper_wrap_index + 1] = gamutReachTable[gamutReachTable.first_nominal_index + 1];
903908

904909
return gamutReachTable;
905910
}
@@ -1252,8 +1257,9 @@ void make_upper_hull_gamma(
12521257
}
12531258

12541259
// Copy last populated entries to empty spot 'wrapping' entries
1255-
gamutCuspTable[gamutCuspTable.lower_wrap_index][2] = gamutCuspTable[gamutCuspTable.last_nominal_index][2];
1256-
gamutCuspTable[gamutCuspTable.upper_wrap_index][2] = gamutCuspTable[gamutCuspTable.first_nominal_index][2];
1260+
gamutCuspTable[gamutCuspTable.lower_wrap_index][2] = gamutCuspTable[gamutCuspTable.last_nominal_index][2];
1261+
gamutCuspTable[gamutCuspTable.upper_wrap_index][2] = gamutCuspTable[gamutCuspTable.first_nominal_index][2];
1262+
gamutCuspTable[gamutCuspTable.upper_wrap_index + 1][2] = gamutCuspTable[gamutCuspTable.first_nominal_index + 1][2];
12571263
}
12581264

12591265
// Tonescale pre-calculations

‎tests/cpu/ops/fixedfunction/FixedFunctionOpCPU_tests.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -592,11 +592,12 @@ OCIO_ADD_TEST(FixedFunctionOpCPU, aces_ot_20_edge_cases)
592592
constexpr int num_channels = 4;
593593
std::array<float, test_cases * num_channels> input_32f = {
594594
0.0f, 0.0f, 0.0f, 1.0f,
595-
0.742242277f, 0.0931933373f, 0.321542144f, 1.0f // Bug 2220: related to hue angle calculation not wrapping
595+
0.742242277f, 0.0931933373f, 0.321542144f, 1.0f // Bug #2220: related to hue angle calculation not wrapping
596596
};
597597
constexpr std::array<float, test_cases * num_channels> expected_32f = {
598598
0.0f, 0.0f, 0.0f, 1.0f,
599-
0.0f, 0.0f, 0.0f, 1.0f,
599+
0.74736571311951f, -0.0019352473318577f, 0.19451357424259, 1.0f, // Note: exact output value is not significant to
600+
// the test as the bug is in the internal logic
600601
};
601602

602603
OCIO::FixedFunctionOpData::Params params = {
@@ -610,7 +611,7 @@ OCIO_ADD_TEST(FixedFunctionOpCPU, aces_ot_20_edge_cases)
610611
= std::make_shared<OCIO::FixedFunctionOpData>(OCIO::FixedFunctionOpData::ACES_OUTPUT_TRANSFORM_20_FWD,
611612
params);
612613

613-
ApplyFixedFunction(input_32f.data(), expected_32f.data(), test_cases, funcData, 1e-6f, __LINE__);
614+
ApplyFixedFunction(input_32f.data(), expected_32f.data(), test_cases, funcData, 1e-4f, __LINE__);
614615
}
615616

616617
OCIO_ADD_TEST(FixedFunctionOpCPU, aces_ot_20_p3d65_100n_rt)

0 commit comments

Comments
 (0)