Re: [waffle] [PATCH] egl: Support robust access contexts with EGL 1.5.

2016-04-15 Thread Emil Velikov
Hi Bas,

On 15 April 2016 at 22:04, Bas Nieuwenhuizen  wrote:
> EGL 1.5 also supports the EGL_CONTEXT_OPENGL_ROBUST_ACCESS
> attribute without any extensions for both ES and non-ES.
>
> Signed-off-by: Bas Nieuwenhuizen 
> ---
>  src/waffle/egl/wegl_config.c  | 9 ++---
>  src/waffle/egl/wegl_context.c | 9 +++--
>  src/waffle/egl/wegl_display.c | 3 +--
>  src/waffle/egl/wegl_display.h | 2 ++
>  4 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/src/waffle/egl/wegl_config.c b/src/waffle/egl/wegl_config.c
> index 1c3f416..08e06fb 100644
> --- a/src/waffle/egl/wegl_config.c
> +++ b/src/waffle/egl/wegl_config.c
> @@ -56,17 +56,20 @@ check_context_attrs(struct wegl_display *dpy,
>  }
>
>  if (attrs->context_robust && !dpy->EXT_create_context_robustness &&
> +dpy->major_version == 1 && dpy->minor_version < 5 &&
>  attrs->context_api != WAFFLE_CONTEXT_OPENGL) {
>  wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
> - "EGL_EXT_create_context_robustness is required in order 
> to "
> - "request a robust access context for OpenGL ES");
> + "EGL_EXT_create_context_robustness or EGL 1.5 is "
> + "required in order to request a robust access context "
> + "for OpenGL ES");
>  return false;
>  }
>
>  if (attrs->context_robust && !dpy->KHR_create_context &&
> +dpy->major_version == 1 && dpy->minor_version < 5 &&
>  attrs->context_api == WAFFLE_CONTEXT_OPENGL) {
>  wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
> - "EGL_KHR_create_context is required in order to "
> + "EGL_KHR_create_context or EGL 1.5 is required in order 
> to "
>   "request a robust access context for OpenGL");
>  return false;
>  }
> diff --git a/src/waffle/egl/wegl_context.c b/src/waffle/egl/wegl_context.c
> index 67cbc04..4b208fa 100644
> --- a/src/waffle/egl/wegl_context.c
> +++ b/src/waffle/egl/wegl_context.c
> @@ -96,8 +96,13 @@ create_real_context(struct wegl_config *config,
>  }
>
>  if (attrs->context_robust) {
> -assert(dpy->KHR_create_context);
> -context_flags |= EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR;
> +if (dpy->major_version > 1 || dpy->minor_version >= 5) {
> +attrib_list[i++] = EGL_CONTEXT_OPENGL_ROBUST_ACCESS_EXT;
We want EGL_CONTEXT_OPENGL_ROBUST_ACCESS (note missing _EXT) here.
For whatever reason the two differ 0x31B2 vs 0x30BF (according to the
includes in mesa).

> +attrib_list[i++] = EGL_TRUE;
> +} else {
> +assert(dpy->KHR_create_context);
> +context_flags |= 
> EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR;
> +}
>  }
>
>  if (wcore_config_attrs_version_ge(attrs, 32))  {
> diff --git a/src/waffle/egl/wegl_display.c b/src/waffle/egl/wegl_display.c
> index 16af142..a5e3429 100644
> --- a/src/waffle/egl/wegl_display.c
> +++ b/src/waffle/egl/wegl_display.c
> @@ -64,7 +64,6 @@ wegl_display_init(struct wegl_display *dpy,
>  {
>  struct wegl_platform *plat = wegl_platform(wc_plat);
>  bool ok;
> -EGLint major, minor;
>
>  ok = wcore_display_init(&dpy->wcore, wc_plat);
>  if (!ok)
> @@ -76,7 +75,7 @@ wegl_display_init(struct wegl_display *dpy,
>  goto fail;
>  }
>
> -ok = plat->eglInitialize(dpy->egl, &major, &minor);
> +ok = plat->eglInitialize(dpy->egl, &dpy->major_version, 
> &dpy->minor_version);
>  if (!ok) {
>  wegl_emit_error(plat, "eglInitialize");
>  goto fail;
> diff --git a/src/waffle/egl/wegl_display.h b/src/waffle/egl/wegl_display.h
> index b82a2ec..0d03ec8 100644
> --- a/src/waffle/egl/wegl_display.h
> +++ b/src/waffle/egl/wegl_display.h
> @@ -39,6 +39,8 @@ struct wegl_display {
>  EGLDisplay egl;
>  bool EXT_create_context_robustness;
>  bool KHR_create_context;
> +EGLint major_version;
> +EGLint minor_version;

While working on a similar version specific EGL bits, I've added a
single boolean as it seemed easer/shorter to check (yes I'm that
lazy).
Just throwing the idea out there, unless others insist you don't have
to do any of that.

With the s/_EXT//g issue above
Reviewed-by: Emil Velikov 

-Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] [PATCH] egl: correctly set the attrib/bits for robust contexts

2016-04-15 Thread Emil Velikov
On 16 April 2016 at 00:34, Emil Velikov  wrote:
> The EXT_create_context_robustness extension covers only GLES contexts,
> while the latter EGL_KHR_create_context works for both GL and GLES ones.
>
Actually the second extension seems a bit inconsistent. The following
section clearly lists GLES

OpenGL and OpenGL ES Context Flags
--

If the EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR bit is set in
EGL_CONTEXT_FLAGS_KHR, then a context supporting  will be created.

Then it continues with

Robust buffer access is defined in the
GL_ARB_robustness extension specification, and the resulting context
must also support either the GL_ARB_robustness extension, or a
version of OpenGL incorporating equivalent functionality. This bit
is supported for OpenGL contexts.


Then issues 7 and 10 go to confirm that GLES and robustness is not a
thing in this extension, yet no-one bothered moving the hunk to the
OpenGL only section :'-(

This spec is quite and interesting read - forward compat seems to be
in the exact same boat. While debug is funnier - added with the GL +
GLES section initially, discarded as not supported in issue 7, then
brought back with issue 9.

So overall I should bring the KHR_create_context bits back into OpenGL
denominator.

-Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


[waffle] [PATCH] egl: correctly set the attrib/bits for robust contexts

2016-04-15 Thread Emil Velikov
The EXT_create_context_robustness extension covers only GLES contexts,
while the latter EGL_KHR_create_context works for both GL and GLES ones.

Cc: Bas Nieuwenhuizen 
Signed-off-by: Emil Velikov 
---

Seems like these lovely extensions managed to confused most of us.
Although in all honestly, chances are pretty small that one will 
implement the KHR extension and no the EXT one and vice-versa.

Emil

 src/waffle/egl/wegl_config.c  | 12 +++-
 src/waffle/egl/wegl_context.c | 11 +--
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/src/waffle/egl/wegl_config.c b/src/waffle/egl/wegl_config.c
index 1c3f416..9e0bc2e 100644
--- a/src/waffle/egl/wegl_config.c
+++ b/src/waffle/egl/wegl_config.c
@@ -56,18 +56,12 @@ check_context_attrs(struct wegl_display *dpy,
 }
 
 if (attrs->context_robust && !dpy->EXT_create_context_robustness &&
-attrs->context_api != WAFFLE_CONTEXT_OPENGL) {
+attrs->context_api != WAFFLE_CONTEXT_OPENGL && 
!dpy->KHR_create_context) {
 wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
  "EGL_EXT_create_context_robustness is required in order 
to "
- "request a robust access context for OpenGL ES");
-return false;
-}
-
-if (attrs->context_robust && !dpy->KHR_create_context &&
-attrs->context_api == WAFFLE_CONTEXT_OPENGL) {
-wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
+ "request a robust access context for OpenGL ES."
  "EGL_KHR_create_context is required in order to "
- "request a robust access context for OpenGL");
+ "request a robust access context for OpenGL or OpenGL 
ES");
 return false;
 }
 
diff --git a/src/waffle/egl/wegl_context.c b/src/waffle/egl/wegl_context.c
index 67cbc04..027ed97 100644
--- a/src/waffle/egl/wegl_context.c
+++ b/src/waffle/egl/wegl_context.c
@@ -77,6 +77,10 @@ create_real_context(struct wegl_config *config,
 context_flags |= EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR;
 }
 
+if (attrs->context_robust && dpy->KHR_create_context) {
+context_flags |= EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR;
+}
+
 switch (waffle_context_api) {
 case WAFFLE_CONTEXT_OPENGL:
 if (dpy->KHR_create_context) {
@@ -95,11 +99,6 @@ create_real_context(struct wegl_config *config,
 context_flags |= EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR;
 }
 
-if (attrs->context_robust) {
-assert(dpy->KHR_create_context);
-context_flags |= EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR;
-}
-
 if (wcore_config_attrs_version_ge(attrs, 32))  {
 assert(dpy->KHR_create_context);
 switch (attrs->context_profile) {
@@ -133,7 +132,7 @@ create_real_context(struct wegl_config *config,
 assert(attrs->context_minor_version == 0);
 }
 
-if (attrs->context_robust) {
+if (attrs->context_robust && EXT_create_context_robustness) {
 attrib_list[i++] = EGL_CONTEXT_OPENGL_ROBUST_ACCESS_EXT;
 attrib_list[i++] = EGL_TRUE;
 }
-- 
2.8.0

___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


[waffle] [PATCH v2] wgl: don't use ARB_create_context with pre 3.2 contexts

2016-04-15 Thread Emil Velikov
Direct port of previous commit.

v2: The version should be 3.2 and not 3.0 as originally.

Cc: Jose Fonseca 
Cc: Ilia Mirkin 
Signed-off-by: Emil Velikov 
---
 src/waffle/wgl/wgl_config.c  |  7 ---
 src/waffle/wgl/wgl_context.c | 12 +++-
 src/waffle/wgl/wgl_context.h | 15 +++
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/src/waffle/wgl/wgl_config.c b/src/waffle/wgl/wgl_config.c
index 59a70a6..43bcfa5 100644
--- a/src/waffle/wgl/wgl_config.c
+++ b/src/waffle/wgl/wgl_config.c
@@ -32,6 +32,7 @@
 #include "wcore_error.h"
 
 #include "wgl_config.h"
+#include "wgl_context.h"
 #include "wgl_display.h"
 #include "wgl_error.h"
 #include "wgl_platform.h"
@@ -73,11 +74,11 @@ wgl_config_check_context_attrs(struct wgl_display *dpy,
 
 switch (attrs->context_api) {
 case WAFFLE_CONTEXT_OPENGL:
-if (!wcore_config_attrs_version_eq(attrs, 10) && 
!dpy->ARB_create_context) {
+if (wgl_context_needs_arb_create_context(attrs) &&
+!dpy->ARB_create_context) {
 wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
  "WGL_ARB_create_context is required in order to "
- "request an OpenGL version not equal to the 
default "
- "value 1.0");
+ "request an OpenGL version greater or equal than 
3.2");
 return false;
 }
 else if (wcore_config_attrs_version_ge(attrs, 32) && 
!dpy->ARB_create_context_profile) {
diff --git a/src/waffle/wgl/wgl_context.c b/src/waffle/wgl/wgl_context.c
index dd45f81..32b2e2d 100644
--- a/src/waffle/wgl/wgl_context.c
+++ b/src/waffle/wgl/wgl_context.c
@@ -152,7 +152,17 @@ wgl_context_create_native(struct wgl_config *config,
 HGLRC real_share_ctx = share_ctx ? share_ctx->hglrc : NULL;
 HGLRC hglrc;
 
-if (dpy->ARB_create_context) {
+// Use ARB_create_context when we have
+// - OpenGL version 1.0, or
+// - OpenGL version 3.2 or greater, or
+// - OpenGL with fwd_compat, or
+// - Debug context
+//
+// The first one of the four is optional, the remainder hard requirement
+// for the use of ARB_create_context.
+if (dpy->ARB_create_context &&
+(wcore_config_attrs_version_eq(&config->wcore.attrs, 10) ||
+ wgl_context_needs_arb_create_context(&config->wcore.attrs))) {
 bool ok;
 
 // Choose a large size to prevent accidental overflow.
diff --git a/src/waffle/wgl/wgl_context.h b/src/waffle/wgl/wgl_context.h
index c55ad58..28c25c9 100644
--- a/src/waffle/wgl/wgl_context.h
+++ b/src/waffle/wgl/wgl_context.h
@@ -27,6 +27,7 @@
 
 #include 
 
+#include "wcore_config_attrs.h"
 #include "wcore_context.h"
 #include "wcore_util.h"
 
@@ -51,3 +52,17 @@ wgl_context_create(struct wcore_platform *wc_plat,
 
 bool
 wgl_context_destroy(struct wcore_context *wc_self);
+
+static inline bool
+wgl_context_needs_arb_create_context(const struct wcore_config_attrs *attrs)
+{
+if (attrs->context_api == WAFFLE_CONTEXT_OPENGL &&
+(wcore_config_attrs_version_ge(attrs, 32) ||
+ attrs->context_forward_compatible))
+return true;
+
+if (attrs->context_debug)
+return true;
+
+return false;
+}
-- 
2.8.0

___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


[waffle] [PATCH v2] glx: don't use ARB_create_context with pre 3.2 contexts

2016-04-15 Thread Emil Velikov
This way if the user requests GL pre 3.2 context which lacks the
flags/extra bits which require ARB_create_context one can safely fall
back to the normal/legacy entry point.

This resolves piglits on non 3.2 capable drivers such as classic swrast,
nouveau_vieux and alike.

v2: The version should be 3.2 and not 3.0 as originally.

Cc: Ilia Mirkin 
Reviewed-by: Jose Fonseca  (v1)
Signed-off-by: Emil Velikov 
---
 src/waffle/glx/glx_config.c  |  7 ---
 src/waffle/glx/glx_context.c | 12 +++-
 src/waffle/glx/glx_context.h | 16 
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/src/waffle/glx/glx_config.c b/src/waffle/glx/glx_config.c
index 7792aa0..259e538 100644
--- a/src/waffle/glx/glx_config.c
+++ b/src/waffle/glx/glx_config.c
@@ -32,6 +32,7 @@
 #include "wcore_error.h"
 
 #include "glx_config.h"
+#include "glx_context.h"
 #include "glx_display.h"
 #include "glx_platform.h"
 #include "glx_wrappers.h"
@@ -70,11 +71,11 @@ glx_config_check_context_attrs(struct glx_display *dpy,
 
 switch (attrs->context_api) {
 case WAFFLE_CONTEXT_OPENGL:
-if (!wcore_config_attrs_version_eq(attrs, 10) && 
!dpy->ARB_create_context) {
+if (glx_context_needs_arb_create_context(attrs) &&
+!dpy->ARB_create_context) {
 wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
  "GLX_ARB_create_context is required in order to "
- "request an OpenGL version not equal to the 
default "
- "value 1.0");
+ "request an OpenGL version greater or equal than 
3.2");
 return false;
 }
 else if (wcore_config_attrs_version_ge(attrs, 32) && 
!dpy->ARB_create_context_profile) {
diff --git a/src/waffle/glx/glx_context.c b/src/waffle/glx/glx_context.c
index 1f9290b..cb10e32 100644
--- a/src/waffle/glx/glx_context.c
+++ b/src/waffle/glx/glx_context.c
@@ -169,7 +169,17 @@ glx_context_create_native(struct glx_config *config,
 struct glx_display *dpy = glx_display(config->wcore.display);
 struct glx_platform *platform = glx_platform(dpy->wcore.platform);
 
-if (dpy->ARB_create_context) {
+// Use ARB_create_context when we have
+// - OpenGL version 1.0, or
+// - OpenGL version 3.2 or greater, or
+// - OpenGL with fwd_compat, or
+// - Debug context
+//
+// The first one of the four is optional, the remainder hard requirement
+// for the use of ARB_create_context.
+if (dpy->ARB_create_context &&
+(wcore_config_attrs_version_eq(&config->wcore.attrs, 10) ||
+ glx_context_needs_arb_create_context(&config->wcore.attrs))) {
 bool ok;
 
 // Choose a large size to prevent accidental overflow.
diff --git a/src/waffle/glx/glx_context.h b/src/waffle/glx/glx_context.h
index bb2a4dd..23ffd74 100644
--- a/src/waffle/glx/glx_context.h
+++ b/src/waffle/glx/glx_context.h
@@ -29,6 +29,7 @@
 
 #include 
 
+#include "wcore_config_attrs.h"
 #include "wcore_context.h"
 #include "wcore_util.h"
 
@@ -55,3 +56,18 @@ glx_context_destroy(struct wcore_context *wc_self);
 
 union waffle_native_context*
 glx_context_get_native(struct wcore_context *wc_self);
+
+
+static inline bool
+glx_context_needs_arb_create_context(const struct wcore_config_attrs *attrs)
+{
+if (attrs->context_api == WAFFLE_CONTEXT_OPENGL &&
+(wcore_config_attrs_version_ge(attrs, 32) ||
+ attrs->context_forward_compatible))
+return true;
+
+if (attrs->context_debug)
+return true;
+
+return false;
+}
-- 
2.8.0

___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] [PATCH 1/3] glx: don't use ARB_create_context with pre 3.0 contexts

2016-04-15 Thread Emil Velikov
On 15 April 2016 at 20:04, Chad Versace  wrote:
> On 04/06/2016 02:12 AM, Jose Fonseca wrote:
>> On 05/04/16 22:45, Emil Velikov wrote:
>>> This way if the user requests GL pre 3.0 context which lacks the
>>> flags/extra bits which require ARB_create_context one can safely fall
>>> back to the normal/legacy entry point.
>>>
>>> This resolves piglits on non 3.0 capable drivers such as classic swrast,
>>> nouveau_vieux and alike.
>>>
>>> Cc: Jose Fonseca 
>>> Cc: Ilia Mirkin 
>>> Signed-off-by: Emil Velikov 
>
>
>
>>> +static inline bool
>>> +glx_context_needs_arb_create_context(const struct wcore_config_attrs 
>>> *attrs)
>>> +{
>>> +if (attrs->context_api == WAFFLE_CONTEXT_OPENGL &&
>>> +(wcore_config_attrs_version_ge(attrs, 30) ||
>>> + attrs->context_forward_compatible))
>>> +return true;
>>> +
>>> +if (attrs->context_debug)
>>> +return true;
>>> +
>>> +return false;
>>> +}
>
>> Looks good to me.  Thanks.
>>
>> Reviewed-by: Jose Fonseca 
>
> I reviewed the thread on the Piglit list, and I'm in favor of this change.
>
> Jose and Emil, I believe the critical version should be 3.2, not 3.0. I don't
> understand why this patch uses 3.0 as the cutoff version.  The
> GLX_ARB_create_context spec says:
>
> The presence of an OpenGL 3.2 or later implementation determines whether 
> or
> not GLX_ARB_create_context_profile is required.
>
> And the WGL spec contains the same text.
>
> In other words, it never makes sense to request a 3.2 context without
> GLX_ARB_create_context, because the availability of 3.2 mandates the
> availability of GLX_ARB_create_context_profile.

Looking at another hunk from the spec makes me wonder:

created. Forward-compatible contexts are defined only for OpenGL
versions 3.0 and later.

So one cannot get a fwd compat context with 3.0 or 3.1, if they don't
support OpenGL 3.2. Not sure I'm getting why they choose to make it
its way - there must be something subtle :-) Regardless, updated
patches coming in a second.

-Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

2016-04-15 Thread Emil Velikov
On 15 April 2016 at 21:56, Chad Versace  wrote:
> On 04/15/2016 09:36 AM, Jason Ekstrand wrote:
>> On Fri, Apr 15, 2016 at 3:03 AM, Emil Velikov > > wrote:
>>
>> On 15 April 2016 at 03:32, Michel Dänzer > > wrote:
>> > On 15.04.2016 11:14, Michel Dänzer wrote:
>> >> On 14.04.2016 22:16, Emil Velikov wrote:
>> >>> On 14 April 2016 at 09:23, Michel Dänzer > > wrote:
>>  From: Michel Dänzer > >
>> 
>>  Fixes build failure due to wl_proxy_marshal_constructor_versioned 
>> being
>>  unresolved when building against current wayland.
>> 
>>  This API was introduced in wayland 1.9.91 by commit 557032e3 ("Track
>>  protocol object versions inside wl_proxy."). The waffle code doesn't
>>  reference wl_proxy_marshal_constructor_versioned directly but
>>  indirectly via wayland-scanner.
>> 
>>  v2:
>>  * Add paragraph about how wl_proxy_marshal_constructor_versioned was
>>    introduced. (Emil Velikov)
>>  * Only resolve wl_proxy_marshal_constructor_versioned with wayland 
>> >=
>>    1.9.91.
>> 
>>  Signed-off-by: Michel Dänzer > >
>>  ---
>>   src/waffle/wayland/wayland_wrapper.c | 5 +
>>   src/waffle/wayland/wayland_wrapper.h | 8 
>>   2 files changed, 13 insertions(+)
>> 
>>  diff --git a/src/waffle/wayland/wayland_wrapper.c 
>> b/src/waffle/wayland/wayland_wrapper.c
>>  index 6ffd5a9..fb66f9a 100644
>>  --- a/src/waffle/wayland/wayland_wrapper.c
>>  +++ b/src/waffle/wayland/wayland_wrapper.c
>>  @@ -106,6 +106,11 @@ wayland_wrapper_init(void)
>>   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_add_listener);
>>   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal);
>>   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor);
>>  +#if WAYLAND_VERSION_MAJOR == 1 && \
>>  +(WAYLAND_VERSION_MINOR > 9 || \
>>  + (WAYLAND_VERSION_MINOR == 9 && WAYLAND_VERSION_MICRO >= 91))
>>  +
>> RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor_versioned);
>>  +#endif
>>   #undef RETRIEVE_WL_CLIENT_SYMBOL
>> 
>> >>> I am slightly worried about this approach. It adds a so called 
>> 'hidden
>> >>> dependency' and with it a possibility of things going horribly wrong.
>> >>> It is something that we try to avoid with mesa as the deps version at
>> >>> build time != run-time one. Or in other words, one might build 
>> against
>> >>> wayland 1.9 and things will go crazy as you run wayland 1.10, or vice
>> >>> versa.
>
> I prefer to avoid adding this "hidden dependency" (as Emil calls it) for
> a Wayland function that Waffle doesn't use or need. Jason's solution of
> importing wayland-client-protocol.h avoids that dependency, and it also
> prevents this build-breakage problem from occuring in the future.
>
Then again importing bits from other projects tend to be quite a nasty
solution. The place where I borrowed the idea from (libSDL) does/did
not imports the protocol.

>> I think this is actually mostly ok.  In the Wayland project, we were very
>> careful to ensure that anything built against 1.9 would work when linked
>> against 1.10.  This should continue to be the case even with waffle's
>> shim-layer.
>>
>> The problem is not that libwayland added a new symbol nor is it a problem
>> that wayland-protocol.h was updated to use that new symbol.  The problem is
>> that waffle sticks a shim layer in between wayland-protocol.h and libwayland.
>> This makes the wayland-protocol.h file effectively internal to waffle but
>> still updatable by the distro.  This is a layering violation.  To keep this
>> from happening in the future, you probably want to just check a version of
>> wayland-client-protocol.h into the waffle repo so that it doesn't change out
>> from under you and make waffle just use wayland-client-core.h.  You can even
>> check in the version from libwayland 1.9 if you'd like to keep waffle
>> building against older versions.
>
> I think I understand you, but I'm not confident. Wayland's header dependencies
> are, we can all admit, confusing.
>
> If Waffle does the following...
>
> a. Check into the repo the wayland-client-protocol.h from Wayland 1.9.
>
> ... then ...
>
> c. Waffle will successfully build against distro-provided Wayland headers
>for wayland >= 1.9. Specifically, the system's wayland-client.h will
>include Waffle's imported wayland-client-protocol.h, and nothing will
>explode.
>
> d. If Waffle is built against the system's wayland-client.h from Wayland
>1.x (where x >= 9), the libwaffle can successfully dlopen and run 
> against
>libwayland 1.y (where y > x).
>
> Jaso

[waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

2016-04-15 Thread Emil Velikov
From: Michel Dänzer 

Fixes build failure due to wl_proxy_marshal_constructor_versioned being
unresolved when building against current wayland.

This API was introduced in wayland 1.9.91 by commit 557032e3 ("Track
protocol object versions inside wl_proxy."). The waffle code doesn't
reference wl_proxy_marshal_constructor_versioned directly but
indirectly via wayland-scanner.

v2:
* Add paragraph about how wl_proxy_marshal_constructor_versioned was
  introduced. (Emil Velikov)
* Only resolve wl_proxy_marshal_constructor_versioned with wayland >=
  1.9.91.

Signed-off-by: Michel Dänzer 

v3:
* Always resolve wl_proxy_marshal_constructor_versioned, yet do not
error out if it's missing.

Cc: Jason Ekstrand 
Cc: Chad Versace 
Signed-off-by: Emil Velikov 
---

Gents, I've taken the liberty to update Michel patch with what I have in 
mind. Why ? I'm wondering if some of the things mentioned are not 
getting across, and as we all know 'a patch speaks a thousand words'

-Emil

 src/waffle/wayland/wayland_wrapper.c | 8 
 src/waffle/wayland/wayland_wrapper.h | 8 
 2 files changed, 16 insertions(+)

diff --git a/src/waffle/wayland/wayland_wrapper.c 
b/src/waffle/wayland/wayland_wrapper.c
index 6ffd5a9..d08e3b5 100644
--- a/src/waffle/wayland/wayland_wrapper.c
+++ b/src/waffle/wayland/wayland_wrapper.c
@@ -106,6 +106,14 @@ wayland_wrapper_init(void)
 RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_add_listener);
 RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal);
 RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor);
+
+#define OPTIONAL_WL_CLIENT_SYMBOL(S)\
+wfl_##S = (__typeof__(wfl_##S))dlsym(dl_wl_client, #S);
+
+// Introduced with Wayland 1.10. Used over wl_proxy_marshal_constructor
+OPTIONAL_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor_versioned);
+
+#undef OPTIONAL_WL_CLIENT_SYMBOL
 #undef RETRIEVE_WL_CLIENT_SYMBOL
 
 error:
diff --git a/src/waffle/wayland/wayland_wrapper.h 
b/src/waffle/wayland/wayland_wrapper.h
index 40a581a..3282587 100644
--- a/src/waffle/wayland/wayland_wrapper.h
+++ b/src/waffle/wayland/wayland_wrapper.h
@@ -75,6 +75,13 @@ struct wl_proxy *
 const struct wl_interface *interface,
 ...);
 
+struct wl_proxy *
+(*wfl_wl_proxy_marshal_constructor_versioned)(struct wl_proxy *proxy,
+ uint32_t opcode,
+ const struct wl_interface 
*interface,
+ uint32_t version,
+ ...);
+
 #ifdef _WAYLAND_CLIENT_H
 #error Do not include wayland-client.h ahead of wayland_wrapper.h
 #endif
@@ -92,3 +99,4 @@ struct wl_proxy *
 #define wl_proxy_add_listener (*wfl_wl_proxy_add_listener)
 #define wl_proxy_marshal (*wfl_wl_proxy_marshal)
 #define wl_proxy_marshal_constructor (*wfl_wl_proxy_marshal_constructor)
+#define wl_proxy_marshal_constructor_versioned 
(*wfl_wl_proxy_marshal_constructor_versioned)
-- 
2.8.0

___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

2016-04-15 Thread Jason Ekstrand
On Fri, Apr 15, 2016 at 1:56 PM, Chad Versace 
wrote:

> On 04/15/2016 09:36 AM, Jason Ekstrand wrote:
> > On Fri, Apr 15, 2016 at 3:03 AM, Emil Velikov  > wrote:
> >
> > On 15 April 2016 at 03:32, Michel Dänzer  > wrote:
> > > On 15.04.2016 11:14, Michel Dänzer wrote:
> > >> On 14.04.2016 22:16, Emil Velikov wrote:
> > >>> On 14 April 2016 at 09:23, Michel Dänzer  > wrote:
> >  From: Michel Dänzer  michel.daen...@amd.com>>
> > 
> >  Fixes build failure due to
> wl_proxy_marshal_constructor_versioned being
> >  unresolved when building against current wayland.
> > 
> >  This API was introduced in wayland 1.9.91 by commit 557032e3
> ("Track
> >  protocol object versions inside wl_proxy."). The waffle code
> doesn't
> >  reference wl_proxy_marshal_constructor_versioned directly but
> >  indirectly via wayland-scanner.
> > 
> >  v2:
> >  * Add paragraph about how
> wl_proxy_marshal_constructor_versioned was
> >    introduced. (Emil Velikov)
> >  * Only resolve wl_proxy_marshal_constructor_versioned with
> wayland >=
> >    1.9.91.
> > 
> >  Signed-off-by: Michel Dänzer  michel.daen...@amd.com>>
> >  ---
> >   src/waffle/wayland/wayland_wrapper.c | 5 +
> >   src/waffle/wayland/wayland_wrapper.h | 8 
> >   2 files changed, 13 insertions(+)
> > 
> >  diff --git a/src/waffle/wayland/wayland_wrapper.c
> b/src/waffle/wayland/wayland_wrapper.c
> >  index 6ffd5a9..fb66f9a 100644
> >  --- a/src/waffle/wayland/wayland_wrapper.c
> >  +++ b/src/waffle/wayland/wayland_wrapper.c
> >  @@ -106,6 +106,11 @@ wayland_wrapper_init(void)
> >   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_add_listener);
> >   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal);
> >   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor);
> >  +#if WAYLAND_VERSION_MAJOR == 1 && \
> >  +(WAYLAND_VERSION_MINOR > 9 || \
> >  + (WAYLAND_VERSION_MINOR == 9 && WAYLAND_VERSION_MICRO >=
> 91))
> >  +
> RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor_versioned);
> >  +#endif
> >   #undef RETRIEVE_WL_CLIENT_SYMBOL
> > 
> > >>> I am slightly worried about this approach. It adds a so called
> 'hidden
> > >>> dependency' and with it a possibility of things going horribly
> wrong.
> > >>> It is something that we try to avoid with mesa as the deps
> version at
> > >>> build time != run-time one. Or in other words, one might build
> against
> > >>> wayland 1.9 and things will go crazy as you run wayland 1.10, or
> vice
> > >>> versa.
>
> I prefer to avoid adding this "hidden dependency" (as Emil calls it) for
> a Wayland function that Waffle doesn't use or need. Jason's solution of
> importing wayland-client-protocol.h avoids that dependency, and it also
> prevents this build-breakage problem from occuring in the future.
>
> > I think this is actually mostly ok.  In the Wayland project, we were very
> > careful to ensure that anything built against 1.9 would work when linked
> > against 1.10.  This should continue to be the case even with waffle's
> > shim-layer.
> >
> > The problem is not that libwayland added a new symbol nor is it a problem
> > that wayland-protocol.h was updated to use that new symbol.  The problem
> is
> > that waffle sticks a shim layer in between wayland-protocol.h and
> libwayland.
> > This makes the wayland-protocol.h file effectively internal to waffle but
> > still updatable by the distro.  This is a layering violation.  To keep
> this
> > from happening in the future, you probably want to just check a version
> of
> > wayland-client-protocol.h into the waffle repo so that it doesn't change
> out
> > from under you and make waffle just use wayland-client-core.h.  You can
> even
> > check in the version from libwayland 1.9 if you'd like to keep waffle
> > building against older versions.
>
> I think I understand you, but I'm not confident. Wayland's header
> dependencies
> are, we can all admit, confusing.
>
> If Waffle does the following...
>
> a. Check into the repo the wayland-client-protocol.h from Wayland 1.9.
>
> ... then ...
>
> c. Waffle will successfully build against distro-provided Wayland
> headers
>for wayland >= 1.9. Specifically, the system's wayland-client.h will
>include Waffle's imported wayland-client-protocol.h, and nothing
> will
>explode.
>
> d. If Waffle is built against the system's wayland-client.h from
> Wayland
>1.x (where x >= 9), the libwaffle can successfully dlopen and run
> against
>libwayland 1.y (where y > x).
>
> Jason, is that correct?
>

It should be, yes.  It's best to make sure that the header file you stash
in

[waffle] [PATCH] egl: Support robust access contexts with EGL 1.5.

2016-04-15 Thread Bas Nieuwenhuizen
EGL 1.5 also supports the EGL_CONTEXT_OPENGL_ROBUST_ACCESS
attribute without any extensions for both ES and non-ES.

Signed-off-by: Bas Nieuwenhuizen 
---
 src/waffle/egl/wegl_config.c  | 9 ++---
 src/waffle/egl/wegl_context.c | 9 +++--
 src/waffle/egl/wegl_display.c | 3 +--
 src/waffle/egl/wegl_display.h | 2 ++
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/waffle/egl/wegl_config.c b/src/waffle/egl/wegl_config.c
index 1c3f416..08e06fb 100644
--- a/src/waffle/egl/wegl_config.c
+++ b/src/waffle/egl/wegl_config.c
@@ -56,17 +56,20 @@ check_context_attrs(struct wegl_display *dpy,
 }
 
 if (attrs->context_robust && !dpy->EXT_create_context_robustness &&
+dpy->major_version == 1 && dpy->minor_version < 5 &&
 attrs->context_api != WAFFLE_CONTEXT_OPENGL) {
 wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
- "EGL_EXT_create_context_robustness is required in order 
to "
- "request a robust access context for OpenGL ES");
+ "EGL_EXT_create_context_robustness or EGL 1.5 is "
+ "required in order to request a robust access context "
+ "for OpenGL ES");
 return false;
 }
 
 if (attrs->context_robust && !dpy->KHR_create_context &&
+dpy->major_version == 1 && dpy->minor_version < 5 &&
 attrs->context_api == WAFFLE_CONTEXT_OPENGL) {
 wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
- "EGL_KHR_create_context is required in order to "
+ "EGL_KHR_create_context or EGL 1.5 is required in order 
to "
  "request a robust access context for OpenGL");
 return false;
 }
diff --git a/src/waffle/egl/wegl_context.c b/src/waffle/egl/wegl_context.c
index 67cbc04..4b208fa 100644
--- a/src/waffle/egl/wegl_context.c
+++ b/src/waffle/egl/wegl_context.c
@@ -96,8 +96,13 @@ create_real_context(struct wegl_config *config,
 }
 
 if (attrs->context_robust) {
-assert(dpy->KHR_create_context);
-context_flags |= EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR;
+if (dpy->major_version > 1 || dpy->minor_version >= 5) {
+attrib_list[i++] = EGL_CONTEXT_OPENGL_ROBUST_ACCESS_EXT;
+attrib_list[i++] = EGL_TRUE;
+} else {
+assert(dpy->KHR_create_context);
+context_flags |= EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR;
+}
 }
 
 if (wcore_config_attrs_version_ge(attrs, 32))  {
diff --git a/src/waffle/egl/wegl_display.c b/src/waffle/egl/wegl_display.c
index 16af142..a5e3429 100644
--- a/src/waffle/egl/wegl_display.c
+++ b/src/waffle/egl/wegl_display.c
@@ -64,7 +64,6 @@ wegl_display_init(struct wegl_display *dpy,
 {
 struct wegl_platform *plat = wegl_platform(wc_plat);
 bool ok;
-EGLint major, minor;
 
 ok = wcore_display_init(&dpy->wcore, wc_plat);
 if (!ok)
@@ -76,7 +75,7 @@ wegl_display_init(struct wegl_display *dpy,
 goto fail;
 }
 
-ok = plat->eglInitialize(dpy->egl, &major, &minor);
+ok = plat->eglInitialize(dpy->egl, &dpy->major_version, 
&dpy->minor_version);
 if (!ok) {
 wegl_emit_error(plat, "eglInitialize");
 goto fail;
diff --git a/src/waffle/egl/wegl_display.h b/src/waffle/egl/wegl_display.h
index b82a2ec..0d03ec8 100644
--- a/src/waffle/egl/wegl_display.h
+++ b/src/waffle/egl/wegl_display.h
@@ -39,6 +39,8 @@ struct wegl_display {
 EGLDisplay egl;
 bool EXT_create_context_robustness;
 bool KHR_create_context;
+EGLint major_version;
+EGLint minor_version;
 };
 
 DEFINE_CONTAINER_CAST_FUNC(wegl_display,
-- 
2.8.0

___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] [PATCH 1/3] glx: don't use ARB_create_context with pre 3.0 contexts

2016-04-15 Thread Chad Versace
On 04/15/2016 12:30 PM, Jose Fonseca wrote:
> On 15/04/16 20:04, Chad Versace wrote:
>> On 04/06/2016 02:12 AM, Jose Fonseca wrote:
>>> On 05/04/16 22:45, Emil Velikov wrote:
 This way if the user requests GL pre 3.0 context which lacks the
 flags/extra bits which require ARB_create_context one can safely fall
 back to the normal/legacy entry point.

 This resolves piglits on non 3.0 capable drivers such as classic swrast,
 nouveau_vieux and alike.

 Cc: Jose Fonseca 
 Cc: Ilia Mirkin 
 Signed-off-by: Emil Velikov 
>>
>>
>>
 +static inline bool
 +glx_context_needs_arb_create_context(const struct wcore_config_attrs 
 *attrs)
 +{
 +if (attrs->context_api == WAFFLE_CONTEXT_OPENGL &&
 +(wcore_config_attrs_version_ge(attrs, 30) ||
 + attrs->context_forward_compatible))
 +return true;
 +
 +if (attrs->context_debug)
 +return true;
 +
 +return false;
 +}
>>
>>> Looks good to me.  Thanks.
>>>
>>> Reviewed-by: Jose Fonseca 
>>
>> I reviewed the thread on the Piglit list, and I'm in favor of this change.
>>
>> Jose and Emil, I believe the critical version should be 3.2, not 3.0. I don't
>> understand why this patch uses 3.0 as the cutoff version.  The
>> GLX_ARB_create_context spec says:
>>
>>  The presence of an OpenGL 3.2 or later implementation determines 
>> whether or
>>  not GLX_ARB_create_context_profile is required.
>>
>> And the WGL spec contains the same text.
>>
>> In other words, it never makes sense to request a 3.2 context without
>> GLX_ARB_create_context, because the availability of 3.2 mandates the
>> availability of GLX_ARB_create_context_profile.
>>
> 
> I somehow thought the requirement was introduced in 3.0, but it's indeed 3.2.

Emil, resubmit patches 1 and 2 with the version bumped to 3.2, and I'll merge 
them.

___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

2016-04-15 Thread Chad Versace
On 04/15/2016 09:36 AM, Jason Ekstrand wrote:
> On Fri, Apr 15, 2016 at 3:03 AM, Emil Velikov  > wrote:
> 
> On 15 April 2016 at 03:32, Michel Dänzer  > wrote:
> > On 15.04.2016 11:14, Michel Dänzer wrote:
> >> On 14.04.2016 22:16, Emil Velikov wrote:
> >>> On 14 April 2016 at 09:23, Michel Dänzer  > wrote:
>  From: Michel Dänzer  >
> 
>  Fixes build failure due to wl_proxy_marshal_constructor_versioned 
> being
>  unresolved when building against current wayland.
> 
>  This API was introduced in wayland 1.9.91 by commit 557032e3 ("Track
>  protocol object versions inside wl_proxy."). The waffle code doesn't
>  reference wl_proxy_marshal_constructor_versioned directly but
>  indirectly via wayland-scanner.
> 
>  v2:
>  * Add paragraph about how wl_proxy_marshal_constructor_versioned was
>    introduced. (Emil Velikov)
>  * Only resolve wl_proxy_marshal_constructor_versioned with wayland >=
>    1.9.91.
> 
>  Signed-off-by: Michel Dänzer  >
>  ---
>   src/waffle/wayland/wayland_wrapper.c | 5 +
>   src/waffle/wayland/wayland_wrapper.h | 8 
>   2 files changed, 13 insertions(+)
> 
>  diff --git a/src/waffle/wayland/wayland_wrapper.c 
> b/src/waffle/wayland/wayland_wrapper.c
>  index 6ffd5a9..fb66f9a 100644
>  --- a/src/waffle/wayland/wayland_wrapper.c
>  +++ b/src/waffle/wayland/wayland_wrapper.c
>  @@ -106,6 +106,11 @@ wayland_wrapper_init(void)
>   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_add_listener);
>   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal);
>   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor);
>  +#if WAYLAND_VERSION_MAJOR == 1 && \
>  +(WAYLAND_VERSION_MINOR > 9 || \
>  + (WAYLAND_VERSION_MINOR == 9 && WAYLAND_VERSION_MICRO >= 91))
>  +
> RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor_versioned);
>  +#endif
>   #undef RETRIEVE_WL_CLIENT_SYMBOL
> 
> >>> I am slightly worried about this approach. It adds a so called 'hidden
> >>> dependency' and with it a possibility of things going horribly wrong.
> >>> It is something that we try to avoid with mesa as the deps version at
> >>> build time != run-time one. Or in other words, one might build against
> >>> wayland 1.9 and things will go crazy as you run wayland 1.10, or vice
> >>> versa.

I prefer to avoid adding this "hidden dependency" (as Emil calls it) for
a Wayland function that Waffle doesn't use or need. Jason's solution of
importing wayland-client-protocol.h avoids that dependency, and it also
prevents this build-breakage problem from occuring in the future.

> I think this is actually mostly ok.  In the Wayland project, we were very
> careful to ensure that anything built against 1.9 would work when linked
> against 1.10.  This should continue to be the case even with waffle's
> shim-layer.
> 
> The problem is not that libwayland added a new symbol nor is it a problem
> that wayland-protocol.h was updated to use that new symbol.  The problem is
> that waffle sticks a shim layer in between wayland-protocol.h and libwayland.
> This makes the wayland-protocol.h file effectively internal to waffle but
> still updatable by the distro.  This is a layering violation.  To keep this
> from happening in the future, you probably want to just check a version of
> wayland-client-protocol.h into the waffle repo so that it doesn't change out
> from under you and make waffle just use wayland-client-core.h.  You can even
> check in the version from libwayland 1.9 if you'd like to keep waffle
> building against older versions.

I think I understand you, but I'm not confident. Wayland's header dependencies
are, we can all admit, confusing.

If Waffle does the following...

a. Check into the repo the wayland-client-protocol.h from Wayland 1.9.

... then ...

c. Waffle will successfully build against distro-provided Wayland headers
   for wayland >= 1.9. Specifically, the system's wayland-client.h will
   include Waffle's imported wayland-client-protocol.h, and nothing will
   explode.

d. If Waffle is built against the system's wayland-client.h from Wayland
   1.x (where x >= 9), the libwaffle can successfully dlopen and run against
   libwayland 1.y (where y > x).

Jason, is that correct?

To allow Waffle to continue building against older Wayland version, we may be
able to import Wayland 1.8's (instead of 1.9's) wayland-client-protocol.h, as
1.8 is the first release in which the wayland-client-protocol.h was split out
from wayland-client.h.

> Another option would be to add a wrapper around
> wl_proxy_marsh

Re: [waffle] [PATCH 1/3] glx: don't use ARB_create_context with pre 3.0 contexts

2016-04-15 Thread Jose Fonseca

On 15/04/16 20:04, Chad Versace wrote:

On 04/06/2016 02:12 AM, Jose Fonseca wrote:

On 05/04/16 22:45, Emil Velikov wrote:

This way if the user requests GL pre 3.0 context which lacks the
flags/extra bits which require ARB_create_context one can safely fall
back to the normal/legacy entry point.

This resolves piglits on non 3.0 capable drivers such as classic swrast,
nouveau_vieux and alike.

Cc: Jose Fonseca 
Cc: Ilia Mirkin 
Signed-off-by: Emil Velikov 





+static inline bool
+glx_context_needs_arb_create_context(const struct wcore_config_attrs *attrs)
+{
+if (attrs->context_api == WAFFLE_CONTEXT_OPENGL &&
+(wcore_config_attrs_version_ge(attrs, 30) ||
+ attrs->context_forward_compatible))
+return true;
+
+if (attrs->context_debug)
+return true;
+
+return false;
+}



Looks good to me.  Thanks.

Reviewed-by: Jose Fonseca 


I reviewed the thread on the Piglit list, and I'm in favor of this change.

Jose and Emil, I believe the critical version should be 3.2, not 3.0. I don't
understand why this patch uses 3.0 as the cutoff version.  The
GLX_ARB_create_context spec says:

 The presence of an OpenGL 3.2 or later implementation determines whether or
 not GLX_ARB_create_context_profile is required.

And the WGL spec contains the same text.

In other words, it never makes sense to request a 3.2 context without
GLX_ARB_create_context, because the availability of 3.2 mandates the
availability of GLX_ARB_create_context_profile.



I somehow thought the requirement was introduced in 3.0, but it's indeed 
3.2.



Jose
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] [PATCH 1/3] glx: don't use ARB_create_context with pre 3.0 contexts

2016-04-15 Thread Chad Versace
On 04/06/2016 02:12 AM, Jose Fonseca wrote:
> On 05/04/16 22:45, Emil Velikov wrote:
>> This way if the user requests GL pre 3.0 context which lacks the
>> flags/extra bits which require ARB_create_context one can safely fall
>> back to the normal/legacy entry point.
>>
>> This resolves piglits on non 3.0 capable drivers such as classic swrast,
>> nouveau_vieux and alike.
>>
>> Cc: Jose Fonseca 
>> Cc: Ilia Mirkin 
>> Signed-off-by: Emil Velikov 



>> +static inline bool
>> +glx_context_needs_arb_create_context(const struct wcore_config_attrs *attrs)
>> +{
>> +if (attrs->context_api == WAFFLE_CONTEXT_OPENGL &&
>> +(wcore_config_attrs_version_ge(attrs, 30) ||
>> + attrs->context_forward_compatible))
>> +return true;
>> +
>> +if (attrs->context_debug)
>> +return true;
>> +
>> +return false;
>> +}

> Looks good to me.  Thanks.
> 
> Reviewed-by: Jose Fonseca 

I reviewed the thread on the Piglit list, and I'm in favor of this change.

Jose and Emil, I believe the critical version should be 3.2, not 3.0. I don't
understand why this patch uses 3.0 as the cutoff version.  The
GLX_ARB_create_context spec says:

The presence of an OpenGL 3.2 or later implementation determines whether or
not GLX_ARB_create_context_profile is required.

And the WGL spec contains the same text.

In other words, it never makes sense to request a 3.2 context without
GLX_ARB_create_context, because the availability of 3.2 mandates the
availability of GLX_ARB_create_context_profile.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] [PATCH 3/3] tests/gl_basic_test: query context flags only for desktop GL 3.0 or later

2016-04-15 Thread Chad Versace
On 04/05/2016 02:45 PM, Emil Velikov wrote:
> Spotted while attemting to use classic swrast with waffle.
> 
> Signed-off-by: Emil Velikov 
> ---
>  tests/functional/gl_basic_test.c | 36 +++-
>  1 file changed, 27 insertions(+), 9 deletions(-)

Emil, I pushed this patch to master. I'm still looking at the others, and the 
relevant
Piglit discussion.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

2016-04-15 Thread Jason Ekstrand
On Fri, Apr 15, 2016 at 3:03 AM, Emil Velikov 
wrote:

> On 15 April 2016 at 03:32, Michel Dänzer  wrote:
> > On 15.04.2016 11:14, Michel Dänzer wrote:
> >> On 14.04.2016 22:16, Emil Velikov wrote:
> >>> On 14 April 2016 at 09:23, Michel Dänzer  wrote:
>  From: Michel Dänzer 
> 
>  Fixes build failure due to wl_proxy_marshal_constructor_versioned
> being
>  unresolved when building against current wayland.
> 
>  This API was introduced in wayland 1.9.91 by commit 557032e3 ("Track
>  protocol object versions inside wl_proxy."). The waffle code doesn't
>  reference wl_proxy_marshal_constructor_versioned directly but
>  indirectly via wayland-scanner.
> 
>  v2:
>  * Add paragraph about how wl_proxy_marshal_constructor_versioned was
>    introduced. (Emil Velikov)
>  * Only resolve wl_proxy_marshal_constructor_versioned with wayland >=
>    1.9.91.
> 
>  Signed-off-by: Michel Dänzer 
>  ---
>   src/waffle/wayland/wayland_wrapper.c | 5 +
>   src/waffle/wayland/wayland_wrapper.h | 8 
>   2 files changed, 13 insertions(+)
> 
>  diff --git a/src/waffle/wayland/wayland_wrapper.c
> b/src/waffle/wayland/wayland_wrapper.c
>  index 6ffd5a9..fb66f9a 100644
>  --- a/src/waffle/wayland/wayland_wrapper.c
>  +++ b/src/waffle/wayland/wayland_wrapper.c
>  @@ -106,6 +106,11 @@ wayland_wrapper_init(void)
>   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_add_listener);
>   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal);
>   RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor);
>  +#if WAYLAND_VERSION_MAJOR == 1 && \
>  +(WAYLAND_VERSION_MINOR > 9 || \
>  + (WAYLAND_VERSION_MINOR == 9 && WAYLAND_VERSION_MICRO >= 91))
>  +
> RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor_versioned);
>  +#endif
>   #undef RETRIEVE_WL_CLIENT_SYMBOL
> 
> >>> I am slightly worried about this approach. It adds a so called 'hidden
> >>> dependency' and with it a possibility of things going horribly wrong.
> >>> It is something that we try to avoid with mesa as the deps version at
> >>> build time != run-time one. Or in other words, one might build against
> >>> wayland 1.9 and things will go crazy as you run wayland 1.10, or vice
> >>> versa.
>

Sorry for the late reply.  I tried to reply yesterday but wasn't a list
member yet so it ended up in the moderation bitbucket.

I think this is actually mostly ok.  In the Wayland project, we were very
careful to ensure that anything built against 1.9 would work when linked
against 1.10.  This should continue to be the case even with waffle's
shim-layer.

The problem is not that libwayland added a new symbol nor is it a problem
that wayland-protocol.h was updated to use that new symbol.  The problem is
that waffle sticks a shim layer in between wayland-protocol.h and
libwayland.  This makes the wayland-protocol.h file effectively internal to
waffle but still updatable by the distro.  This is a layering violation.
To keep this from happening in the future, you probably want to just check
a version of wayland-client-protocol.h into the waffle repo so that it
doesn't change out from under you and make waffle just use
wayland-client-core.h.  You can even check in the version from libwayland
1.9 if you'd like to keep waffle building against older versions.

Another option would be to add a wrapper around
wl_proxy_marshal_constructor_versioned that calls
wl_proxy_marshal_constructor_versioned if it's available and falls back to
wl_proxy_marshal_constructor it it's not.  Then pull in the header from
wayland 1.10.  That way it will build and even link against any libwayland
version but will use the new versioned constructor function when it's
available.

In either case, I think checking wayland-client-protocol.h into the repo is
a must.


> >>> Obviously that's not perfect, although unavoidable. Why ? As distros
> >>> do not know about the requirement (i.e. it's not mandated at configure
> >>> time) they won't rebuild and things won't work. At the same time if
> >>> they do rebuild (again without the explicit requirement), things will
> >>> break if one needs to revert to older (yet still in version range as
> >>> per the deps list) wayland.
> >>
> >> That's not true at least for Debian and derivatives, which keep track of
> >> which symbols were added in which version and generate accordingly
> >> versioned dependencies.
> >
> > It occurred to me (just after sending out the previous post, sigh...)
> > that this automatic mechanism might not work for waffle's dependency on
> > wayland if we don't link the wayland libraries directly. Even so, IME
> > this is a common issue distro maintainers of libraries have to deal
> > with, nothing particularly tricky.
> >
> Interesting - last time I've played around with Debian/derivatives, it
> did track the symbols exposed by the libraries although it did not
> check if anyone that depends on the new symbol 

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

2016-04-15 Thread Emil Velikov
On 15 April 2016 at 03:32, Michel Dänzer  wrote:
> On 15.04.2016 11:14, Michel Dänzer wrote:
>> On 14.04.2016 22:16, Emil Velikov wrote:
>>> On 14 April 2016 at 09:23, Michel Dänzer  wrote:
 From: Michel Dänzer 

 Fixes build failure due to wl_proxy_marshal_constructor_versioned being
 unresolved when building against current wayland.

 This API was introduced in wayland 1.9.91 by commit 557032e3 ("Track
 protocol object versions inside wl_proxy."). The waffle code doesn't
 reference wl_proxy_marshal_constructor_versioned directly but
 indirectly via wayland-scanner.

 v2:
 * Add paragraph about how wl_proxy_marshal_constructor_versioned was
   introduced. (Emil Velikov)
 * Only resolve wl_proxy_marshal_constructor_versioned with wayland >=
   1.9.91.

 Signed-off-by: Michel Dänzer 
 ---
  src/waffle/wayland/wayland_wrapper.c | 5 +
  src/waffle/wayland/wayland_wrapper.h | 8 
  2 files changed, 13 insertions(+)

 diff --git a/src/waffle/wayland/wayland_wrapper.c 
 b/src/waffle/wayland/wayland_wrapper.c
 index 6ffd5a9..fb66f9a 100644
 --- a/src/waffle/wayland/wayland_wrapper.c
 +++ b/src/waffle/wayland/wayland_wrapper.c
 @@ -106,6 +106,11 @@ wayland_wrapper_init(void)
  RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_add_listener);
  RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal);
  RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor);
 +#if WAYLAND_VERSION_MAJOR == 1 && \
 +(WAYLAND_VERSION_MINOR > 9 || \
 + (WAYLAND_VERSION_MINOR == 9 && WAYLAND_VERSION_MICRO >= 91))
 +RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor_versioned);
 +#endif
  #undef RETRIEVE_WL_CLIENT_SYMBOL

