From 588c17fa18db3ae27622f9334c260415a8067916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 23 Jan 2021 15:58:26 +0100 Subject: [PATCH] Vk: make the PixelFormat -> ImageAspect converter public. And, because now we need it for testing and general debugging, add a debug output operator for ImageAspect[s] as well. --- src/Magnum/Vk/Image.cpp | 51 ++++++++++++++++++++++++++++ src/Magnum/Vk/Image.h | 34 +++++++++++++++++-- src/Magnum/Vk/ImageView.cpp | 35 ++++--------------- src/Magnum/Vk/Test/ImageTest.cpp | 51 +++++++++++++++++++++++++++- src/Magnum/Vk/Test/ImageViewTest.cpp | 39 ++++++++------------- 5 files changed, 155 insertions(+), 55 deletions(-) diff --git a/src/Magnum/Vk/Image.cpp b/src/Magnum/Vk/Image.cpp index 38c14e382..b4c2d1aa5 100644 --- a/src/Magnum/Vk/Image.cpp +++ b/src/Magnum/Vk/Image.cpp @@ -26,6 +26,8 @@ #include "Image.h" #include "ImageCreateInfo.h" +#include + #include "Magnum/Vk/Assert.h" #include "Magnum/Vk/Device.h" #include "Magnum/Vk/DeviceProperties.h" @@ -65,6 +67,55 @@ ImageCreateInfo::ImageCreateInfo(const VkImageCreateInfo& info): member instead of doing a copy */ _info(info) {} +Debug& operator<<(Debug& debug, const ImageAspect value) { + debug << "Vk::ImageAspect" << Debug::nospace; + + switch(value) { + /* LCOV_EXCL_START */ + #define _c(value) case Vk::ImageAspect::value: return debug << "::" << Debug::nospace << #value; + _c(Color) + _c(Depth) + _c(Stencil) + #undef _c + /* LCOV_EXCL_STOP */ + } + + /* Flag bits should be in hex, unlike plain values */ + return debug << "(" << Debug::nospace << reinterpret_cast(UnsignedInt(value)) << Debug::nospace << ")"; +} + +Debug& operator<<(Debug& debug, const ImageAspects value) { + return Containers::enumSetDebugOutput(debug, value, "Vk::ImageAspects{}", { + Vk::ImageAspect::Color, + Vk::ImageAspect::Depth, + Vk::ImageAspect::Stencil}); +} + +/* Vulkan, it would kill you if 0 was a valid default, right?! ffs */ +ImageAspects imageAspectsFor(const PixelFormat format) { + /** @todo expand somehow to catch any invalid values? */ + CORRADE_ASSERT(Int(format), "Vk::imageAspectsFor(): can't get an aspect for" << format, {}); + + if(format == PixelFormat::Depth16UnormStencil8UI || + format == PixelFormat::Depth24UnormStencil8UI || + format == PixelFormat::Depth32FStencil8UI) + return ImageAspect::Depth|ImageAspect::Stencil; + if(format == PixelFormat::Depth16Unorm || + format == PixelFormat::Depth24Unorm || + format == PixelFormat::Depth32F) + return ImageAspect::Depth; + if(format == PixelFormat::Stencil8UI) + return ImageAspect::Stencil; + + /** @todo planar formats */ + + return ImageAspect::Color; +} + +ImageAspects imageAspectsFor(const Magnum::PixelFormat format) { + return imageAspectsFor(pixelFormat(format)); +} + Image Image::wrap(Device& device, const VkImage handle, const PixelFormat format, const HandleFlags flags) { Image out{NoCreate}; out._device = &device; diff --git a/src/Magnum/Vk/Image.h b/src/Magnum/Vk/Image.h index 84945e9af..707a34c05 100644 --- a/src/Magnum/Vk/Image.h +++ b/src/Magnum/Vk/Image.h @@ -153,7 +153,8 @@ enum class ImageLayout: Int { @m_since_latest Wraps @type_vk_keyword{ImageAspectFlagBits}. -@see @ref ImageAspects, @ref ImageViewCreateInfo::ImageViewCreateInfo() +@see @ref ImageAspects, @ref imageAspectsFor(), + @ref ImageViewCreateInfo::ImageViewCreateInfo() @m_enum_values_as_keywords */ enum class ImageAspect: UnsignedInt { @@ -164,17 +165,46 @@ enum class ImageAspect: UnsignedInt { /** @todo metadata (sparse?), YCbCr properties */ }; +/** +@debugoperatorenum{ImageAspect} +@m_since_latest +*/ +MAGNUM_VK_EXPORT Debug& operator<<(Debug& out, ImageAspect value); + /** @brief Image aspects @m_since_latest Type-safe wrapper for @type_vk_keyword{ImageAspectFlags}. -@see @ref ImageViewCreateInfo::ImageViewCreateInfo() +@see @ref imageAspectsFor(), @ref ImageViewCreateInfo::ImageViewCreateInfo() */ typedef Containers::EnumSet ImageAspects; CORRADE_ENUMSET_OPERATORS(ImageAspects) +/** +@debugoperatorenum{ImageAspects} +@m_since_latest +*/ +MAGNUM_VK_EXPORT Debug& operator<<(Debug& out, ImageAspects value); + +/** +@brief Image aspects corresponding to given pixel format +@m_since_latest + +Returns @ref ImageAspect::Depth for a depth format, @ref ImageAspect::Stencil +for a stencil format, a combination of both for a combined depth/stencil format +and @ref ImageAspect::Color otherwise. Expects that the format is not +undefined. +*/ +MAGNUM_VK_EXPORT ImageAspects imageAspectsFor(PixelFormat format); + +/** +@overload +@m_since_latest +*/ +MAGNUM_VK_EXPORT ImageAspects imageAspectsFor(Magnum::PixelFormat format); + /** @brief Image @m_since_latest diff --git a/src/Magnum/Vk/ImageView.cpp b/src/Magnum/Vk/ImageView.cpp index 9618ea9c6..9652c0180 100644 --- a/src/Magnum/Vk/ImageView.cpp +++ b/src/Magnum/Vk/ImageView.cpp @@ -33,36 +33,13 @@ namespace Magnum { namespace Vk { -namespace { - -/* Vulkan, it would kill you if 0 was a valid default, right?! ffs */ -/** @todo this might be useful elsewhere as well */ -ImageAspects aspectFor(const PixelFormat format) { - if(format == PixelFormat::Depth16UnormStencil8UI || - format == PixelFormat::Depth24UnormStencil8UI || - format == PixelFormat::Depth32FStencil8UI) - return ImageAspect::Depth|ImageAspect::Stencil; - if(format == PixelFormat::Depth16Unorm || - format == PixelFormat::Depth24Unorm || - format == PixelFormat::Depth32F) - return ImageAspect::Depth; - if(format == PixelFormat::Stencil8UI) - return ImageAspect::Stencil; - - /** @todo planar formats */ - - return ImageAspect::Color; -} - -} - ImageViewCreateInfo::ImageViewCreateInfo(const VkImageViewType type, const VkImage image, const PixelFormat format, const UnsignedInt layerOffset, const UnsignedInt layerCount, const UnsignedInt levelOffset, const UnsignedInt levelCount, const Flags flags): _info{} { _info.sType = VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO; _info.flags = VkImageViewCreateFlags(flags); _info.image = image; _info.viewType = type; _info.format = VkFormat(format); - _info.subresourceRange.aspectMask = VkImageAspectFlags(aspectFor(format)); + _info.subresourceRange.aspectMask = VkImageAspectFlags(imageAspectsFor(format)); _info.subresourceRange.baseMipLevel = levelOffset; _info.subresourceRange.levelCount = levelCount; _info.subresourceRange.baseArrayLayer = layerOffset; @@ -73,10 +50,12 @@ ImageViewCreateInfo::ImageViewCreateInfo(const VkImageViewType type, const VkIma ImageViewCreateInfo::ImageViewCreateInfo(const VkImageViewType type, const VkImage image, const Magnum::CompressedPixelFormat format, const UnsignedInt layerOffset, const UnsignedInt layerCount, const UnsignedInt levelOffset, const UnsignedInt levelCount, const Flags flags): ImageViewCreateInfo{type, image, pixelFormat(format), layerOffset, layerCount, levelOffset, levelCount, flags} {} -ImageViewCreateInfo::ImageViewCreateInfo(const VkImageViewType type, Image& image, const UnsignedInt layerOffset, const UnsignedInt layerCount, const UnsignedInt levelOffset, const UnsignedInt levelCount, const Flags flags): ImageViewCreateInfo{type, image, image.format(), layerOffset, layerCount, levelOffset, levelCount, flags} { - CORRADE_ASSERT(VkFormat(image.format()), - "Vk::ImageViewCreateInfo: the image has unknown format, you have to specify it explicitly", ); -} +ImageViewCreateInfo::ImageViewCreateInfo(const VkImageViewType type, Image& image, const UnsignedInt layerOffset, const UnsignedInt layerCount, const UnsignedInt levelOffset, const UnsignedInt levelCount, const Flags flags): ImageViewCreateInfo{type, image, + /* Assert here instead of inside the constructor to avoid the + imageAspectsFor() assert on invalid format blow up first */ + (CORRADE_CONSTEXPR_ASSERT(VkFormat(image.format()), + "Vk::ImageViewCreateInfo: the image has unknown format, you have to specify it explicitly"), image.format()), + layerOffset, layerCount, levelOffset, levelCount, flags} {} ImageViewCreateInfo::ImageViewCreateInfo(NoInitT) noexcept {} diff --git a/src/Magnum/Vk/Test/ImageTest.cpp b/src/Magnum/Vk/Test/ImageTest.cpp index 49e36d516..f5f32b6c4 100644 --- a/src/Magnum/Vk/Test/ImageTest.cpp +++ b/src/Magnum/Vk/Test/ImageTest.cpp @@ -51,10 +51,17 @@ struct ImageTest: TestSuite::Tester { void createInfoConstructNoInit(); void createInfoConstructFromVk(); + void aspectsFor(); + void aspectsForInvalidFormat(); + void aspectsForGenericFormat(); + void constructNoCreate(); void constructCopy(); void dedicatedMemoryNotDedicated(); + + void debugAspect(); + void debugAspects(); }; ImageTest::ImageTest() { @@ -85,10 +92,17 @@ ImageTest::ImageTest() { &ImageTest::createInfoConstructNoInit, &ImageTest::createInfoConstructFromVk, + &ImageTest::aspectsFor, + &ImageTest::aspectsForInvalidFormat, + &ImageTest::aspectsForGenericFormat, + &ImageTest::constructNoCreate, &ImageTest::constructCopy, - &ImageTest::dedicatedMemoryNotDedicated}); + &ImageTest::dedicatedMemoryNotDedicated, + + &ImageTest::debugAspect, + &ImageTest::debugAspects}); } template void ImageTest::createInfoConstruct() { @@ -226,6 +240,29 @@ void ImageTest::createInfoConstructFromVk() { CORRADE_COMPARE(info->sType, VK_STRUCTURE_TYPE_FORMAT_PROPERTIES_2); } +void ImageTest::aspectsFor() { + CORRADE_COMPARE(imageAspectsFor(PixelFormat::RGBA8Unorm), ImageAspect::Color); + CORRADE_COMPARE(imageAspectsFor(PixelFormat::Depth32FStencil8UI), ImageAspect::Depth|ImageAspect::Stencil); + CORRADE_COMPARE(imageAspectsFor(PixelFormat::Depth16Unorm), ImageAspect::Depth); + CORRADE_COMPARE(imageAspectsFor(PixelFormat::Stencil8UI), ImageAspect::Stencil); +} + +void ImageTest::aspectsForInvalidFormat() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + std::ostringstream out; + Error redirectError{&out}; + imageAspectsFor(PixelFormat{}); + CORRADE_COMPARE(out.str(), "Vk::imageAspectsFor(): can't get an aspect for Vk::PixelFormat(0)\n"); +} + +void ImageTest::aspectsForGenericFormat() { + /* No generic depth/stencil formats yet, can't test */ + CORRADE_COMPARE(imageAspectsFor(Magnum::PixelFormat::R16I), ImageAspect::Color); +} + void ImageTest::constructNoCreate() { { Image image{NoCreate}; @@ -255,6 +292,18 @@ void ImageTest::dedicatedMemoryNotDedicated() { CORRADE_COMPARE(out.str(), "Vk::Image::dedicatedMemory(): image doesn't have a dedicated memory\n"); } +void ImageTest::debugAspect() { + std::ostringstream out; + Debug{&out} << ImageAspect::Depth << ImageAspect(0xdeadcafe); + CORRADE_COMPARE(out.str(), "Vk::ImageAspect::Depth Vk::ImageAspect(0xdeadcafe)\n"); +} + +void ImageTest::debugAspects() { + std::ostringstream out; + Debug{&out} << (ImageAspect::Stencil|ImageAspect(0xf0)) << ImageAspects{}; + CORRADE_COMPARE(out.str(), "Vk::ImageAspect::Stencil|Vk::ImageAspect(0xf0) Vk::ImageAspects{}\n"); +} + }}}} CORRADE_TEST_MAIN(Magnum::Vk::Test::ImageTest) diff --git a/src/Magnum/Vk/Test/ImageViewTest.cpp b/src/Magnum/Vk/Test/ImageViewTest.cpp index eb749ec3f..9579788e9 100644 --- a/src/Magnum/Vk/Test/ImageViewTest.cpp +++ b/src/Magnum/Vk/Test/ImageViewTest.cpp @@ -45,7 +45,7 @@ struct ImageViewTest: TestSuite::Tester { template void createInfoConstruct1D(); void createInfoConstruct1DFromImage(); template void createInfoConstruct2D(); - void createInfoConstruct2DAspect(); + void createInfoConstruct2DDepth(); void createInfoConstruct2DFromImage(); template void createInfoConstruct3D(); void createInfoConstruct3DFromImage(); @@ -64,17 +64,6 @@ struct ImageViewTest: TestSuite::Tester { void constructCopy(); }; -const struct { - const char* name; - PixelFormat format; - VkImageAspectFlags aspect; -} View2DFormatData[] { - {"color", PixelFormat::RGBA8Unorm, VK_IMAGE_ASPECT_COLOR_BIT}, - {"depth + stencil", PixelFormat::Depth32FStencil8UI, VK_IMAGE_ASPECT_DEPTH_BIT|VK_IMAGE_ASPECT_STENCIL_BIT}, - {"depth", PixelFormat::Depth16Unorm, VK_IMAGE_ASPECT_DEPTH_BIT}, - {"stencil", PixelFormat::Stencil8UI, VK_IMAGE_ASPECT_STENCIL_BIT} -}; - ImageViewTest::ImageViewTest() { addTests({&ImageViewTest::createInfoConstruct, &ImageViewTest::createInfoConstruct, @@ -87,12 +76,10 @@ ImageViewTest::ImageViewTest() { &ImageViewTest::createInfoConstruct1DFromImage, &ImageViewTest::createInfoConstruct2D, &ImageViewTest::createInfoConstruct2D, - &ImageViewTest::createInfoConstruct2D}); - - addInstancedTests({&ImageViewTest::createInfoConstruct2DAspect}, - Containers::arraySize(View2DFormatData)); + &ImageViewTest::createInfoConstruct2D, + &ImageViewTest::createInfoConstruct2DDepth, - addTests({&ImageViewTest::createInfoConstruct2DFromImage, + &ImageViewTest::createInfoConstruct2DFromImage, &ImageViewTest::createInfoConstruct3D, &ImageViewTest::createInfoConstruct3D, &ImageViewTest::createInfoConstruct3D, @@ -165,7 +152,11 @@ void ImageViewTest::createInfoConstructFromImageFormatUknown() { Error redirectError{&out}; ImageViewCreateInfo{VK_IMAGE_VIEW_TYPE_2D, image}; CORRADE_COMPARE(out.str(), - "Vk::ImageViewCreateInfo: the image has unknown format, you have to specify it explicitly\n"); + "Vk::ImageViewCreateInfo: the image has unknown format, you have to specify it explicitly\n" + /* The second assert won't appear for the user, it's here only because + the graceful assert can'ลง do an early exist in a delegeated + constructor call */ + "Vk::imageAspectsFor(): can't get an aspect for Vk::PixelFormat(0)\n"); } template void ImageViewTest::createInfoConstruct1D() { @@ -217,13 +208,13 @@ template void ImageViewTest::createInfoConstruct2D() { CORRADE_COMPARE(info->subresourceRange.levelCount, 9); } -void ImageViewTest::createInfoConstruct2DAspect() { - auto&& data = View2DFormatData[testCaseInstanceId()]; - setTestCaseDescription(data.name); +void ImageViewTest::createInfoConstruct2DDepth() { + /* Just to verify proper aspect is chosen. The rest is tested in + ImageTest::aspectFor() */ - ImageViewCreateInfo2D info{imageHandle, data.format}; - CORRADE_COMPARE(info->format, VkFormat(data.format)); - CORRADE_COMPARE(info->subresourceRange.aspectMask, data.aspect); + ImageViewCreateInfo2D info{imageHandle, PixelFormat::Depth24Unorm}; + CORRADE_COMPARE(info->format, VK_FORMAT_X8_D24_UNORM_PACK32); + CORRADE_COMPARE(info->subresourceRange.aspectMask, VK_IMAGE_ASPECT_DEPTH_BIT); } void ImageViewTest::createInfoConstruct2DFromImage() {