From 332bcb62fa8099b39ae83fb09a254fdc3d8f6b7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 11 May 2023 12:44:49 +0200 Subject: [PATCH] It makes no sense to call isPixelFormatSrgb() on a depth/stencil format. Doing so usually points to some error in user code, so directly disallow it, instead of returning false. Such behavior is consistent with pixelFormatChannelCount() and other helper APIs. --- src/Magnum/PixelFormat.cpp | 3 ++- src/Magnum/PixelFormat.h | 3 ++- src/Magnum/Test/PixelFormatTest.cpp | 12 +++++++----- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/Magnum/PixelFormat.cpp b/src/Magnum/PixelFormat.cpp index 40afec253..52d5fa2dc 100644 --- a/src/Magnum/PixelFormat.cpp +++ b/src/Magnum/PixelFormat.cpp @@ -352,6 +352,7 @@ bool isPixelFormatSrgb(const PixelFormat format) { case PixelFormat::RG32F: case PixelFormat::RGB32F: case PixelFormat::RGBA32F: + return false; case PixelFormat::Depth16Unorm: case PixelFormat::Depth24Unorm: case PixelFormat::Depth32F: @@ -359,7 +360,7 @@ bool isPixelFormatSrgb(const PixelFormat format) { case PixelFormat::Depth16UnormStencil8UI: case PixelFormat::Depth24UnormStencil8UI: case PixelFormat::Depth32FStencil8UI: - return false; + CORRADE_ASSERT_UNREACHABLE("isPixelFormatSrgb(): can't determine colorspace of" << format, {}); } #ifdef CORRADE_TARGET_GCC #pragma GCC diagnostic pop diff --git a/src/Magnum/PixelFormat.h b/src/Magnum/PixelFormat.h index e80005996..70f9b1406 100644 --- a/src/Magnum/PixelFormat.h +++ b/src/Magnum/PixelFormat.h @@ -814,7 +814,8 @@ MAGNUM_EXPORT UnsignedInt pixelFormatChannelCount(PixelFormat format); @m_since_latest Returns @cpp true @ce for `*Srgb` formats, @cpp false @ce otherwise. Expects -that the pixel format is *not* implementation-specific. +that the pixel format is *not* implementation-specific and not a +depth/stencil format. @see @ref isPixelFormatImplementationSpecific() */ MAGNUM_EXPORT bool isPixelFormatSrgb(PixelFormat format); diff --git a/src/Magnum/Test/PixelFormatTest.cpp b/src/Magnum/Test/PixelFormatTest.cpp index e11b9149e..f4c0e9b92 100644 --- a/src/Magnum/Test/PixelFormatTest.cpp +++ b/src/Magnum/Test/PixelFormatTest.cpp @@ -50,7 +50,7 @@ struct PixelFormatTest: TestSuite::Tester { void isSrgb(); void isSrgbInvalid(); - void isSrgbImplementationSpecific(); + void isSrgbDepthStencilImplementationSpecific(); void isDepthOrStencil(); void isDepthOrStencilInvalid(); @@ -100,7 +100,7 @@ PixelFormatTest::PixelFormatTest() { &PixelFormatTest::isSrgb, &PixelFormatTest::isSrgbInvalid, - &PixelFormatTest::isSrgbImplementationSpecific, + &PixelFormatTest::isSrgbDepthStencilImplementationSpecific, &PixelFormatTest::isDepthOrStencil, &PixelFormatTest::isDepthOrStencilInvalid, @@ -319,8 +319,8 @@ void PixelFormatTest::channelFormatCountDepthStencilImplementationSpecific() { void PixelFormatTest::isSrgb() { CORRADE_VERIFY(isPixelFormatSrgb(PixelFormat::RG8Srgb)); + CORRADE_VERIFY(!isPixelFormatSrgb(PixelFormat::RG8Snorm)); CORRADE_VERIFY(!isPixelFormatSrgb(PixelFormat::RGB16F)); - CORRADE_VERIFY(!isPixelFormatSrgb(PixelFormat::Stencil8UI)); } void PixelFormatTest::isSrgbInvalid() { @@ -335,14 +335,16 @@ void PixelFormatTest::isSrgbInvalid() { "isPixelFormatSrgb(): invalid format PixelFormat(0xdead)\n"); } -void PixelFormatTest::isSrgbImplementationSpecific() { +void PixelFormatTest::isSrgbDepthStencilImplementationSpecific() { CORRADE_SKIP_IF_NO_ASSERT(); std::ostringstream out; Error redirectError{&out}; isPixelFormatSrgb(pixelFormatWrap(0xdead)); + isPixelFormatSrgb(PixelFormat::Depth16Unorm); CORRADE_COMPARE(out.str(), - "isPixelFormatSrgb(): can't determine colorspace of an implementation-specific format 0xdead\n"); + "isPixelFormatSrgb(): can't determine colorspace of an implementation-specific format 0xdead\n" + "isPixelFormatSrgb(): can't determine colorspace of PixelFormat::Depth16Unorm\n"); } void PixelFormatTest::isDepthOrStencil() {