[Mesa-dev] [PATCH 01/23] vulkan: Import vk_android_native_buffer.h

[Chad Versace]

On Wed 13 Sep 2017, Emil Velikov wrote:
> On 2 September 2017 at 09:17, Chad Versace <chadversary at chromium.org> wrote:
> > Just as Mesa imports the Khronos Vulkan headers, it should import this
> > Android-private Vulkan header too. This guarantees that Mesa will
> > continue to build even when upstream Android breaks header
> > compatibility.
> >
> That's the thing - upstream should not break. Although I share your
> concern/focus to keep Mesa building/working.

I agree, it shouldn't. But Android has done that before; it broke
backwards compatibility in frameworks headers between major releases (L
-> N -> O). (Though I don't recall the details).

> I'm wondering when upstream changes the API/ABI in a non-backward
> compatible manner.
> There will be interesting corruption/crashes + we'll need to still "fix" Mesa.

Right. But at least Mesa will remain bisectable because it will continue
to build.

> There are a few other concerns though. See below.
> 
> > --- /dev/null
> > +++ b/include/vulkan/vk_android_native_buffer.h
> 
> > +#ifndef __VK_ANDROID_NATIVE_BUFFER_H__
> > +#define __VK_ANDROID_NATIVE_BUFFER_H__
> > +
> > +#include <system/window.h>
> 
> As mentioned in 23/23 as-is this will pull the above header as
> dependency for normal Linux builds.
> Regardless of my input in said patch, one could use a local
> redeclaration for "buffer_handle_t", which is seemingly the only
> reason why this include exists?

Patch 2 in the series fixes this problem with a small hack.

> > +#include <vulkan/vulkan.h>
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#define VK_ANDROID_native_buffer 1
> > +
> > +#define VK_ANDROID_NATIVE_BUFFER_EXTENSION_NUMBER 11
> > +#define VK_ANDROID_NATIVE_BUFFER_SPEC_VERSION     5
> > +#define VK_ANDROID_NATIVE_BUFFER_EXTENSION_NAME   "VK_ANDROID_native_buffer"
> > +

> Version does not match the one in vk.xml, fortunately the rest do.

The version matches that in the vk_android_native_buffer.xml that I add
in patch 3.

> None of the remainder of this file has equivalent in vk.xml. That
> combined with the deprecated notations indicates that the API/ABI is
> not stable yet. Thus my earlier concern.

Yes, this API seems unstable to me too. That's why it's so important to
import the header.