Corrected procedure to collect proper size of image from VkImageCreateInfo #2289
Conversation
test_conformance/common/vulkan_wrapper/opencl_vulkan_wrapper.cpp
Outdated
Show resolved
Hide resolved
test_conformance/common/vulkan_wrapper/opencl_vulkan_wrapper.cpp
Outdated
Show resolved
Hide resolved
|
@shajder is this ready for review? I see you removed the "focused review" label - thanks! |
|
I still see a segfault in vkGetImageSubresourceLayout |
|
|
This one is fixed, still it didn't cause crash at my machine (nvidia driver version: 550.120) |
| return TEST_FAIL; | ||
| } | ||
| auto layout = vkImage1D.getSubresourceLayout(); | ||
| errNum = getCLImageInfoFromVkImageInfo( |
There was a problem hiding this comment.
This is causing a test failure in consistence 1d/3d (line 180 for 3D), where host_ptr is NULL and row_pitch/slice_pitch are non zero. The OpenCL spec requires row_pitch/slice_pitch to be zero when hostptr is NULL.
There was a problem hiding this comment.
The test does not run for me since #2349 was merged. I am investigating the issue.
There was a problem hiding this comment.
I am ok with this because it is an improvement. It would be better if row-pitch and slice pitch are specified, if assume linear is set. This should be a follow-up PR if not done in a follow-up patch set.
@joshqti it seems like we're going in circles here. I just removed the computations related to row_pitch/slice_pitch, but please note they were added recently in response to the previous request.
There was a problem hiding this comment.
The issue is that particular subtest passes a NULL host pointer, which is a violation of the OpenCL spec to pass row-pitch or slice pitch with NULL host pointer, and not using image import.
// 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");
I think the problem isn't your change in this case, I think this test case should be deleted. The properties are NULL, and therefore this check has nothing to to with OpenCL-Vulkan interop.
There was a problem hiding this comment.
I see. I have restored changes from last commit and removed mentioned subtests. Let me know if that works.
There was a problem hiding this comment.
Looks good to me, no issues running
|
Merging as discussed in the May 27th memory subgroup. |
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.