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

2016-04-22 Thread Chad Versace 
On 04/15/2016 03:35 PM, Emil Velikov wrote:
> On 15 April 2016 at 20:04, Chad Versace  wrote:

> 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.

The subtlety is that OpenGL 3.2 mandates the existence GLX_ARB_create_context,
but the converse is not true. In fact, Mesa supported GLX_ARB_create_context
long before it supported OpenGL 3.2.

___
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-17 Thread Jose Fonseca 

On 15/04/16 23:35, Emil Velikov wrote:

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.


Right, creating fwd compat context requires XXX_ARB_create_context, but 
your patch has `|| attrs->context_forward_compatible` so it should be 
alright.




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



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 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 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 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 1/3] glx: don't use ARB_create_context with pre 3.0 contexts

2016-04-06 Thread Jose Fonseca 

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 
---
  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..5d02bc3 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.0");
  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..8d532d3 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.0 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..b471b0e 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, 30) ||
+ attrs->context_forward_compatible))
+return true;
+
+if (attrs->context_debug)
+return true;
+
+return false;
+}




Looks good to me.  Thanks.

Reviewed-by: Jose Fonseca 
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle