[Mesa-dev] [PATCH 13/15] anv: support VkSamplerYcbcrConversionInfo in vkCreateImageView

[Tapani Pälli]

On 12/11/18 3:48 PM, Lionel Landwerlin wrote:
> On 27/11/2018 10:53, Tapani Pälli wrote:
>> If a conversion struct was passed, then initialize view using
>> format from the conversion structure.
>>
>> v2: use vk_format directly from the anv_format struct
>>
>> Signed-off-by: Tapani Pälli<tapani.palli at intel.com>
> 
> 
> One suggestion :
> 
> 
> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> 
> 
>> ---
>>   src/intel/vulkan/anv_image.c | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
>> index 79777efe456..2ac3eccbbe0 100644
>> --- a/src/intel/vulkan/anv_image.c
>> +++ b/src/intel/vulkan/anv_image.c
>> @@ -1391,6 +1391,16 @@ anv_CreateImageView(VkDevice _device,
>>      assert(range->layerCount > 0);
>>      assert(range->baseMipLevel < image->levels);
>>   
>> +   /* Check if a conversion info was passed. */
>> +   const struct anv_format *conv_format = NULL;
>> +   const struct VkSamplerYcbcrConversionInfo *conv_info =
>> +      vk_find_struct_const(pCreateInfo->pNext, SAMPLER_YCBCR_CONVERSION_INFO);
>> +
>> +   if (conv_info) {
>> +      ANV_FROM_HANDLE(anv_ycbcr_conversion, conversion, conv_info->conversion);
>> +      conv_format = conversion->format;
>> +   }
>> +
>>      const VkImageViewUsageCreateInfo *usage_info =
>>         vk_find_struct_const(pCreateInfo, IMAGE_VIEW_USAGE_CREATE_INFO);
>>      VkImageUsageFlags view_usage = usage_info ? usage_info->usage : image->usage;
>> @@ -1435,6 +1445,12 @@ anv_CreateImageView(VkDevice _device,
>>      iview->n_planes = anv_image_aspect_get_planes(iview->aspect_mask);
>>      iview->vk_format = pCreateInfo->format;
> 
> 
> Not sure if overly harsh, but I could add this :
> 
> 
> /* "If|image|has anexternal format 
> <#memory-external-android-hardware-buffer-external-formats>,|format|*must*be|VK_FORMAT_UNDEFINED|." 
> */
> 
> assert(!image->external_format || pCreateInfo->format == 
> VK_FORMAT_UNDEFINED);
> 
> /* " If|image|has anexternal format 
> <#memory-external-android-hardware-buffer-external-formats>, 
> the|pNext|chain*must*contain an instance ofVkSamplerYcbcrConversionInfo 
> <#VkSamplerYcbcrConversionInfo>with a|conversion|object created with the 
> same external format as|image|." */
> 
> assert(!image->external_format || conv_info);
> 

Yes makes sense, I've added these!

>>   
>> +   /* Format is undefined, this can happen when using external formats. Set
>> +    * view format from the passed conversion info.
>> +    */
>> +   if (iview->vk_format == VK_FORMAT_UNDEFINED && conv_format)
>> +      iview->vk_format = conv_format->vk_format;
>> +
>>      iview->extent = (VkExtent3D) {
>>         .width  = anv_minify(image->extent.width , range->baseMipLevel),
>>         .height = anv_minify(image->extent.height, range->baseMipLevel),
>> @@ -1451,7 +1467,7 @@ anv_CreateImageView(VkDevice _device,
>>         VkImageAspectFlags vplane_aspect =
>>            anv_plane_to_aspect(iview->aspect_mask, vplane);
>>         struct anv_format_plane format =
>> -         anv_get_format_plane(&device->info, pCreateInfo->format,
>> +         anv_get_format_plane(&device->info, iview->vk_format,
>>                                 vplane_aspect, image->tiling);
>>   
>>         iview->planes[vplane].image_plane = iplane;
> 
>