From f4fd8a0648ccf398436d5a5c99a321fc229547d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 5 Aug 2019 16:21:57 +0200 Subject: [PATCH] DebugTools: rewrite CompareImage guts to use StridedArrayView. Yay. Much simpler code. --- src/Magnum/DebugTools/CompareImage.cpp | 80 +++++++++++--------------- 1 file changed, 35 insertions(+), 45 deletions(-) diff --git a/src/Magnum/DebugTools/CompareImage.cpp b/src/Magnum/DebugTools/CompareImage.cpp index a26a359aa..6001195f9 100644 --- a/src/Magnum/DebugTools/CompareImage.cpp +++ b/src/Magnum/DebugTools/CompareImage.cpp @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -45,30 +46,21 @@ namespace Magnum { namespace DebugTools { namespace Implementation { namespace { -template Math::Vector pixelAt(const char* const pixels, const std::size_t stride, const Vector2i& pos) { - return reinterpret_cast*>(pixels + stride*pos.y())[pos.x()]; -} - -template Float calculateImageDelta(const ImageView2D& actual, const ImageView2D& expected, Containers::ArrayView output) { - CORRADE_INTERNAL_ASSERT(output.size() == std::size_t(expected.size().product())); - - /* Precalculate parameters for pixel access */ - Math::Vector2 dataOffset, dataSize; - - std::tie(dataOffset, dataSize) = actual.dataProperties(); - const char* const actualPixels = actual.data() + dataOffset.sum(); - const std::size_t actualStride = dataSize.x(); - - std::tie(dataOffset, dataSize) = expected.dataProperties(); - const char* const expectedPixels = expected.data() + dataOffset.sum(); - const std::size_t expectedStride = dataSize.x(); +template Float calculateImageDelta(const Containers::StridedArrayView2D>& actual, const Containers::StridedArrayView2D>& expected, const Containers::StridedArrayView2D& output) { + CORRADE_INTERNAL_ASSERT(actual.size() == output.size()); + CORRADE_INTERNAL_ASSERT(output.size() == expected.size()); /* Calculate deltas and maximal value of them */ Float max{}; - for(std::int_fast32_t y = 0; y != expected.size().y(); ++y) { - for(std::int_fast32_t x = 0; x != expected.size().x(); ++x) { - Math::Vector actualPixel{pixelAt(actualPixels, actualStride, {Int(x), Int(y)})}; - Math::Vector expectedPixel{pixelAt(expectedPixels, expectedStride, {Int(x), Int(y)})}; + for(std::size_t i = 0, iMax = expected.size()[0]; i != iMax; ++i) { + Containers::StridedArrayView1D> actualRow = actual[i]; + Containers::StridedArrayView1D> expectedRow = expected[i]; + Containers::StridedArrayView1D outputRow = output[i]; + + for(std::size_t j = 0, jMax = expectedRow.size(); j != jMax; ++j) { + /* Explicitly convert from T to Float */ + auto actualPixel = Math::Vector(actualRow[j]); + auto expectedPixel = Math::Vector(expectedRow[j]); /* First calculate a classic difference */ Math::Vector diff = Math::abs(actualPixel - expectedPixel); @@ -83,7 +75,7 @@ template Float calculateImageDelta(const ImageView2D& /* Calculate the difference and save it to the output image even with NaN and ±Inf (as the user should know) */ - output[y*expected.size().x() + x] = diff.sum()/size; + outputRow[j] = diff.sum()/size; /* On the other hand, infs and NaNs should not contribute to the max delta -- because all other differences would be zero @@ -99,7 +91,10 @@ template Float calculateImageDelta(const ImageView2D& std::tuple, Float, Float> calculateImageDelta(const ImageView2D& actual, const ImageView2D& expected) { /* Calculate a delta image */ - Containers::Array delta{Containers::NoInit, std::size_t(expected.size().product())}; + Containers::Array deltaData{Containers::NoInit, + std::size_t(expected.size().product())}; + Containers::StridedArrayView2D delta{deltaData, + {std::size_t(expected.size().y()), std::size_t(expected.size().x())}}; CORRADE_ASSERT(!isPixelFormatImplementationSpecific(expected.format()), "DebugTools::CompareImage: can't compare implementation-specific pixel formats", {}); @@ -108,12 +103,16 @@ std::tuple, Float, Float> calculateImageDelta(const Ima switch(expected.format()) { #define _c(format, size, T) \ case PixelFormat::format: \ - max = calculateImageDelta(actual, expected, delta); \ + max = calculateImageDelta( \ + actual.pixels>(), \ + expected.pixels>(), delta); \ break; #define _d(first, second, size, T) \ case PixelFormat::first: \ case PixelFormat::second: \ - max = calculateImageDelta(actual, expected, delta); \ + max = calculateImageDelta( \ + actual.pixels>(), \ + expected.pixels>(), delta); \ break; /* LCOV_EXCL_START */ _d(R8Unorm, R8UI, 1, UnsignedByte) @@ -164,9 +163,9 @@ std::tuple, Float, Float> calculateImageDelta(const Ima *deliberately* leaves specials in. The `max` has them already filtered out so if this would filter them out as well, there would be nothing left that could cause the comparison to fail. */ - const Float mean = Math::Algorithms::kahanSum(delta.begin(), delta.end())/delta.size(); + const Float mean = Math::Algorithms::kahanSum(deltaData.begin(), deltaData.end())/deltaData.size(); - return std::make_tuple(std::move(delta), max, mean); + return std::make_tuple(std::move(deltaData), max, mean); } namespace { @@ -220,16 +219,18 @@ void printDeltaImage(Debug& out, Containers::ArrayView deltas, cons namespace { -void printPixelAt(Debug& out, const char* const pixels, const std::size_t stride, const Vector2i& pos, const PixelFormat format) { +void printPixelAt(Debug& out, const Containers::StridedArrayView3D& pixels, const Vector2i& pos, const PixelFormat format) { + const char* const pixel = &pixels[pos.y()][pos.x()][0]; + switch(format) { #define _c(format, size, T) \ case PixelFormat::format: \ - out << pixelAt(pixels, stride, pos); \ + out << *reinterpret_cast*>(pixel); \ break; #define _d(first, second, size, T) \ case PixelFormat::first: \ case PixelFormat::second: \ - out << pixelAt(pixels, stride, pos); \ + out << *reinterpret_cast*>(pixel); \ break; /* LCOV_EXCL_START */ _d(R8Unorm, R8UI, 1, UnsignedByte) @@ -267,10 +268,10 @@ void printPixelAt(Debug& out, const char* const pixels, const std::size_t stride /* Take the opportunity and print 8-bit colors in hex */ case PixelFormat::RGB8Unorm: - out << Color3ub{pixelAt<3, UnsignedByte>(pixels, stride, pos)}; + out << *reinterpret_cast(pixel); break; case PixelFormat::RGBA8Unorm: - out << Color4ub{pixelAt<4, UnsignedByte>(pixels, stride, pos)}; + out << *reinterpret_cast(pixel); break; case PixelFormat::R16F: @@ -285,17 +286,6 @@ void printPixelAt(Debug& out, const char* const pixels, const std::size_t stride } void printPixelDeltas(Debug& out, Containers::ArrayView delta, const ImageView2D& actual, const ImageView2D& expected, const Float maxThreshold, const Float meanThreshold, std::size_t maxCount) { - /* Precalculate parameters for pixel access */ - Math::Vector2 offset, size; - - std::tie(offset, size) = actual.dataProperties(); - const char* const actualPixels = actual.data() + offset.sum(); - const std::size_t actualStride = size.x(); - - std::tie(offset, size) = expected.dataProperties(); - const char* const expectedPixels = expected.data() + offset.sum(); - const std::size_t expectedStride = size.x(); - /* Find first maxCount values above mean threshold and put them into a sorted map. Need to reverse the condition in order to catch NaNs. */ std::multimap large; @@ -321,11 +311,11 @@ void printPixelDeltas(Debug& out, Containers::ArrayView delta, cons << Debug::nospace << "," << Debug::nospace << pos.y() << Debug::nospace << "]"; - printPixelAt(out, actualPixels, actualStride, pos, expected.format()); + printPixelAt(out, actual.pixels(), pos, expected.format()); out << Debug::nospace << ", expected"; - printPixelAt(out, expectedPixels, expectedStride, pos, expected.format()); + printPixelAt(out, expected.pixels(), pos, expected.format()); out << "(Δ =" << Debug::boldColor(delta[it->second] > maxThreshold ? Debug::Color::Red : Debug::Color::Yellow) << delta[it->second]