From fffa291765cf014996c404258e7cfdc26f161220 Mon Sep 17 00:00:00 2001 From: Andrew Cassidy Date: Tue, 24 May 2022 22:57:51 -0700 Subject: [PATCH] Fix LeastSquares mode and add tests for every quality level --- CHANGELOG.md | 7 +++ quicktex/__init__.py | 1 - quicktex/s3tc/bc1/BC1Encoder.cpp | 85 ++++++++++++++++++++++---------- tests/test_bc1.py | 15 +++--- tests/test_install.py | 3 +- 5 files changed, 76 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a5c23d..9e3f963 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ All notable changes to this project will be documented in this file +## Unreleased + +### Fixed + +- Fixed LeastSquares endpoint mode producint incorrect results + + ## 0.1.3 - 2022-04-13 ### Fixed diff --git a/quicktex/__init__.py b/quicktex/__init__.py index 87db137..8091bd6 100644 --- a/quicktex/__init__.py +++ b/quicktex/__init__.py @@ -1,7 +1,6 @@ try: from _quicktex import * from _quicktex import __version__ - from _quicktex import _debug_build except ImportError as e: if 'libomp.dylib' in e.msg: print('\033[41m\033[01mERROR: LIBOMP NOT FOUND! PLEASE INSTALL IT WITH \033[04m`brew install libomp`\033[0m') diff --git a/quicktex/s3tc/bc1/BC1Encoder.cpp b/quicktex/s3tc/bc1/BC1Encoder.cpp index 6735ed4..9c71a8f 100644 --- a/quicktex/s3tc/bc1/BC1Encoder.cpp +++ b/quicktex/s3tc/bc1/BC1Encoder.cpp @@ -33,11 +33,11 @@ #include "../../ColorBlock.h" #include "../../Matrix4x4.h" #include "../../Texture.h" +#include "../../VecUtil.h" #include "../../Vector4.h" #include "../../Vector4Int.h" #include "../../bitwiseEnums.h" #include "../../util.h" -#include "../../VecUtil.h" #include "Histogram.h" #include "OrderTable.h" #include "SingleColorTable.h" @@ -46,8 +46,10 @@ namespace quicktex::s3tc { // constructors -BC1Encoder::BC1Encoder(unsigned int level, ColorMode color_mode, InterpolatorPtr interpolator) : _interpolator(interpolator), _color_mode(color_mode) { - if (color_mode != ColorMode::FourColor && color_mode != ColorMode::ThreeColor && color_mode != ColorMode::ThreeColorBlack) { +BC1Encoder::BC1Encoder(unsigned int level, ColorMode color_mode, InterpolatorPtr interpolator) + : _interpolator(interpolator), _color_mode(color_mode) { + if (color_mode != ColorMode::FourColor && color_mode != ColorMode::ThreeColor && + color_mode != ColorMode::ThreeColorBlack) { throw std::invalid_argument("Encoder color mode must be FourColor, ThreeColor, or ThreeColorBlack"); } @@ -74,7 +76,9 @@ BC1Encoder::BC1Encoder(unsigned int level, ColorMode color_mode, InterpolatorPtr // Getters and Setters void BC1Encoder::SetLevel(unsigned level) { - if (level > 19) throw std::invalid_argument("Level out of range, bust be between 0 and 18 inclusive"); // theres a secret level 19 but shhhhhh + if (level > 19) + throw std::invalid_argument( + "Level out of range, bust be between 0 and 18 inclusive"); // theres a secret level 19 but shhhhhh two_ls_passes = false; two_ep_passes = false; @@ -250,14 +254,20 @@ void BC1Encoder::SetLevel(unsigned level) { _orderings3 = clamp(_orderings3, 1U, OrderTable<3>::BestOrderCount); } -void BC1Encoder::SetOrderings4(unsigned orderings4) { _orderings4 = clamp(orderings4, 1U, OrderTable<4>::BestOrderCount); } -void BC1Encoder::SetOrderings3(unsigned orderings3) { _orderings3 = clamp(orderings3, 1U, OrderTable<3>::BestOrderCount); } +void BC1Encoder::SetOrderings4(unsigned orderings4) { + _orderings4 = clamp(orderings4, 1U, OrderTable<4>::BestOrderCount); +} +void BC1Encoder::SetOrderings3(unsigned orderings3) { + _orderings3 = clamp(orderings3, 1U, OrderTable<3>::BestOrderCount); +} void BC1Encoder::SetOrderings(OrderingPair orderings) { SetOrderings4(std::get<0>(orderings)); SetOrderings3(std::get<1>(orderings)); } -void BC1Encoder::SetPowerIterations(unsigned int power_iters) { _power_iterations = clamp(power_iters, min_power_iterations, max_power_iterations); } +void BC1Encoder::SetPowerIterations(unsigned int power_iters) { + _power_iterations = clamp(power_iters, min_power_iterations, max_power_iterations); +} // Public methods BC1Block BC1Encoder::EncodeBlock(const ColorBlock<4, 4> &pixels) const { @@ -305,7 +315,9 @@ BC1Block BC1Encoder::EncodeBlock(const ColorBlock<4, 4> &pixels) const { // First refinement pass using ordered cluster fit if (result.error > 0 && use_likely_orderings) { - for (unsigned iter = 0; iter < total_cf_passes; iter++) { RefineBlockCF(result, pixels, metrics, _error_mode, _orderings4); } + for (unsigned iter = 0; iter < total_cf_passes; iter++) { + RefineBlockCF(result, pixels, metrics, _error_mode, _orderings4); + } } // try for 3-color block @@ -326,13 +338,15 @@ BC1Block BC1Encoder::EncodeBlock(const ColorBlock<4, 4> &pixels) const { } // try for 3-color block with black - if (result.error > 0 && (_color_mode == ColorMode::ThreeColorBlack) && metrics.has_black && !metrics.max.IsBlack()) { + if (result.error > 0 && (_color_mode == ColorMode::ThreeColorBlack) && metrics.has_black && + !metrics.max.IsBlack()) { EncodeResults trial_result; BlockMetrics metrics_no_black = pixels.GetMetrics(true); FindEndpoints(trial_result, pixels, metrics_no_black, EndpointMode::PCA, true); FindSelectors(trial_result, pixels, ErrorMode::Full); - RefineBlockLS(trial_result, pixels, metrics_no_black, ErrorMode::Full, total_ls_passes); + RefineBlockLS(trial_result, pixels, metrics_no_black, ErrorMode::Full, + total_ls_passes); if (trial_result.error < result.error) { result = trial_result; } } @@ -457,7 +471,8 @@ void BC1Encoder::FindEndpointsSingleColor(EncodeResults &result, Color color, bo // selectors decided when writing, no point deciding them now } -void BC1Encoder::FindEndpointsSingleColor(EncodeResults &result, const CBlock &pixels, Color color, bool is_3color) const { +void BC1Encoder::FindEndpointsSingleColor(EncodeResults &result, const CBlock &pixels, Color color, + bool is_3color) const { std::array colors = _interpolator->InterpolateBC1(result.low, result.high, is_3color); Vector4Int result_vector = (Vector4Int)colors[2]; @@ -472,7 +487,8 @@ void BC1Encoder::FindEndpointsSingleColor(EncodeResults &result, const CBlock &p } } -void BC1Encoder::FindEndpoints(EncodeResults &result, const CBlock &pixels, const BlockMetrics &metrics, EndpointMode endpoint_mode, bool ignore_black) const { +void BC1Encoder::FindEndpoints(EncodeResults &result, const CBlock &pixels, const BlockMetrics &metrics, + EndpointMode endpoint_mode, bool ignore_black) const { if (metrics.is_greyscale) { // specialized greyscale case const unsigned fr = pixels.Get(0, 0).r; @@ -502,10 +518,11 @@ void BC1Encoder::FindEndpoints(EncodeResults &result, const CBlock &pixels, cons auto &sums = metrics.sums; auto &min = metrics.min; + auto &max = metrics.max; unsigned chan0 = (unsigned)diff.MaxChannelRGB(); // primary axis of the bounding box l[chan0] = (float)min[chan0]; - h[chan0] = (float)min[chan0]; + h[chan0] = (float)max[chan0]; assert((diff[chan0] >= diff[(chan0 + 1) % 3]) && (diff[chan0] >= diff[(chan0 + 2) % 3])); @@ -582,6 +599,7 @@ void BC1Encoder::FindEndpoints(EncodeResults &result, const CBlock &pixels, cons result.high = Color::PreciseRound565(h); } else if (endpoint_mode == EndpointMode::BoundingBoxInt) { // Algorithm from icbc.h compress_dxt1_fast(), but converted to integer. + // TODO: handle constant blue channel better Color min, max; @@ -608,6 +626,9 @@ void BC1Encoder::FindEndpoints(EncodeResults &result, const CBlock &pixels, cons } else if (endpoint_mode == EndpointMode::PCA) { // the slow way // Select 2 colors along the principle axis. (There must be a faster/simpler way.) + + // TODO: handle constant blue channel better + auto min = Vector4::FromColorRGB(metrics.min); auto max = Vector4::FromColorRGB(metrics.max); auto avg = Vector4::FromColorRGB(metrics.avg); @@ -638,8 +659,9 @@ void BC1Encoder::FindEndpoints(EncodeResults &result, const CBlock &pixels, cons if (covariance[0][2] < 0) delta[0] = -delta[0]; // r vs b if (covariance[1][2] < 0) delta[1] = -delta[1]; // g vs b - // using the covariance matrix, stretch the delta vector towards the primary axis of the data using power iteration - // the end result of this may actually be the same as the least squares approach, will have to do more research + // using the covariance matrix, stretch the delta vector towards the primary axis of the data using power + // iteration the end result of this may actually be the same as the least squares approach, will have to do more + // research for (unsigned power_iter = 0; power_iter < _power_iterations; power_iter++) { delta = covariance * delta; } // if we found any correlation, then this is our new axis. otherwise we fallback to the luma vector @@ -678,7 +700,8 @@ void BC1Encoder::FindEndpoints(EncodeResults &result, const CBlock &pixels, cons result.color_mode = ColorMode::Incomplete; } -template void BC1Encoder::FindSelectors(EncodeResults &result, const CBlock &pixels, ErrorMode error_mode) const { +template +void BC1Encoder::FindSelectors(EncodeResults &result, const CBlock &pixels, ErrorMode error_mode) const { assert(!((error_mode != ErrorMode::Full) && (bool)(M & ColorMode::ThreeColor))); const int color_count = (unsigned)M & 0x0F; @@ -687,11 +710,11 @@ template void BC1Encoder::FindSelectors(EncodeResults std::array color_vectors; if (color_count == 4) { - color_vectors = {Vector4Int::FromColorRGB(colors[0]), Vector4Int::FromColorRGB(colors[2]), Vector4Int::FromColorRGB(colors[3]), - Vector4Int::FromColorRGB(colors[1])}; + color_vectors = {Vector4Int::FromColorRGB(colors[0]), Vector4Int::FromColorRGB(colors[2]), + Vector4Int::FromColorRGB(colors[3]), Vector4Int::FromColorRGB(colors[1])}; } else { - color_vectors = {Vector4Int::FromColorRGB(colors[0]), Vector4Int::FromColorRGB(colors[2]), Vector4Int::FromColorRGB(colors[1]), - Vector4Int::FromColorRGB(colors[3])}; + color_vectors = {Vector4Int::FromColorRGB(colors[0]), Vector4Int::FromColorRGB(colors[2]), + Vector4Int::FromColorRGB(colors[1]), Vector4Int::FromColorRGB(colors[3])}; } unsigned total_error = 0; @@ -715,7 +738,8 @@ template void BC1Encoder::FindSelectors(EncodeResults // llvm is just going to unswitch this anyways so its not an issue auto diff = pixel_vector - color_vectors[selector]; total_error += diff.SqrMag(); - if (i % 4 != 0 && total_error >= result.error) break; // check only once per row if we're generating too much error + if (i % 4 != 0 && total_error >= result.error) + break; // check only once per row if we're generating too much error } result.selectors[i] = selector; @@ -780,7 +804,8 @@ template void BC1Encoder::FindSelectors(EncodeResults result.color_mode = M; } -template bool BC1Encoder::RefineEndpointsLS(EncodeResults &result, const CBlock &pixels, BlockMetrics metrics) const { +template +bool BC1Encoder::RefineEndpointsLS(EncodeResults &result, const CBlock &pixels, BlockMetrics metrics) const { const int color_count = (unsigned)M & 0x0F; static_assert(color_count == 3 || color_count == 4); assert(result.color_mode != ColorMode::Incomplete); @@ -795,7 +820,8 @@ template bool BC1Encoder::RefineEndpointsLS(EncodeResu const uint8_t sel = result.selectors[i]; if ((bool)(M & ColorMode::ThreeColorBlack) && color.IsBlack()) continue; - if ((bool)(M & ColorMode::ThreeColor) && sel == 3U) continue; // NOTE: selectors for 3-color are in linear order here, but not in original + if ((bool)(M & ColorMode::ThreeColor) && sel == 3U) + continue; // NOTE: selectors for 3-color are in linear order here, but not in original assert(sel < color_count); const Vector4Int color_vector = Vector4Int::FromColorRGB(color); @@ -826,7 +852,9 @@ template bool BC1Encoder::RefineEndpointsLS(EncodeResu return true; } -template void BC1Encoder::RefineEndpointsLS(EncodeResults &result, std::array &sums, Vector4 &matrix, Hash hash) const { +template +void BC1Encoder::RefineEndpointsLS(EncodeResults &result, std::array &sums, Vector4 &matrix, + Hash hash) const { const int color_count = (unsigned)M & 0x0F; static_assert(color_count == 3 || color_count == 4); assert(result.color_mode != ColorMode::Incomplete); @@ -852,7 +880,8 @@ template void BC1Encoder::RefineEndpointsLS(EncodeResu } template -void BC1Encoder::RefineBlockLS(EncodeResults &result, const CBlock &pixels, const BlockMetrics &metrics, ErrorMode error_mode, unsigned passes) const { +void BC1Encoder::RefineBlockLS(EncodeResults &result, const CBlock &pixels, const BlockMetrics &metrics, + ErrorMode error_mode, unsigned passes) const { assert(error_mode != ErrorMode::None || passes == 1); for (unsigned pass = 0; pass < passes; pass++) { @@ -877,7 +906,8 @@ void BC1Encoder::RefineBlockLS(EncodeResults &result, const CBlock &pixels, cons } template -void BC1Encoder::RefineBlockCF(EncodeResults &result, const CBlock &pixels, const BlockMetrics &metrics, ErrorMode error_mode, unsigned orderings) const { +void BC1Encoder::RefineBlockCF(EncodeResults &result, const CBlock &pixels, const BlockMetrics &metrics, + ErrorMode error_mode, unsigned orderings) const { const int color_count = (unsigned)M & 0x0F; static_assert(color_count == 3 || color_count == 4); assert(result.color_mode != ColorMode::Incomplete); @@ -956,7 +986,8 @@ void BC1Encoder::EndpointSearch(EncodeResults &result, const CBlock &pixels) con for (unsigned i = 0; i < _search_rounds; i++) { const unsigned voxel_index = (unsigned)(i & 15); - assert((unsigned)Voxels[(unsigned)Voxels[voxel_index][3]][3] == voxel_index); // make sure voxels are symmetrical + assert((unsigned)Voxels[(unsigned)Voxels[voxel_index][3]][3] == + voxel_index); // make sure voxels are symmetrical if ((int)(i & 31) == forbidden_direction) continue; diff --git a/tests/test_bc1.py b/tests/test_bc1.py index 4f9040f..0d48099 100644 --- a/tests/test_bc1.py +++ b/tests/test_bc1.py @@ -138,9 +138,10 @@ class TestBC1Texture: class TestBC1Encoder: """Test BC1Encoder""" - def test_block_4color(self, color_mode): + @pytest.mark.parametrize('level', range(18)) + def test_block_4color(self, level, color_mode): """Test encoder output with 4 color greyscale test block""" - encoder = BC1Encoder(color_mode=color_mode) + encoder = BC1Encoder(level, color_mode) out_tex = encoder.encode(BC1Blocks.greyscale.texture) out_block = out_tex[0, 0] @@ -149,9 +150,10 @@ class TestBC1Encoder: assert not out_block.is_3color assert out_block == BC1Blocks.greyscale.block - def test_block_3color(self, color_mode): + @pytest.mark.parametrize('level', range(2, 18)) # lowest 2 levels can be improved, but right now choke on this test + def test_block_3color(self, level, color_mode): """Test encoder output with 3 color test block""" - encoder = BC1Encoder(color_mode=color_mode) + encoder = BC1Encoder(level, color_mode) out_tex = encoder.encode(BC1Blocks.three_color.texture) out_block = out_tex[0, 0] @@ -164,9 +166,10 @@ class TestBC1Encoder: else: assert not out_block.is_3color - def test_block_3color_black(self, color_mode): + @pytest.mark.parametrize('level', range(2, 18)) # lowest 2 levels can be improved, but right now choke on this test + def test_block_3color_black(self, level, color_mode): """Test encoder output with 3 color test block with black pixels""" - encoder = BC1Encoder(color_mode=color_mode) + encoder = BC1Encoder(level, color_mode) out_tex = encoder.encode(BC1Blocks.three_color_black.texture) out_block = out_tex[0, 0] diff --git a/tests/test_install.py b/tests/test_install.py index f61ea21..7feaa1f 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -1,11 +1,12 @@ """Test if everything is installed correctly""" +import _quicktex import pytest import quicktex class TestInstall: - @pytest.mark.skipif(quicktex._debug_build, reason="Debug builds dont have valid version strings") + @pytest.mark.skipif(_quicktex._debug_build, reason="Debug builds dont have valid version strings") def test_version(self): """Test if the extension module version matches what setuptools returns""" try: