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

[Eric Engestrom]

On Tuesday, 2018-11-06 12:55:06 +0000, Lionel Landwerlin wrote:
> 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.

That was my original question; my r-b stands then, thanks :)

> 
> 
> -
> 
> Lionel
> 
> 
> > 
> > > > > -#endif
> > > > >       return anv_image_create(device,
> > > > >          &(struct anv_image_create_info) {
> 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev