From f30a191978581097b90962a617b434d19d64704a Mon Sep 17 00:00:00 2001 From: Marcin Hajder Date: Tue, 27 May 2025 19:17:14 +0200 Subject: [PATCH] Corrected procedure to collect proper size of image from VkImageCreateInfo (#2289) Fixes #2155 according to issue description Additional remark: The image size was previously calculated based on the memory size, which seems unusual. Due to Vulkan's configuration capabilities, the size of memory allocated for a specific texture may differ from what would be expected based on the texture dimensions. Thus, calculating the image dimensions back from the memory size of a Vulkan texture can be challenging. --- .../vulkan_wrapper/opencl_vulkan_wrapper.cpp | 122 +++++++++++++----- .../vulkan_wrapper/opencl_vulkan_wrapper.hpp | 6 +- .../common/vulkan_wrapper/vulkan_wrapper.cpp | 10 +- .../common/vulkan_wrapper/vulkan_wrapper.hpp | 3 +- .../vulkan/test_vulkan_api_consistency.cpp | 17 +-- ...st_vulkan_api_consistency_for_1dimages.cpp | 27 +--- ...st_vulkan_api_consistency_for_3dimages.cpp | 27 +--- 7 files changed, 128 insertions(+), 84 deletions(-) diff --git a/test_conformance/common/vulkan_wrapper/opencl_vulkan_wrapper.cpp b/test_conformance/common/vulkan_wrapper/opencl_vulkan_wrapper.cpp index cbfde8a8..f4245703 100644 --- a/test_conformance/common/vulkan_wrapper/opencl_vulkan_wrapper.cpp +++ b/test_conformance/common/vulkan_wrapper/opencl_vulkan_wrapper.cpp @@ -428,38 +428,68 @@ size_t GetElementNBytes(const cl_image_format *format) return result; } -cl_int get2DImageDimensions(const VkImageCreateInfo *VulkanImageCreateInfo, - cl_image_format *img_fmt, size_t totalImageSize, - size_t &width, size_t &height) +cl_int getImageDimensions(const VkImageCreateInfo *VulkanImageCreateInfo, + cl_image_format *img_fmt, cl_image_desc *img_desc, + VkExtent3D max_ext, const VkSubresourceLayout *layout) { - cl_int result = CL_SUCCESS; - if (totalImageSize == 0) + test_assert_error( + VulkanImageCreateInfo != nullptr, + "getImageDimensions: invalid VulkanImageCreateInfo pointer!"); + test_assert_error(img_fmt != nullptr, + "getImageDimensions: invalid img_fmt pointer!"); + test_assert_error(img_desc != nullptr, + "getImageDimensions: invalid img_desc pointer!"); + + img_desc->image_depth = VulkanImageCreateInfo->extent.depth; + img_desc->image_width = VulkanImageCreateInfo->extent.width; + img_desc->image_height = VulkanImageCreateInfo->extent.height; + + if (layout != nullptr) { - result = CL_INVALID_VALUE; + size_t element_size = GetElementNBytes(img_fmt); + size_t row_pitch = element_size * VulkanImageCreateInfo->extent.width; + row_pitch = row_pitch < layout->rowPitch ? layout->rowPitch : row_pitch; + img_desc->image_row_pitch = row_pitch; + + size_t slice_pitch = row_pitch * VulkanImageCreateInfo->extent.height; + slice_pitch = + slice_pitch < layout->depthPitch ? layout->depthPitch : slice_pitch; + img_desc->image_slice_pitch = slice_pitch; } - size_t element_size = GetElementNBytes(img_fmt); - size_t row_pitch = element_size * VulkanImageCreateInfo->extent.width; - row_pitch = row_pitch % 64 == 0 ? row_pitch : ((row_pitch / 64) + 1) * 64; - width = row_pitch / element_size; - height = totalImageSize / row_pitch; + switch (img_desc->image_type) + { + case CL_MEM_OBJECT_IMAGE3D: + test_assert_error(img_desc->image_depth >= 1 + && img_desc->image_depth <= max_ext.depth, + "getImageDimensions: invalid image depth!"); + case CL_MEM_OBJECT_IMAGE2D: + test_assert_error(img_desc->image_height >= 1 + && img_desc->image_height <= max_ext.height, + "getImageDimensions: invalid image height!"); + case CL_MEM_OBJECT_IMAGE1D: + test_assert_error(img_desc->image_width >= 1 + && img_desc->image_width <= max_ext.width, + "getImageDimensions: invalid image width!"); + } - return result; + return CL_SUCCESS; } cl_int -getCLImageInfoFromVkImageInfo(const VkImageCreateInfo *VulkanImageCreateInfo, - size_t totalImageSize, cl_image_format *img_fmt, - cl_image_desc *img_desc) +getCLImageInfoFromVkImageInfo(const cl_device_id device, + const VkImageCreateInfo *VulkanImageCreateInfo, + cl_image_format *img_fmt, cl_image_desc *img_desc, + const VkSubresourceLayout *layout) { - cl_int result = CL_SUCCESS; + cl_int error = CL_SUCCESS; cl_image_format clImgFormat = { 0 }; - result = + error = getCLFormatFromVkFormat(VulkanImageCreateInfo->format, &clImgFormat); - if (CL_SUCCESS != result) + if (CL_SUCCESS != error) { - return result; + return error; } memcpy(img_fmt, &clImgFormat, sizeof(cl_image_format)); @@ -469,25 +499,57 @@ getCLImageInfoFromVkImageInfo(const VkImageCreateInfo *VulkanImageCreateInfo, return CL_INVALID_VALUE; } - result = - get2DImageDimensions(VulkanImageCreateInfo, img_fmt, totalImageSize, - img_desc->image_width, img_desc->image_height); - if (CL_SUCCESS != result) + VkExtent3D max_ext = { 0, 0, 0 }; + + size_t width = 0, height = 0, depth = 0; + if (img_desc->image_type == CL_MEM_OBJECT_IMAGE3D) { - throw std::runtime_error("get2DImageDimensions failed!!!"); + error = clGetDeviceInfo(device, CL_DEVICE_IMAGE3D_MAX_WIDTH, + sizeof(width), &width, NULL); + test_error(error, "Unable to get CL_DEVICE_IMAGE3D_MAX_WIDTH"); + error = clGetDeviceInfo(device, CL_DEVICE_IMAGE3D_MAX_HEIGHT, + sizeof(height), &height, NULL); + test_error(error, "Unable to get CL_DEVICE_IMAGE3D_MAX_HEIGHT"); + error = clGetDeviceInfo(device, CL_DEVICE_IMAGE3D_MAX_DEPTH, + sizeof(depth), &depth, NULL); + test_error(error, "Unable to get CL_DEVICE_IMAGE3D_MAX_DEPTH"); + } + else + { + error = clGetDeviceInfo(device, CL_DEVICE_IMAGE2D_MAX_WIDTH, + sizeof(width), &width, NULL); + test_error(error, "Unable to get CL_DEVICE_IMAGE2D_MAX_WIDTH"); + error = clGetDeviceInfo(device, CL_DEVICE_IMAGE2D_MAX_HEIGHT, + sizeof(height), &height, NULL); + test_error(error, "Unable to get CL_DEVICE_IMAGE2D_MAX_HEIGHT"); + } + + max_ext.depth = depth; + max_ext.height = height; + max_ext.width = width; + + // If image_row_pitch is zero and the image is created from an external + // memory handle, then the image row pitch is implementation-defined + img_desc->image_row_pitch = 0; + // If image_slice_pitch is zero and the image is created from an external + // memory handle, then the image slice pitch is implementation-defined + img_desc->image_slice_pitch = 0; + + error = getImageDimensions(VulkanImageCreateInfo, img_fmt, img_desc, + max_ext, layout); + if (CL_SUCCESS != error) + { + throw std::runtime_error("getImageDimensions failed!!!"); } img_desc->image_depth = static_cast(VulkanImageCreateInfo->extent.depth); img_desc->image_array_size = 0; - img_desc->image_row_pitch = 0; // Row pitch set to zero as host_ptr is NULL - img_desc->image_slice_pitch = - img_desc->image_row_pitch * img_desc->image_height; img_desc->num_mip_levels = 0; img_desc->num_samples = 0; img_desc->buffer = NULL; - return result; + return error; } cl_int check_external_memory_handle_type( @@ -806,7 +868,7 @@ clExternalMemoryImage::clExternalMemoryImage( image2D.getVkImageCreateInfo(); errcode_ret = getCLImageInfoFromVkImageInfo( - &VulkanImageCreateInfo, image2D.getSize(), &img_format, &image_desc); + deviceId, &VulkanImageCreateInfo, &img_format, &image_desc); if (CL_SUCCESS != errcode_ret) { throw std::runtime_error("getCLImageInfoFromVkImageInfo failed\n"); @@ -1212,6 +1274,8 @@ cl_external_memory_handle_type_khr vkToOpenCLExternalMemoryHandleType( case VULKAN_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_WIN32_KMT: case VULKAN_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_WIN32_NT_KMT: return CL_EXTERNAL_MEMORY_HANDLE_OPAQUE_WIN32_KMT_KHR; + case VULKAN_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_WIN32_NT_NAME: + return CL_EXTERNAL_MEMORY_HANDLE_OPAQUE_WIN32_NAME_KHR; } return 0; } diff --git a/test_conformance/common/vulkan_wrapper/opencl_vulkan_wrapper.hpp b/test_conformance/common/vulkan_wrapper/opencl_vulkan_wrapper.hpp index 65364f43..f9a305e1 100644 --- a/test_conformance/common/vulkan_wrapper/opencl_vulkan_wrapper.hpp +++ b/test_conformance/common/vulkan_wrapper/opencl_vulkan_wrapper.hpp @@ -71,8 +71,10 @@ extern pfnclEnqueueReleaseExternalMemObjectsKHR extern pfnclReleaseSemaphoreKHR clReleaseSemaphoreKHRptr; extern pfnclReImportSemaphoreSyncFdKHR pfnclReImportSemaphoreSyncFdKHRptr; -cl_int getCLImageInfoFromVkImageInfo(const VkImageCreateInfo *, size_t, - cl_image_format *, cl_image_desc *); +cl_int +getCLImageInfoFromVkImageInfo(const cl_device_id, const VkImageCreateInfo *, + cl_image_format *, cl_image_desc *, + const VkSubresourceLayout *layout = nullptr); cl_int check_external_memory_handle_type( cl_device_id deviceID, cl_external_memory_handle_type_khr requiredHandleType); diff --git a/test_conformance/common/vulkan_wrapper/vulkan_wrapper.cpp b/test_conformance/common/vulkan_wrapper/vulkan_wrapper.cpp index f41e196d..7254742d 100644 --- a/test_conformance/common/vulkan_wrapper/vulkan_wrapper.cpp +++ b/test_conformance/common/vulkan_wrapper/vulkan_wrapper.cpp @@ -1839,7 +1839,13 @@ VulkanImage::VulkanImage( vkImageCreateInfo.pNext = &vkExternalMemoryImageCreateInfo; } - vkCreateImage(m_device, &vkImageCreateInfo, NULL, &m_vkImage); + VkResult vkStatus = + vkCreateImage(m_device, &vkImageCreateInfo, NULL, &m_vkImage); + if (vkStatus != VK_SUCCESS) + { + throw std::runtime_error("Error: Failed create image."); + } + VulkanImageCreateInfo = vkImageCreateInfo; VkMemoryDedicatedRequirements vkMemoryDedicatedRequirements = {}; @@ -1968,7 +1974,7 @@ VulkanExtent3D VulkanImage2D::getExtent3D(uint32_t mipLevel) const return VulkanExtent3D(width, height, depth); } -VkSubresourceLayout VulkanImage2D::getSubresourceLayout() const +VkSubresourceLayout VulkanImage::getSubresourceLayout() const { VkImageSubresource subresource = { VK_IMAGE_ASPECT_COLOR_BIT, 0, 0 }; diff --git a/test_conformance/common/vulkan_wrapper/vulkan_wrapper.hpp b/test_conformance/common/vulkan_wrapper/vulkan_wrapper.hpp index 2c49caf3..9cb031f5 100644 --- a/test_conformance/common/vulkan_wrapper/vulkan_wrapper.hpp +++ b/test_conformance/common/vulkan_wrapper/vulkan_wrapper.hpp @@ -491,6 +491,8 @@ public: VulkanSharingMode sharingMode = VULKAN_SHARING_MODE_EXCLUSIVE); virtual ~VulkanImage(); virtual VulkanExtent3D getExtent3D(uint32_t mipLevel = 0) const; + virtual VkSubresourceLayout getSubresourceLayout() const; + VulkanFormat getFormat() const; uint32_t getNumMipLevels() const; uint32_t getNumLayers() const; @@ -560,7 +562,6 @@ public: VulkanSharingMode sharingMode = VULKAN_SHARING_MODE_EXCLUSIVE); virtual ~VulkanImage2D(); virtual VulkanExtent3D getExtent3D(uint32_t mipLevel = 0) const; - virtual VkSubresourceLayout getSubresourceLayout() const; VulkanImage2D(const VulkanImage2D &image2D); }; diff --git a/test_conformance/vulkan/test_vulkan_api_consistency.cpp b/test_conformance/vulkan/test_vulkan_api_consistency.cpp index 7e5d4de7..7410cc7f 100644 --- a/test_conformance/vulkan/test_vulkan_api_consistency.cpp +++ b/test_conformance/vulkan/test_vulkan_api_consistency.cpp @@ -232,12 +232,11 @@ struct ConsistencyExternalImageTest : public VulkanTestBase const VulkanMemoryTypeList& memoryTypeList = vkImage2D.getMemoryTypeList(); - uint64_t totalImageMemSize = vkImage2D.getSize(); log_info("Memory type index: %u\n", (uint32_t)memoryTypeList[0]); log_info("Memory type property: %d\n", memoryTypeList[0].getMemoryTypeProperty()); - log_info("Image size : %ld\n", totalImageMemSize); + log_info("Image size : %ld\n", vkImage2D.getSize()); VulkanDeviceMemory* vkDeviceMem = new VulkanDeviceMemory(*vkDevice, vkImage2D, memoryTypeList[0], @@ -298,14 +297,12 @@ struct ConsistencyExternalImageTest : public VulkanTestBase const VkImageCreateInfo VulkanImageCreateInfo = vkImage2D.getVkImageCreateInfo(); - errNum = getCLImageInfoFromVkImageInfo(&VulkanImageCreateInfo, - totalImageMemSize, &img_format, - &image_desc); - if (errNum != CL_SUCCESS) - { - log_error("getCLImageInfoFromVkImageInfo failed!!!"); - return TEST_FAIL; - } + auto layout = vkImage2D.getSubresourceLayout(); + errNum = getCLImageInfoFromVkImageInfo( + device, &VulkanImageCreateInfo, &img_format, &image_desc, + vulkanImageTiling == VULKAN_IMAGE_TILING_LINEAR ? &layout + : nullptr); + test_error_fail(errNum, "getCLImageInfoFromVkImageInfo failed!!!"); clMemWrapper image; diff --git a/test_conformance/vulkan/test_vulkan_api_consistency_for_1dimages.cpp b/test_conformance/vulkan/test_vulkan_api_consistency_for_1dimages.cpp index bab9bd86..b6797f0c 100644 --- a/test_conformance/vulkan/test_vulkan_api_consistency_for_1dimages.cpp +++ b/test_conformance/vulkan/test_vulkan_api_consistency_for_1dimages.cpp @@ -90,12 +90,11 @@ struct ConsistencyExternalImage1DTest : public VulkanTestBase const VulkanMemoryTypeList& memoryTypeList = vkImage1D.getMemoryTypeList(); - uint64_t totalImageMemSize = vkImage1D.getSize(); log_info("Memory type index: %u\n", (uint32_t)memoryTypeList[0]); log_info("Memory type property: %d\n", memoryTypeList[0].getMemoryTypeProperty()); - log_info("Image size : %lu\n", totalImageMemSize); + log_info("Image size : %lu\n", vkImage1D.getSize()); VulkanDeviceMemory* vkDeviceMem = new VulkanDeviceMemory(*vkDevice, vkImage1D, memoryTypeList[0], @@ -156,14 +155,12 @@ struct ConsistencyExternalImage1DTest : public VulkanTestBase const VkImageCreateInfo VulkanImageCreateInfo = vkImage1D.getVkImageCreateInfo(); - errNum = getCLImageInfoFromVkImageInfo(&VulkanImageCreateInfo, - totalImageMemSize, &img_format, - &image_desc); - if (errNum != CL_SUCCESS) - { - log_error("getCLImageInfoFromVkImageInfo failed!!!"); - return TEST_FAIL; - } + auto layout = vkImage1D.getSubresourceLayout(); + errNum = getCLImageInfoFromVkImageInfo( + device, &VulkanImageCreateInfo, &img_format, &image_desc, + vulkanImageTiling == VULKAN_IMAGE_TILING_LINEAR ? &layout + : nullptr); + test_error_fail(errNum, "getCLImageInfoFromVkImageInfo failed!!!"); clMemWrapper image; @@ -174,16 +171,6 @@ struct ConsistencyExternalImage1DTest : public VulkanTestBase test_error(errNum, "Unable to create Image with Properties"); image.reset(); - // Passing NULL properties and a valid image_format and image_desc - image = clCreateImageWithProperties(context, NULL, CL_MEM_READ_WRITE, - &img_format, &image_desc, NULL, - &errNum); - test_error(errNum, - "Unable to create image with NULL properties " - "with valid image format and image desc"); - - image.reset(); - // Passing image_format as NULL image = clCreateImageWithProperties(context, extMemProperties.data(), CL_MEM_READ_WRITE, NULL, diff --git a/test_conformance/vulkan/test_vulkan_api_consistency_for_3dimages.cpp b/test_conformance/vulkan/test_vulkan_api_consistency_for_3dimages.cpp index 1a78dd8f..e84954e9 100644 --- a/test_conformance/vulkan/test_vulkan_api_consistency_for_3dimages.cpp +++ b/test_conformance/vulkan/test_vulkan_api_consistency_for_3dimages.cpp @@ -92,12 +92,11 @@ struct ConsistencyExternalImage3DTest : public VulkanTestBase const VulkanMemoryTypeList& memoryTypeList = vkImage3D.getMemoryTypeList(); - uint64_t totalImageMemSize = vkImage3D.getSize(); log_info("Memory type index: %u\n", (uint32_t)memoryTypeList[0]); log_info("Memory type property: %d\n", memoryTypeList[0].getMemoryTypeProperty()); - log_info("Image size : %lu\n", totalImageMemSize); + log_info("Image size : %lu\n", vkImage3D.getSize()); VulkanDeviceMemory* vkDeviceMem = new VulkanDeviceMemory(*vkDevice, vkImage3D, memoryTypeList[0], @@ -158,14 +157,12 @@ struct ConsistencyExternalImage3DTest : public VulkanTestBase const VkImageCreateInfo VulkanImageCreateInfo = vkImage3D.getVkImageCreateInfo(); - errNum = getCLImageInfoFromVkImageInfo(&VulkanImageCreateInfo, - totalImageMemSize, &img_format, - &image_desc); - if (errNum != CL_SUCCESS) - { - log_error("getCLImageInfoFromVkImageInfo failed!!!"); - return TEST_FAIL; - } + auto layout = vkImage3D.getSubresourceLayout(); + errNum = getCLImageInfoFromVkImageInfo( + device, &VulkanImageCreateInfo, &img_format, &image_desc, + vulkanImageTiling == VULKAN_IMAGE_TILING_LINEAR ? &layout + : nullptr); + test_error_fail(errNum, "getCLImageInfoFromVkImageInfo failed!!!"); clMemWrapper image; @@ -176,16 +173,6 @@ struct ConsistencyExternalImage3DTest : public VulkanTestBase test_error(errNum, "Unable to create Image with Properties"); image.reset(); - // Passing NULL properties and a valid image_format and image_desc - image = clCreateImageWithProperties(context, NULL, CL_MEM_READ_WRITE, - &img_format, &image_desc, NULL, - &errNum); - test_error(errNum, - "Unable to create image with NULL properties " - "with valid image format and image desc"); - - image.reset(); - // Passing image_format as NULL image = clCreateImageWithProperties(context, extMemProperties.data(), CL_MEM_READ_WRITE, NULL,