>>> I am slightly worried about this approach. It adds a so called 'hidden
>>> dependency' and with it a possibility of things going horribly wrong.
>>> It is something that we try to avoid with mesa as the deps version at
>>> build time != run-time one. Or in other words, one might build against
>>> wayland 1.9 and things will go crazy as you run wayland 1.10, or vice
>>> versa.
>>>
>>> Obviously that's not perfect, although unavoidable. Why ? As distros
>>> do not know about the requirement (i.e. it's not mandated at configure
>>> time) they won't rebuild and things won't work. At the same time if
>>> they do rebuild (again without the explicit requirement), things will
>>> break if one needs to revert to older (yet still in version range as
>>> per the deps list) wayland.
>>
>> That's not true at least for Debian and derivatives, which keep track of
>> which symbols were added in which version and generate accordingly
>> versioned dependencies.
>
> It occurred to me (just after sending out the previous post, sigh...)
> that this automatic mechanism might not work for waffle's dependency on
> wayland if we don't link the wayland libraries directly. Even so, IME
> this is a common issue distro maintainers of libraries have to deal
> with, nothing particularly tricky.
>
Interesting - last time I've played around with Debian/derivatives, it
did track the symbols exposed by the libraries although it did not
check if anyone that depends on the new symbol needs to be rebuild and
more importantly the versions need to be bumped. Sadly other
distributions do not do that to this moment (afaict) - Arch, Gentoo,
Fedora (?), Suse (?).

To put it bluntly - with one approach things will eventually break
[and get fixed by distro maintainers], while with the other things
will just work.

Afaict the latter does add much more code/complexity over the former, right ?

-Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle