From f589aa1a5d0f90d37722ab9ff79d5e181a903bcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 4 Jun 2023 14:55:20 +0200 Subject: [PATCH] DebugTools: improve pixel view format autodetection in CompareImage. Originally it was just assuming that any Vector3ub or Color3ub is a normalized format. That was kinda enough for many cases, but it started to get annoying with sRGB image comparisons, as those had to be manually reinterpret with a sRGB-less format in order to pass. Now the pixel format detection looks at the expected image format as well, and if the underlying type and component count matches, it inherits the sRGB and normalized property from it as well. If not, it falls back to an integer format for vectors, and normalized format for colors. For vectors this is different from previous behavior but shouldn't cause any problem in practice -- the only result will be that the image comparison fails with a different message for pixel format mismatch than before. This now also properly and fully tests the pixelFormatFor() helper, and adds a missing Color3 specialization of it. --- doc/changelog.dox | 3 +- src/Magnum/DebugTools/CompareImage.cpp | 17 +- src/Magnum/DebugTools/CompareImage.h | 382 +++++++++++++----- .../DebugTools/Test/CompareImageTest.cpp | 289 ++++++++++++- 4 files changed, 579 insertions(+), 112 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 220603f97..b482986e4 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -478,7 +478,8 @@ See also: relying on memory-mapping, and is now available on WebGL 2 as well (see [mosra/magnum#560](https://github.com/mosra/magnum/pull/560)) - @ref DebugTools::CompareImage now supports comparing half-float pixel - formats as well + formats as well, pixel format autodetection for pixel views tries to + match sRGB and normalization properties of the expected image format @subsubsection changelog-latest-changes-gl GL library diff --git a/src/Magnum/DebugTools/CompareImage.cpp b/src/Magnum/DebugTools/CompareImage.cpp index d4c67ff8f..e8b29074f 100644 --- a/src/Magnum/DebugTools/CompareImage.cpp +++ b/src/Magnum/DebugTools/CompareImage.cpp @@ -545,6 +545,14 @@ TestSuite::ComparisonStatusFlags ImageComparatorBase::compare(const PixelFormat return flags; } +TestSuite::ComparisonStatusFlags ImageComparatorBase::compare(PixelFormat(*const actualFormatFor)(PixelFormat), const Containers::StridedArrayView3D& actualPixels, const ImageView2D& expected) { + /* Figure out the actual format for the pixel view, attempting to match it + with what's in the expected image (such as making it sRGB if the + expected format is also sRGB and has the same component count and + underlying type as the view) */ + return compare(actualFormatFor(expected.format()), actualPixels, expected); +} + TestSuite::ComparisonStatusFlags ImageComparatorBase::operator()(const ImageView2D& actual, const ImageView2D& expected) { return compare(actual.format(), actual.pixels(), expected); } @@ -608,7 +616,7 @@ TestSuite::ComparisonStatusFlags ImageComparatorBase::operator()(const Container return flags; } -TestSuite::ComparisonStatusFlags ImageComparatorBase::compare(const PixelFormat actualFormat, const Containers::StridedArrayView3D& actualPixels, const Containers::StringView expected) { +TestSuite::ComparisonStatusFlags ImageComparatorBase::compare(const PixelFormat actualFormat, PixelFormat(*const actualFormatFor)(PixelFormat), const Containers::StridedArrayView3D& actualPixels, const Containers::StringView expected) { _state->expectedFilename = expected; Containers::Pointer importer; @@ -644,6 +652,11 @@ TestSuite::ComparisonStatusFlags ImageComparatorBase::compare(const PixelFormat return TestSuite::ComparisonStatusFlag::Failed|TestSuite::ComparisonStatusFlag::Diagnostic; } + /* Now that the image is loaded, try to match the actual format to it if + requested */ + if(actualFormatFor) + _state->actualFormat = actualFormatFor(_state->expectedImageData->format()); + /* Save a view on the expected image data and proxy to the actual data comparison. If comparison failed, offer to save a diagnostic. */ _state->expectedImage.emplace(*_state->expectedImageData); @@ -654,7 +667,7 @@ TestSuite::ComparisonStatusFlags ImageComparatorBase::compare(const PixelFormat } TestSuite::ComparisonStatusFlags ImageComparatorBase::operator()(const ImageView2D& actual, const Containers::StringView expected) { - return compare(actual.format(), actual.pixels(), expected); + return compare(actual.format(), nullptr, actual.pixels(), expected); } TestSuite::ComparisonStatusFlags ImageComparatorBase::operator()(const Containers::StringView actual, const ImageView2D& expected) { diff --git a/src/Magnum/DebugTools/CompareImage.h b/src/Magnum/DebugTools/CompareImage.h index 36944b668..36c8b01cb 100644 --- a/src/Magnum/DebugTools/CompareImage.h +++ b/src/Magnum/DebugTools/CompareImage.h @@ -62,7 +62,9 @@ class CompareFileToImage; namespace Implementation { -template constexpr PixelFormat pixelFormatFor(); +/* Used by the pixel view comparators to try to guess a format matching the + expected image */ +template constexpr PixelFormat pixelFormatFor(PixelFormat); class MAGNUM_DEBUGTOOLS_EXPORT ImageComparatorBase { public: @@ -80,11 +82,15 @@ class MAGNUM_DEBUGTOOLS_EXPORT ImageComparatorBase { TestSuite::ComparisonStatusFlags operator()(const ImageView2D& actual, Containers::StringView expected); - /* Used in templated CompareImage::operator() */ TestSuite::ComparisonStatusFlags compare(PixelFormat actualFormat, const Containers::StridedArrayView3D& actualPixels, const ImageView2D& expected); + /* Used in templated CompareImage::operator(), the actual format gets + matched to the format in the expected image */ + TestSuite::ComparisonStatusFlags compare(PixelFormat(*actualFormatFor)(PixelFormat), const Containers::StridedArrayView3D& actualPixels, const ImageView2D& expected); - /* Used in templated CompareImageToFile::operator() */ - TestSuite::ComparisonStatusFlags compare(PixelFormat actualFormat, const Containers::StridedArrayView3D& actualPixels, Containers::StringView expected); + /* Used in templated CompareImageToFile::operator(). If actualFormatFor + isn't nullptr, the actualFormat gets overriden by it once the + expected image is loaded. */ + TestSuite::ComparisonStatusFlags compare(PixelFormat actualFormat, PixelFormat(*actualFormatFor)(PixelFormat), const Containers::StridedArrayView3D& actualPixels, Containers::StringView expected); void printMessage(TestSuite::ComparisonStatusFlags flags, Debug& out, Containers::StringView actual, Containers::StringView expected) const; @@ -121,9 +127,8 @@ template<> class MAGNUM_DEBUGTOOLS_EXPORT Comparator TestSuite::ComparisonStatusFlags operator()(const Containers::StridedArrayView2D& actualPixels, const Magnum::ImageView2D& expected) { - /** @todo do some tryFindCompatibleFormat() here */ return Magnum::DebugTools::Implementation::ImageComparatorBase::compare( - Magnum::DebugTools::Implementation::pixelFormatFor(), + Magnum::DebugTools::Implementation::pixelFormatFor, Containers::arrayCast<3, const char>(actualPixels), expected); } }; @@ -149,9 +154,9 @@ template<> class MAGNUM_DEBUGTOOLS_EXPORT Comparator TestSuite::ComparisonStatusFlags operator()(const Containers::StridedArrayView2D& actualPixels, const Containers::StringView& expected) { - /** @todo do some tryFindCompatibleFormat() here */ return Magnum::DebugTools::Implementation::ImageComparatorBase::compare( - Magnum::DebugTools::Implementation::pixelFormatFor(), + Magnum::DebugTools::Implementation::pixelFormatFor(Magnum::PixelFormat{}), + Magnum::DebugTools::Implementation::pixelFormatFor, Containers::arrayCast<3, const char>(actualPixels), expected); } }; @@ -323,12 +328,16 @@ three-component you can cast the pixel data to just a three-component type: @snippet MagnumDebugTools-gl.cpp CompareImage-pixels-rgb -Currently, comparing against pixel views has a few inherent limitations --- it -has to be cast to one of Magnum scalar or vector types and the format is -then autodetected from the passed type, with normalized formats preferred. In -practice this means e.g. @ref Math::Vector2 "Math::Vector2" will -be understood as @ref PixelFormat::RG8Unorm and there's currently no way to -interpret it as @ref PixelFormat::RG8UI, for example. +The pixel views are expected to be cast to one of Magnum scalar or vector +types. The format is then autodetected from the passed type. For types that map +to more than one @ref PixelFormat, such as @relativeref{Magnum,Vector3ub} that +can be @ref PixelFormat::RGB8Unorm, @relativeref{PixelFormat,RGB8Srgb} or +@relativeref{PixelFormat,RGB8UI}, an attempt is made to match the pixel format +of the expected image if possible. If not possible, such as comparing a +@relativeref{Magnum,Vector3ub} view to a @ref PixelFormat::RGB8Snorm, the +comparison fails the same way as if comparing two views of different pixel +formats. Byte and short color types always autodetect to normalized (sRGB) +pixel formats, never to integer formats. @see @ref CompareMaterial */ @@ -605,84 +614,273 @@ class CompareFileToImage { namespace Implementation { -/* LCOV_EXCL_START */ -/* One-component types */ -template<> constexpr PixelFormat pixelFormatFor() { return PixelFormat::R8Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::R8Unorm; } -template<> constexpr PixelFormat pixelFormatFor() { return PixelFormat::R8Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::R8Snorm; } -template<> constexpr PixelFormat pixelFormatFor() { return PixelFormat::R16Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::R16Unorm; } -template<> constexpr PixelFormat pixelFormatFor() { return PixelFormat::R16Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::R16Snorm; } -template<> constexpr PixelFormat pixelFormatFor() { return PixelFormat::R32UI; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::R32UI; } -template<> constexpr PixelFormat pixelFormatFor() { return PixelFormat::R32I; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::R32I; } -template<> constexpr PixelFormat pixelFormatFor() { return PixelFormat::R32F; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::R32F; } - -/* Two-component types */ -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG8Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG8Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG8Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG8Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG16Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG16Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG16Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG16Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG32UI; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG32UI; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG32I; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG32I; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG32F; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RG32F; } - -/* Three-component types */ -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB8Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB8Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB8Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB8Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB8Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB8Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB16Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB16Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB16Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB16Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB16Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB16Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB32UI; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB32UI; } -/* Skipping Math::Color3, as that isn't much used */ -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB32I; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB32I; } -/* Skipping Math::Color3, as that isn't much used */ -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB32F; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGB32F; } - -/* Four-component types */ -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA8Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA8Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA8Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA8Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA8Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA8Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA16Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA16Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA16Unorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA16Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA16Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA16Snorm; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA32UI; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA32UI; } -/* Skipping Math::Color4, as that isn't much used */ -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA32I; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA32I; } -/* Skipping Math::Color4, as that isn't much used */ -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA32F; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA32F; } -template<> constexpr PixelFormat pixelFormatFor>() { return PixelFormat::RGBA32F; } -/* LCOV_EXCL_STOP */ +/* One-component types. The Vector<1, T> types aren't used that often so they + delegate to the T variants, the tests use Vector<1, T> to cover all + variants. */ +template<> constexpr PixelFormat pixelFormatFor(PixelFormat expected) { + /* Attempt to match Srgb or Unorm if the expected image has it */ + return expected == PixelFormat::R8Srgb ? PixelFormat::R8Srgb : + expected == PixelFormat::R8Unorm ? PixelFormat::R8Unorm : + PixelFormat::R8UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor(expected); +} +template<> constexpr PixelFormat pixelFormatFor(PixelFormat expected) { + /* Attempt to match Snorm if the expected image has it */ + return expected == PixelFormat::R8Snorm ? PixelFormat::R8Snorm : + PixelFormat::R8I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor(expected); +} +template<> constexpr PixelFormat pixelFormatFor(PixelFormat expected) { + /* Attempt to match Unorm if the expected image has it */ + return expected == PixelFormat::R16Unorm ? PixelFormat::R16Unorm : + PixelFormat::R16UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor(expected); +} +template<> constexpr PixelFormat pixelFormatFor(PixelFormat expected) { + /* Attempt to match Snorm if the expected image has it */ + return expected == PixelFormat::R16Snorm ? PixelFormat::R16Snorm : + PixelFormat::R16I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor(expected); +} +template<> constexpr PixelFormat pixelFormatFor(PixelFormat) { + return PixelFormat::R32UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor(expected); +} +template<> constexpr PixelFormat pixelFormatFor(PixelFormat) { + return PixelFormat::R32I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor(expected); +} +template<> constexpr PixelFormat pixelFormatFor(PixelFormat) { + return PixelFormat::R32F; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor(expected); +} + +/* Two-component types. The Vector<2, T> types aren't used that often so they + delegate to the Vector2 variants, the tests use Vector<2, T> to cover all + variants. */ +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Srgb or Unorm if the expected image has it */ + return expected == PixelFormat::RG8Srgb ? PixelFormat::RG8Srgb : + expected == PixelFormat::RG8Unorm ? PixelFormat::RG8Unorm : + PixelFormat::RG8UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Snorm if the expected image has it */ + return expected == PixelFormat::RG8Snorm ? PixelFormat::RG8Snorm : + PixelFormat::RG8I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Unorm if the expected image has it */ + return expected == PixelFormat::RG16Unorm ? PixelFormat::RG16Unorm : + PixelFormat::RG16UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Snorm if the expected image has it */ + return expected == PixelFormat::RG16Snorm ? PixelFormat::RG16Snorm : + PixelFormat::RG16I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RG32UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RG32I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RG32F; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} + +/* Three-component types. The Vector<3, T> types aren't used that often so they + delegate to the Vector3 variants, the tests use Vector<3, T> to cover all + variants. */ +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Srgb or Unorm if the expected image has it */ + return expected == PixelFormat::RGB8Srgb ? PixelFormat::RGB8Srgb : + expected == PixelFormat::RGB8Unorm ? PixelFormat::RGB8Unorm : + PixelFormat::RGB8UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Srgb if the expected image has it. No integer fallback + for color types. */ + return expected == PixelFormat::RGB8Srgb ? PixelFormat::RGB8Srgb : + PixelFormat::RGB8Unorm; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Snorm if the expected image has it */ + return expected == PixelFormat::RGB8Snorm ? PixelFormat::RGB8Snorm : + PixelFormat::RGB8I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + /* No integer fallback for colors */ + return PixelFormat::RGB8Snorm; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Unorm if the expected image has it */ + return expected == PixelFormat::RGB16Unorm ? PixelFormat::RGB16Unorm : + PixelFormat::RGB16UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RGB16Unorm; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Snorm if the expected image has it */ + return expected == PixelFormat::RGB16Snorm ? PixelFormat::RGB16Snorm : + PixelFormat::RGB16I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RGB16Snorm; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RGB32UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +/* Skipping Math::Color3, as integer colors should always match + normalized types */ +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RGB32I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +/* Skipping Math::Color3, as integer colors should always match normalized + types */ +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RGB32F; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RGB32F; +} + +/* Four-component types. The Vector<4, T> types aren't used that often so they + delegate to the Vector4 variants, the tests use Vector<4, T> to cover all + variants. */ +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Srgb or Unorm if the expected image has it */ + return expected == PixelFormat::RGBA8Srgb ? PixelFormat::RGBA8Srgb : + expected == PixelFormat::RGBA8Unorm ? PixelFormat::RGBA8Unorm : + PixelFormat::RGBA8UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Srgb if the expected image has it. No integer fallback + for color types. */ + return expected == PixelFormat::RGBA8Srgb ? PixelFormat::RGBA8Srgb : + PixelFormat::RGBA8Unorm; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Snorm if the expected image has it */ + return expected == PixelFormat::RGBA8Snorm ? PixelFormat::RGBA8Snorm : + PixelFormat::RGBA8I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + /* No integer fallback for colors */ + return PixelFormat::RGBA8Snorm; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Unorm if the expected image has it */ + return expected == PixelFormat::RGBA16Unorm ? PixelFormat::RGBA16Unorm : + PixelFormat::RGBA16UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + /* No integer fallback for colors */ + return PixelFormat::RGBA16Unorm; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + /* Attempt to match Snorm if the expected image has it */ + return expected == PixelFormat::RGBA16Snorm ? PixelFormat::RGBA16Snorm : + PixelFormat::RGBA16I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + /* No integer fallback for colors */ + return PixelFormat::RGBA16Snorm; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RGBA32UI; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +/* Skipping Math::Color4, as integer colors should always match + normalized types */ +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RGBA32I; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +/* Skipping Math::Color4, as integer colors should always match normalized + types */ +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RGBA32F; +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat expected) { + return pixelFormatFor>(expected); +} +template<> constexpr PixelFormat pixelFormatFor>(PixelFormat) { + return PixelFormat::RGBA32F; +} } diff --git a/src/Magnum/DebugTools/Test/CompareImageTest.cpp b/src/Magnum/DebugTools/Test/CompareImageTest.cpp index 0dc09988e..56f9950d1 100644 --- a/src/Magnum/DebugTools/Test/CompareImageTest.cpp +++ b/src/Magnum/DebugTools/Test/CompareImageTest.cpp @@ -115,18 +115,32 @@ struct CompareImageTest: TestSuite::Tester { void fileToImageActualLoadFailed(); void fileToImageActualIsCompressed(); - void pixelsToImageZeroDelta(); + template void pixelFormatFor(); + void pixelFormatForColor(); + + template void pixelsToImageZeroDelta(); void pixelsToImageNonZeroDelta(); + void pixelsToImageDifferentFormat(); void pixelsToImageError(); - void pixelsToFileZeroDelta(); + template void pixelsToFileZeroDelta(); void pixelsToFileNonZeroDelta(); + void pixelsToFileDifferentFormat(); void pixelsToFileError(); + void pixelsToFileExpectedLoadFailed(); private: Containers::Optional> _importerManager; Containers::Optional> _converterManager; }; +const struct { + const char* name; + bool srgb; +} PixelsToImageData[]{ + {"", false}, + {"sRGB", true} +}; + CompareImageTest::CompareImageTest() { addTests({&CompareImageTest::formatUnknown, &CompareImageTest::formatPackedDepthStencil, @@ -209,13 +223,32 @@ CompareImageTest::CompareImageTest() { addTests({&CompareImageTest::fileToImageActualIsCompressed}); - addTests({&CompareImageTest::pixelsToImageZeroDelta, - &CompareImageTest::pixelsToImageNonZeroDelta, + addTests({&CompareImageTest::pixelFormatFor<1>, + &CompareImageTest::pixelFormatFor<2>, + &CompareImageTest::pixelFormatFor<3>, + &CompareImageTest::pixelFormatFor<4>, + &CompareImageTest::pixelFormatForColor}); + + addInstancedTests({ + &CompareImageTest::pixelsToImageZeroDelta, + &CompareImageTest::pixelsToImageZeroDelta}, + Containers::arraySize(PixelsToImageData)); + + addTests({&CompareImageTest::pixelsToImageNonZeroDelta, + &CompareImageTest::pixelsToImageDifferentFormat, &CompareImageTest::pixelsToImageError}); - addTests({&CompareImageTest::pixelsToFileZeroDelta, - &CompareImageTest::pixelsToFileNonZeroDelta, - &CompareImageTest::pixelsToFileError}, + addInstancedTests({ + &CompareImageTest::pixelsToFileZeroDelta, + &CompareImageTest::pixelsToFileZeroDelta}, + Containers::arraySize(PixelsToImageData), + &CompareImageTest::setupExternalPluginManager, + &CompareImageTest::teardownExternalPluginManager); + + addTests({&CompareImageTest::pixelsToFileNonZeroDelta, + &CompareImageTest::pixelsToFileDifferentFormat, + &CompareImageTest::pixelsToFileError, + &CompareImageTest::pixelsToFileExpectedLoadFailed}, &CompareImageTest::setupExternalPluginManager, &CompareImageTest::teardownExternalPluginManager); @@ -1654,15 +1687,132 @@ void CompareImageTest::fileToImageActualIsCompressed() { "Actual image a (.../CompareImageCompressed.dds) is compressed, comparison not possible.\n"); } -void CompareImageTest::pixelsToImageZeroDelta() { - /* Same as image(), but taking pixels instead */ +template void CompareImageTest::pixelFormatFor() { + setTestCaseTemplateName(Utility::format("{}", dimensions)); + + /* Defaults to an integer / float format if no match */ + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(PixelFormat{})), + pixelFormat(PixelFormat::R8UI, dimensions, false)); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(PixelFormat{})), + pixelFormat(PixelFormat::R8I, dimensions, false)); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(PixelFormat{})), + pixelFormat(PixelFormat::R16UI, dimensions, false)); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(PixelFormat{})), + pixelFormat(PixelFormat::R16I, dimensions, false)); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(PixelFormat{})), + pixelFormat(PixelFormat::R32UI, dimensions, false)); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(PixelFormat{})), + pixelFormat(PixelFormat::R32I, dimensions, false)); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(PixelFormat{})), + pixelFormat(PixelFormat::R32F, dimensions, false)); + + /* Matching normalized type if the image has it */ + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(pixelFormat(PixelFormat::R8Unorm, dimensions, false))), + pixelFormat(PixelFormat::R8Unorm, dimensions, false)); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(pixelFormat(PixelFormat::R8Snorm, dimensions, false))), + pixelFormat(PixelFormat::R8Snorm, dimensions, false)); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(pixelFormat(PixelFormat::R16Unorm, dimensions, false))), + pixelFormat(PixelFormat::R16Unorm, dimensions, false)); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(pixelFormat(PixelFormat::R16Snorm, dimensions, false))), + pixelFormat(PixelFormat::R16Snorm, dimensions, false)); + + /* Matching sRGB type if the image has it */ + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(pixelFormat(PixelFormat::R8Srgb, dimensions, true))), + pixelFormat(PixelFormat::R8Srgb, dimensions, true)); + + /* But not if it has different underlying type */ + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(pixelFormat(PixelFormat::R8Snorm, dimensions, false))), + pixelFormat(PixelFormat::R16I, dimensions, false)); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(pixelFormat(PixelFormat::R8Srgb, dimensions, false))), + pixelFormat(PixelFormat::R16UI, dimensions, false)); +} - CORRADE_COMPARE_WITH(ExpectedRgb.pixels(), - ExpectedRgb, (CompareImage{40.0f, 20.0f})); +void CompareImageTest::pixelFormatForColor() { + /* Defaults to a normalized format if no match */ + CORRADE_COMPARE( + (Implementation::pixelFormatFor(PixelFormat{})), + PixelFormat::RGB8Unorm); + CORRADE_COMPARE( + (Implementation::pixelFormatFor(PixelFormat{})), + PixelFormat::RGBA8Unorm); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(PixelFormat{})), + PixelFormat::RGB8Snorm); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(PixelFormat{})), + PixelFormat::RGBA8Snorm); + CORRADE_COMPARE( + (Implementation::pixelFormatFor(PixelFormat{})), + PixelFormat::RGB16Unorm); + CORRADE_COMPARE( + (Implementation::pixelFormatFor(PixelFormat{})), + PixelFormat::RGBA16Unorm); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(PixelFormat{})), + PixelFormat::RGB16Snorm); + CORRADE_COMPARE( + (Implementation::pixelFormatFor>(PixelFormat{})), + PixelFormat::RGBA16Snorm); + CORRADE_COMPARE( + (Implementation::pixelFormatFor(PixelFormat{})), + PixelFormat::RGB32F); + CORRADE_COMPARE( + (Implementation::pixelFormatFor(PixelFormat{})), + PixelFormat::RGBA32F); + + /* Matching sRGB type if the image has it */ + CORRADE_COMPARE( + Implementation::pixelFormatFor(PixelFormat::RGB8Srgb), + PixelFormat::RGB8Srgb); + CORRADE_COMPARE( + Implementation::pixelFormatFor(PixelFormat::RGBA8Srgb), + PixelFormat::RGBA8Srgb); + + /* But not if it has different underlying type or dimension count */ + CORRADE_COMPARE( + Implementation::pixelFormatFor(PixelFormat::RGB8Srgb), + PixelFormat::RGB16Unorm); + CORRADE_COMPARE( + Implementation::pixelFormatFor(PixelFormat::RGB8Srgb), + PixelFormat::RGBA8Unorm); +} + +template void CompareImageTest::pixelsToImageZeroDelta() { + auto&& data = PixelsToImageData[testCaseInstanceId()]; + setTestCaseTemplateName(std::is_same::value ? "Color3ub" : "Vector3ub"); + setTestCaseDescription(data.name); + + /* Same as image(), but taking pixels instead. For T being Color3ub, the + autodetected PixelFormat is RGB8Unorm, for Vector3ub it's RGB8UI. It + should get matched to either RGB8Unorm or RGB8Srgb based on the format + in the expected image. */ + + /* Same as ExpectedRGB but with pixel format being different */ + const ImageView2D expected{ + PixelStorage{}.setSkip({1, 0, 0}).setRowLength(3), + pixelFormat(PixelFormat::RGB8Unorm, 3, data.srgb), + {2, 2}, ExpectedRgbData}; + + CORRADE_COMPARE_WITH(expected.pixels(), + expected, (CompareImage{40.0f, 20.0f})); /* No diagnostic as there's no error */ TestSuite::Comparator compare{40.0f, 20.0f}; - CORRADE_COMPARE(compare(ExpectedRgb.pixels(), ExpectedRgb), TestSuite::ComparisonStatusFlags{}); + CORRADE_COMPARE(compare(expected.pixels(), expected), TestSuite::ComparisonStatusFlags{}); } void CompareImageTest::pixelsToImageNonZeroDelta() { @@ -1685,6 +1835,22 @@ void CompareImageTest::pixelsToImageNonZeroDelta() { CORRADE_COMPARE(out.str(), ImageCompareVerbose); } +void CompareImageTest::pixelsToImageDifferentFormat() { + std::stringstream out; + + { + TestSuite::Comparator compare{{}, {}}; + TestSuite::ComparisonStatusFlags flags = + compare(ExpectedRgb.pixels(), ExpectedRgb); + /* No diagnostic as we don't have any expected filename */ + CORRADE_COMPARE(flags, TestSuite::ComparisonStatusFlag::Failed); + Debug d{&out, Debug::Flag::DisableColors}; + compare.printMessage(flags, d, "a", "b"); + } + + CORRADE_COMPARE(out.str(), "Images a and b have different format, actual PixelFormat::RGB8I but PixelFormat::RGB8Unorm expected.\n"); +} + void CompareImageTest::pixelsToImageError() { /* Same as imageError(), but taking pixels instead */ @@ -1703,8 +1869,15 @@ void CompareImageTest::pixelsToImageError() { CORRADE_COMPARE(out.str(), ImageCompareError); } -void CompareImageTest::pixelsToFileZeroDelta() { - /* Same as imageToFile(), but taking pixels instead */ +template void CompareImageTest::pixelsToFileZeroDelta() { + auto&& data = PixelsToImageData[testCaseInstanceId()]; + setTestCaseTemplateName(std::is_same::value ? "Color3ub" : "Vector3ub"); + setTestCaseDescription(data.name); + + /* Same as imageToFile(), but taking pixels instead. For T being Color3ub, + the autodetected PixelFormat is RGB8Unorm, for Vector3ub it's RGB8UI. It + should get matched to either RGB8Unorm or RGB8Srgb based on the format + in the expected image. */ if(!(_importerManager->loadState("AnyImageImporter") & PluginManager::LoadState::Loaded) || !(_importerManager->loadState("TgaImporter") & PluginManager::LoadState::Loaded)) @@ -1716,12 +1889,18 @@ void CompareImageTest::pixelsToFileZeroDelta() { views. */ Containers::String expectedFilename = Utility::Path::join(DEBUGTOOLS_TEST_DIR, "CompareImageExpected.tga"); - CORRADE_COMPARE_WITH(ExpectedRgb.pixels(), expectedFilename, + /* Same as ExpectedRGB but with pixel format being different */ + const ImageView2D expected{ + PixelStorage{}.setSkip({1, 0, 0}).setRowLength(3), + pixelFormat(PixelFormat::RGB8Unorm, 3, data.srgb), + {2, 2}, ExpectedRgbData}; + + CORRADE_COMPARE_WITH(expected.pixels(), expectedFilename, (CompareImageToFile{*_importerManager, 40.0f, 20.0f})); /* No diagnostic as there's no error */ TestSuite::Comparator compare{&*_importerManager, nullptr, 40.0f, 20.0f}; - CORRADE_COMPARE(compare(ExpectedRgb.pixels(), expectedFilename), + CORRADE_COMPARE(compare(expected.pixels(), expectedFilename), TestSuite::ComparisonStatusFlags{}); } @@ -1756,6 +1935,33 @@ void CompareImageTest::pixelsToFileNonZeroDelta() { CORRADE_COMPARE(out.str(), ImageCompareVerbose); } +void CompareImageTest::pixelsToFileDifferentFormat() { + if(!(_importerManager->loadState("AnyImageImporter") & PluginManager::LoadState::Loaded) || + !(_importerManager->loadState("TgaImporter") & PluginManager::LoadState::Loaded)) + CORRADE_SKIP("AnyImageImporter / TgaImporter plugins not found."); + + /* The filenames are referenced as string views as the assumption is that + the whole comparison and diagnostic printing gets done in a single + expression. Thus don't pass them as temporaries to avoid dangling + views. */ + Containers::String expectedFilename = Utility::Path::join(DEBUGTOOLS_TEST_DIR, "CompareImageExpected.tga"); + + std::stringstream out; + + { + TestSuite::Comparator compare{&*_importerManager, nullptr, {}, {}}; + TestSuite::ComparisonStatusFlags flags = + compare(ExpectedRgb.pixels>(), expectedFilename); + /* Diagnostic but we're not checking it, here we just want to make sure + the right format gets detected */ + CORRADE_COMPARE(flags, TestSuite::ComparisonStatusFlag::Failed|TestSuite::ComparisonStatusFlag::Diagnostic); + Debug d{&out, Debug::Flag::DisableColors}; + compare.printMessage(flags, d, "a", "b"); + } + + CORRADE_COMPARE(out.str(), "Images a and b have different format, actual PixelFormat::RGB8Snorm but PixelFormat::RGB8Unorm expected.\n"); +} + void CompareImageTest::pixelsToFileError() { /* Same as imageToFileError(), but taking pixels instead */ @@ -1772,7 +1978,10 @@ void CompareImageTest::pixelsToFileError() { std::stringstream out; TestSuite::Comparator compare{&*_importerManager, &*_converterManager, 20.0f, 10.0f}; - TestSuite::ComparisonStatusFlags flags = compare(ActualRgb.pixels(), expectedFilename); + /* Vector3ub gets matched to PixelFormat::R8UI initially, but once the + expected image is loaded it gets updated to PixelFormat::R8Unorm to + match it */ + TestSuite::ComparisonStatusFlags flags = compare(ActualRgb.pixels(), expectedFilename); /* The diagnostic flag should be slapped on the failure coming from the operator() comparing two ImageViews */ CORRADE_COMPARE(flags, TestSuite::ComparisonStatusFlag::Failed|TestSuite::ComparisonStatusFlag::Diagnostic); @@ -1809,6 +2018,52 @@ void CompareImageTest::pixelsToFileError() { Utility::Path::join(DEBUGTOOLS_TEST_DIR, "CompareImageActual.tga"), TestSuite::Compare::File); } +void CompareImageTest::pixelsToFileExpectedLoadFailed() { + /* Same as imageToFileExpectedLoadFailed(), but taking pixels instead */ + + if(!(_importerManager->loadState("AnyImageImporter") & PluginManager::LoadState::Loaded) || + !(_importerManager->loadState("TgaImporter") & PluginManager::LoadState::Loaded)) + CORRADE_SKIP("AnyImageImporter / TgaImporter plugins not found."); + + std::stringstream out; + + TestSuite::Comparator compare{&*_importerManager, &*_converterManager, 20.0f, 10.0f}; + TestSuite::ComparisonStatusFlags flags = compare(ActualRgb.pixels(), "nonexistent.tga"); + /* Actual file *could* be loaded, so save it! */ + CORRADE_COMPARE(flags, TestSuite::ComparisonStatusFlag::Failed|TestSuite::ComparisonStatusFlag::Diagnostic); + + { + Debug d{&out, Debug::Flag::DisableColors}; + compare.printMessage(flags, d, "a", "b"); + } + + CORRADE_COMPARE(out.str(), "Expected image b (nonexistent.tga) could not be loaded.\n"); + + /* Create the output dir if it doesn't exist, but avoid stale files making + false positives */ + CORRADE_VERIFY(Utility::Path::make(COMPAREIMAGETEST_SAVE_DIR)); + Containers::String filename = Utility::Path::join(COMPAREIMAGETEST_SAVE_DIR, "nonexistent.tga"); + if(Utility::Path::exists(filename)) + CORRADE_VERIFY(Utility::Path::remove(filename)); + + if(!(_converterManager->loadState("AnyImageConverter") & PluginManager::LoadState::Loaded) || + !(_converterManager->loadState("TgaImageConverter") & PluginManager::LoadState::Loaded)) + CORRADE_SKIP("AnyImageConverter / TgaImageConverter plugins not found."); + + { + out.str({}); + Debug redirectOutput(&out); + compare.saveDiagnostic(flags, redirectOutput, COMPAREIMAGETEST_SAVE_DIR); + } + + /* We expect the *actual* contents, but under the *expected* filename. + Comparing file contents, expecting the converter makes exactly the same + file. */ + CORRADE_COMPARE(out.str(), Utility::formatString("-> {}\n", filename)); + CORRADE_COMPARE_AS(filename, + Utility::Path::join(DEBUGTOOLS_TEST_DIR, "CompareImageActual.tga"), TestSuite::Compare::File); +} + }}}} CORRADE_TEST_MAIN(Magnum::DebugTools::Test::CompareImageTest)