diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 4c36c662030..7665aab6a95 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -14,6 +14,7 @@ A new header is inserted each time a *tag* is created. - gltf_viewer: Exercise picking functionality. - OpenGL: add WebGL support for ReadPixels - Vulkan: add assert and error message for OOM (debug builds) +- Vulkan: fix crash with picking and 2-component ReadPixels. ## v1.23.2 diff --git a/filament/backend/src/DataReshaper.h b/filament/backend/src/DataReshaper.h index 6308fb9c4ef..3be41181acf 100644 --- a/filament/backend/src/DataReshaper.h +++ b/filament/backend/src/DataReshaper.h @@ -58,26 +58,16 @@ class DataReshaper { } } - // Converts a 4-channel image of UBYTE, INT, UINT, or FLOAT to a different type. + // Converts a n-channel image of UBYTE, INT, UINT, or FLOAT to a different type. template static void reshapeImage(uint8_t* dest, const uint8_t* src, size_t srcBytesPerRow, - size_t dstBytesPerRow, size_t dstChannelCount, size_t height, bool swizzle, bool flip) { - const size_t srcChannelCount = 4; + size_t srcChannelCount, size_t dstBytesPerRow, size_t dstChannelCount, + size_t width, size_t height, bool swizzle) { const dstComponentType dstMaxValue = getMaxValue(); const srcComponentType srcMaxValue = getMaxValue(); - const size_t width = (srcBytesPerRow / sizeof(srcComponentType)) / srcChannelCount; const size_t minChannelCount = filament::math::min(srcChannelCount, dstChannelCount); assert_invariant(minChannelCount <= 4); const int inds[4] = {swizzle ? 2 : 0, 1, swizzle ? 0 : 2, 3}; - - int srcStride; - if (flip) { - src += srcBytesPerRow * (height - 1); - srcStride = -srcBytesPerRow; - } else { - srcStride = srcBytesPerRow; - } - for (size_t row = 0; row < height; ++row) { const srcComponentType* in = (const srcComponentType*) src; dstComponentType* out = (dstComponentType*) dest; @@ -97,15 +87,15 @@ class DataReshaper { in += srcChannelCount; out += dstChannelCount; } - src += srcStride; + src += srcBytesPerRow; dest += dstBytesPerRow; } } - // Converts a 4-channel image of UBYTE, INT, UINT, or FLOAT to a different type. + // Converts a n-channel image of UBYTE, INT, UINT, or FLOAT to a different type. static bool reshapeImage(PixelBufferDescriptor* dst, PixelDataType srcType, - const uint8_t* srcBytes, int srcBytesPerRow, int width, int height, bool swizzle, - bool flip) { + uint32_t srcChannelCount, const uint8_t* srcBytes, int srcBytesPerRow, int width, + int height, bool swizzle) { size_t dstChannelCount; switch (dst->format) { case PixelDataFormat::R_INTEGER: dstChannelCount = 1; break; @@ -118,8 +108,9 @@ class DataReshaper { case PixelDataFormat::RGBA: dstChannelCount = 4; break; default: return false; } - void (*reshaper)(uint8_t*, const uint8_t*, size_t, size_t, size_t, size_t, bool, bool) - = nullptr; + void (*reshaper)(uint8_t* dest, const uint8_t* src, size_t srcBytesPerRow, + size_t srcChannelCount, size_t dstBytesPerRow, size_t dstChannelCount, + size_t width, size_t height, bool swizzle) = nullptr; constexpr auto UBYTE = PixelDataType::UBYTE, FLOAT = PixelDataType::FLOAT, UINT = PixelDataType::UINT, INT = PixelDataType::INT; switch (dst->type) { @@ -165,11 +156,10 @@ class DataReshaper { uint8_t* dstBytes = (uint8_t*) dst->buffer; const int dstBytesPerRow = PixelBufferDescriptor::computeDataSize(dst->format, dst->type, dst->stride ? dst->stride : width, 1, dst->alignment); - reshaper(dstBytes, srcBytes, srcBytesPerRow, dstBytesPerRow, dstChannelCount, height, - swizzle, flip); + reshaper(dstBytes, srcBytes, srcBytesPerRow, srcChannelCount, dstBytesPerRow, + dstChannelCount, width, height, swizzle); return true; } - }; template<> inline float getMaxValue() { return 1.0f; } diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 1d0ace1436b..6e3473c1613 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -1639,8 +1639,8 @@ void VulkanDriver::readPixels(Handle src, uint32_t x, uint32_t y vkMapMemory(device, stagingMemory, 0, VK_WHOLE_SIZE, 0, (void**) &srcPixels); srcPixels += subResourceLayout.offset; - if (!DataReshaper::reshapeImage(&pbd, getComponentType(srcFormat), srcPixels, - subResourceLayout.rowPitch, width, height, swizzle, false)) { + if (!DataReshaper::reshapeImage(&pbd, getComponentType(srcFormat), getComponentCount(srcFormat), + srcPixels, subResourceLayout.rowPitch, width, height, swizzle)) { utils::slog.e << "Unsupported PixelDataFormat or PixelDataType" << utils::io::endl; } diff --git a/filament/backend/src/vulkan/VulkanUtility.cpp b/filament/backend/src/vulkan/VulkanUtility.cpp index c78851343ba..2a3976eb8b5 100644 --- a/filament/backend/src/vulkan/VulkanUtility.cpp +++ b/filament/backend/src/vulkan/VulkanUtility.cpp @@ -471,6 +471,109 @@ PixelDataType getComponentType(VkFormat format) { return {}; } +uint32_t getComponentCount(VkFormat format) { + switch (format) { + case VK_FORMAT_R8_UNORM: + case VK_FORMAT_R8_SNORM: + case VK_FORMAT_R8_USCALED: + case VK_FORMAT_R8_SSCALED: + case VK_FORMAT_R8_UINT: + case VK_FORMAT_R8_SINT: + case VK_FORMAT_R8_SRGB: + case VK_FORMAT_R16_UNORM: + case VK_FORMAT_R16_SNORM: + case VK_FORMAT_R16_USCALED: + case VK_FORMAT_R16_SSCALED: + case VK_FORMAT_R16_UINT: + case VK_FORMAT_R16_SINT: + case VK_FORMAT_R16_SFLOAT: + case VK_FORMAT_R32_UINT: + case VK_FORMAT_R32_SINT: + case VK_FORMAT_R32_SFLOAT: + return 1; + + case VK_FORMAT_R8G8_UNORM: + case VK_FORMAT_R8G8_SNORM: + case VK_FORMAT_R8G8_USCALED: + case VK_FORMAT_R8G8_SSCALED: + case VK_FORMAT_R8G8_UINT: + case VK_FORMAT_R8G8_SINT: + case VK_FORMAT_R8G8_SRGB: + case VK_FORMAT_R16G16_UNORM: + case VK_FORMAT_R16G16_SNORM: + case VK_FORMAT_R16G16_USCALED: + case VK_FORMAT_R16G16_SSCALED: + case VK_FORMAT_R16G16_UINT: + case VK_FORMAT_R16G16_SINT: + case VK_FORMAT_R16G16_SFLOAT: + case VK_FORMAT_R32G32_UINT: + case VK_FORMAT_R32G32_SINT: + case VK_FORMAT_R32G32_SFLOAT: + return 2; + + case VK_FORMAT_R8G8B8_UNORM: + case VK_FORMAT_R8G8B8_SNORM: + case VK_FORMAT_R8G8B8_USCALED: + case VK_FORMAT_R8G8B8_SSCALED: + case VK_FORMAT_R8G8B8_UINT: + case VK_FORMAT_R8G8B8_SINT: + case VK_FORMAT_R8G8B8_SRGB: + case VK_FORMAT_B8G8R8_UNORM: + case VK_FORMAT_B8G8R8_SNORM: + case VK_FORMAT_B8G8R8_USCALED: + case VK_FORMAT_B8G8R8_SSCALED: + case VK_FORMAT_B8G8R8_UINT: + case VK_FORMAT_B8G8R8_SINT: + case VK_FORMAT_B8G8R8_SRGB: + case VK_FORMAT_R16G16B16_UNORM: + case VK_FORMAT_R16G16B16_SNORM: + case VK_FORMAT_R16G16B16_USCALED: + case VK_FORMAT_R16G16B16_SSCALED: + case VK_FORMAT_R16G16B16_UINT: + case VK_FORMAT_R16G16B16_SINT: + case VK_FORMAT_R16G16B16_SFLOAT: + case VK_FORMAT_R32G32B32_UINT: + case VK_FORMAT_R32G32B32_SINT: + case VK_FORMAT_R32G32B32_SFLOAT: + return 3; + + case VK_FORMAT_R8G8B8A8_UNORM: + case VK_FORMAT_R8G8B8A8_SNORM: + case VK_FORMAT_R8G8B8A8_USCALED: + case VK_FORMAT_R8G8B8A8_SSCALED: + case VK_FORMAT_R8G8B8A8_UINT: + case VK_FORMAT_R8G8B8A8_SINT: + case VK_FORMAT_R8G8B8A8_SRGB: + case VK_FORMAT_B8G8R8A8_UNORM: + case VK_FORMAT_B8G8R8A8_SNORM: + case VK_FORMAT_B8G8R8A8_USCALED: + case VK_FORMAT_B8G8R8A8_SSCALED: + case VK_FORMAT_B8G8R8A8_UINT: + case VK_FORMAT_B8G8R8A8_SINT: + case VK_FORMAT_B8G8R8A8_SRGB: + case VK_FORMAT_A8B8G8R8_UNORM_PACK32: + case VK_FORMAT_A8B8G8R8_SNORM_PACK32: + case VK_FORMAT_A8B8G8R8_USCALED_PACK32: + case VK_FORMAT_A8B8G8R8_SSCALED_PACK32: + case VK_FORMAT_A8B8G8R8_UINT_PACK32: + case VK_FORMAT_A8B8G8R8_SINT_PACK32: + case VK_FORMAT_A8B8G8R8_SRGB_PACK32: + case VK_FORMAT_R16G16B16A16_UNORM: + case VK_FORMAT_R16G16B16A16_SNORM: + case VK_FORMAT_R16G16B16A16_USCALED: + case VK_FORMAT_R16G16B16A16_SSCALED: + case VK_FORMAT_R16G16B16A16_UINT: + case VK_FORMAT_R16G16B16A16_SINT: + case VK_FORMAT_R16G16B16A16_SFLOAT: + case VK_FORMAT_R32G32B32A32_UINT: + case VK_FORMAT_R32G32B32A32_SINT: + case VK_FORMAT_R32G32B32A32_SFLOAT: + return 4; + default: assert_invariant(false && "Unknown data type, conversion is not supported."); + } + return {}; +} + VkComponentMapping getSwizzleMap(TextureSwizzle swizzle[4]) { VkComponentMapping map; VkComponentSwizzle* dst = &map.r; diff --git a/filament/backend/src/vulkan/VulkanUtility.h b/filament/backend/src/vulkan/VulkanUtility.h index 560e257b63c..7e0c0ed669d 100644 --- a/filament/backend/src/vulkan/VulkanUtility.h +++ b/filament/backend/src/vulkan/VulkanUtility.h @@ -45,6 +45,7 @@ VkBlendFactor getBlendFactor(BlendFunction mode); VkCullModeFlags getCullMode(CullingMode mode); VkFrontFace getFrontFace(bool inverseFrontFaces); PixelDataType getComponentType(VkFormat format); +uint32_t getComponentCount(VkFormat format); VkComponentMapping getSwizzleMap(TextureSwizzle swizzle[4]); VkImageViewType getImageViewType(SamplerType target); VkImageLayout getDefaultImageLayout(TextureUsage usage);