-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
vulkan: Use image row pitch #2077
Conversation
When importing a Vulkan external image, query and pass OpenCL a row pitch if OpenCL is assuming linear for the imported external handle type. Additionally fix a bug where OpenCL is being told to create mipmapped images at all times.
Program slice pitch if necessary.
@@ -676,6 +677,14 @@ clExternalMemoryImage::clExternalMemoryImage( | |||
std::vector<cl_mem_properties> extMemProperties1; | |||
cl_device_id devList[] = { deviceId, NULL }; | |||
|
|||
VulkanImageTiling vulkanImageTiling = | |||
vkClExternalMemoryHandleTilingAssumption( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this query required? VkImageCreateInfo is available, why not use its tiling field which gets populated during Vulkan image creation?
Also, since this change is for linear images, may I suggest a follow up change (or if possible, include in this?) in vulkan/*consistency_for_{1,3}d*.cpp?
Those are querying opencl linear tiling assumption, which is incorrect because vkCreateImage has a restriction where image type other than 2D is invalid for linear tiled image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are querying OpenCL's tiling assumption because if OpenCL assumes linear, it implies that it cannot infer the underlaying memory layout from the external memory handle type, and a row pitch should be provided to guarantee OpenCL imports the image correctly.
If the OpenCL tiling assumption is not linear, a row pitch is not required, even for linear Vulkan images, because the OpenCL implementation can correctly infer the memory layout from the external memory handle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am trying to recall our previous discussions on assume_linear query and potential to infer.
My understanding is that tiling assumption being not linear doesn't necessarily imply inference.
And we differed discussions on inference to a later point.
We probably need to go through KhronosGroup/OpenCL-Docs#861 and circle back here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment above doesn't necessarily impact the change here, but just want to make sure that we are not using assume_linear query to imply inference which is supposed to be a different capability in itself.
The change itself looks fine to me, but will double check with @saurabhnv and unblock here. |
Merging as discussed in the October 22nd teleconference. |
When importing a Vulkan external image, query and
pass OpenCL a row pitch if OpenCL is assuming linear for the imported external handle type. Additionally fix a bug where OpenCL is being told to create mipmapped images at all times.