Re: [Mesa-dev] [PATCH mesa] khronos/egl: remove dependency on Android NDK header
On Thu, Aug 24, 2017 at 7:49 AM, Eric Engestrom wrote: > Khronos: https://github.com/KhronosGroup/EGL-Registry/pull/22 > Cc: Rob Herring > Cc: Emil Velikov > Signed-off-by: Eric Engestrom > --- > include/EGL/eglplatform.h | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/include/EGL/eglplatform.h b/include/EGL/eglplatform.h > index f045d009c0..bf9ec0bf5f 100644 > --- a/include/EGL/eglplatform.h > +++ b/include/EGL/eglplatform.h > @@ -97,8 +97,7 @@ typedef void *EGLNativeWindowType; > > #elif defined(__ANDROID__) || defined(ANDROID) > > -#include > - > +struct ANativeWindow; > struct egl_native_pixmap_t; How does this work when we need to dereference the struct to call ANativeWindow::dequeueBuffer() and others? Rob ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] khronos/egl: remove dependency on Android NDK header
On Thursday, 2017-08-24 08:54:04 -0500, Rob Herring wrote: > On Thu, Aug 24, 2017 at 7:49 AM, Eric Engestrom > wrote: > > Khronos: https://github.com/KhronosGroup/EGL-Registry/pull/22 > > Cc: Rob Herring > > Cc: Emil Velikov > > Signed-off-by: Eric Engestrom > > --- > > include/EGL/eglplatform.h | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/include/EGL/eglplatform.h b/include/EGL/eglplatform.h > > index f045d009c0..bf9ec0bf5f 100644 > > --- a/include/EGL/eglplatform.h > > +++ b/include/EGL/eglplatform.h > > @@ -97,8 +97,7 @@ typedef void *EGLNativeWindowType; > > > > #elif defined(__ANDROID__) || defined(ANDROID) > > > > -#include > > - > > +struct ANativeWindow; > > struct egl_native_pixmap_t; > > How does this work when we need to dereference the struct to call > ANativeWindow::dequeueBuffer() and others? Right, there are two things at play here: - eglplatform.h doesn't need to know the struct, so it shouldn't include a whole header but simply forward declare for the pointer. - platform_android does need it, but wasn't including the proper headers, so I missed it in my initial grep. It seems these two issues are orthogonal after all. This khronos header patch should land IMO, but platform_android needs a patch to avoid breaking (incoming) and will need the proper libraries that your patch 2/2 provides for the O update. Cheers, Eric ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] khronos/egl: remove dependency on Android NDK header
On 24 August 2017 at 15:22, Eric Engestrom wrote: > On Thursday, 2017-08-24 08:54:04 -0500, Rob Herring wrote: >> On Thu, Aug 24, 2017 at 7:49 AM, Eric Engestrom >> wrote: >> > Khronos: https://github.com/KhronosGroup/EGL-Registry/pull/22 >> > Cc: Rob Herring >> > Cc: Emil Velikov >> > Signed-off-by: Eric Engestrom >> > --- >> > include/EGL/eglplatform.h | 3 +-- >> > 1 file changed, 1 insertion(+), 2 deletions(-) >> > >> > diff --git a/include/EGL/eglplatform.h b/include/EGL/eglplatform.h >> > index f045d009c0..bf9ec0bf5f 100644 >> > --- a/include/EGL/eglplatform.h >> > +++ b/include/EGL/eglplatform.h >> > @@ -97,8 +97,7 @@ typedef void *EGLNativeWindowType; >> > >> > #elif defined(__ANDROID__) || defined(ANDROID) >> > >> > -#include >> > - >> > +struct ANativeWindow; >> > struct egl_native_pixmap_t; >> >> How does this work when we need to dereference the struct to call >> ANativeWindow::dequeueBuffer() and others? > > Right, there are two things at play here: > - eglplatform.h doesn't need to know the struct, so it shouldn't include > a whole header but simply forward declare for the pointer. > - platform_android does need it, but wasn't including the proper > headers, so I missed it in my initial grep. > > It seems these two issues are orthogonal after all. This khronos header > patch should land IMO, but platform_android needs a patch to avoid > breaking (incoming) and will need the proper libraries that your patch > 2/2 provides for the O update. > Thanks guys, for the correction. The two "issues" seem the same but not. On this patch - Eric, can you please mention in the commit message why we don't copy the whole file. With that: Reviewed-by: Emil Velikov -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] khronos/egl: remove dependency on Android NDK header
On Friday, 2017-08-25 16:47:35 +0100, Emil Velikov wrote: > On 24 August 2017 at 15:22, Eric Engestrom wrote: > > On Thursday, 2017-08-24 08:54:04 -0500, Rob Herring wrote: > >> On Thu, Aug 24, 2017 at 7:49 AM, Eric Engestrom > >> wrote: > >> > Khronos: https://github.com/KhronosGroup/EGL-Registry/pull/22 > >> > Cc: Rob Herring > >> > Cc: Emil Velikov > >> > Signed-off-by: Eric Engestrom > >> > --- > >> > include/EGL/eglplatform.h | 3 +-- > >> > 1 file changed, 1 insertion(+), 2 deletions(-) > >> > > >> > diff --git a/include/EGL/eglplatform.h b/include/EGL/eglplatform.h > >> > index f045d009c0..bf9ec0bf5f 100644 > >> > --- a/include/EGL/eglplatform.h > >> > +++ b/include/EGL/eglplatform.h > >> > @@ -97,8 +97,7 @@ typedef void *EGLNativeWindowType; > >> > > >> > #elif defined(__ANDROID__) || defined(ANDROID) > >> > > >> > -#include > >> > - > >> > +struct ANativeWindow; > >> > struct egl_native_pixmap_t; > >> > >> How does this work when we need to dereference the struct to call > >> ANativeWindow::dequeueBuffer() and others? > > > > Right, there are two things at play here: > > - eglplatform.h doesn't need to know the struct, so it shouldn't include > > a whole header but simply forward declare for the pointer. > > - platform_android does need it, but wasn't including the proper > > headers, so I missed it in my initial grep. > > > > It seems these two issues are orthogonal after all. This khronos header > > patch should land IMO, but platform_android needs a patch to avoid > > breaking (incoming) and will need the proper libraries that your patch > > 2/2 provides for the O update. > > > Thanks guys, for the correction. > The two "issues" seem the same but not. > > On this patch - Eric, can you please mention in the commit message why > we don't copy the whole file. I pushed it a couple hours before your email, sorry. /me needs to learn not to push stuff right away, let it sit for a bit... As for "why we don't copy the whole file", we essentially do, but we keep our local platforms additions (WL, GBM, X11 on MacOS, and Haiku), right? Or is there something else? > > With that: > Reviewed-by: Emil Velikov > > -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
