[Mesa-dev] [PATCH 1/2] anv: stub internal android code

[Lionel Landwerlin]

On 06/11/2018 12:52, Eric Engestrom wrote:
> On Tuesday, 2018-11-06 12:42:20 +0000, Lionel Landwerlin wrote:
>> On 06/11/2018 12:20, Eric Engestrom wrote:
>>> On Tuesday, 2018-11-06 11:37:50 +0000, Lionel Landwerlin wrote:
>>>> This reduces the amount of #ifdef ANDROID we'll have to have inside
>>>> the driver. Potentially offering better coverage of the android
>>>> extensions.
>>>>
>>>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>>>> ---
>>>>    src/intel/Makefile.sources           |  4 +++
>>>>    src/intel/Makefile.vulkan.am         |  3 ++-
>>>>    src/intel/vulkan/anv_android.h       | 37 ++++++++++++++++++++++++++++
>>>>    src/intel/vulkan/anv_android_stubs.c | 34 +++++++++++++++++++++++++
>>>>    src/intel/vulkan/anv_image.c         |  2 --
>>>>    src/intel/vulkan/anv_private.h       | 10 +-------
>>>>    src/intel/vulkan/meson.build         |  2 ++
>>>>    7 files changed, 80 insertions(+), 12 deletions(-)
>>>>    create mode 100644 src/intel/vulkan/anv_android.h
>>>>    create mode 100644 src/intel/vulkan/anv_android_stubs.c
>>>>
> [snip]
>>>> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
>>>> index e89ce012be7..fdb13c2938a 100644
>>>> --- a/src/intel/vulkan/anv_image.c
>>>> +++ b/src/intel/vulkan/anv_image.c
>>>> @@ -631,14 +631,12 @@ anv_CreateImage(VkDevice device,
>>>>                    const VkAllocationCallbacks *pAllocator,
>>>>                    VkImage *pImage)
>>>>    {
>>>> -#ifdef ANDROID
>>>>       const VkNativeBufferANDROID *gralloc_info =
>>>>          vk_find_struct_const(pCreateInfo->pNext, NATIVE_BUFFER_ANDROID);
>>> I assume this is never going to match on non-android, right?
>>>
>>> If that's true:
>>> Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>
>>
>> Even if it does, the stub will return EXTENSION_NOT_PRESENT.
> Yes, but then:
>
>>>>       if (gralloc_info)
>>>>          return anv_image_from_gralloc(device, pCreateInfo, gralloc_info,
>>>>                                        pAllocator, pImage);
> This would return EXTENSION_NOT_PRESENT meaning android is not available
> on non-android platforms, instead of falling through to the next attempt
> with anv_image_create() below.
>
> Maybe it needs to be changed to something like this then?
>
>     if (gralloc_info) {
>        VkResult res = anv_image_from_gralloc(device, pCreateInfo, gralloc_info,
>                                              pAllocator, pImage);
>        if (res != VK_ERROR_EXTENSION_NOT_PRESENT)
>           return res;
>     }


Hmm.. not really. If the application requested Gralloc allocation and it 
fails, there is no reason to go for the default case.

Anyway, this should be invalid in the first place because the extension 
wouldn't be available.


-

Lionel


>
>>>> -#endif
>>>>       return anv_image_create(device,
>>>>          &(struct anv_image_create_info) {