Re: [Mesa-dev] [PATCH v2 1/3] mesa: actually support compat profile creation with MESA_GL_VERSION_OVERRIDE

[Timothy Arceri]

On 03/05/18 01:41, Emil Velikov wrote:

On 2 May 2018 at 11:27, Timothy Arceri  wrote:

Since this has gone unnoticed for a while, it proves to be subtle. Add
some commit message elaborating on the issue/solution.


---
  src/mesa/drivers/dri/common/dri_util.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/common/dri_util.c 
b/src/mesa/drivers/dri/common/dri_util.c
index 7cb6248b130..d72f72d0756 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -389,10 +389,11 @@ driCreateContextAttribs(__DRIscreen *screen, int api,
  screen->max_gl_compat_version < 31)
 mesa_api = API_OPENGL_CORE;

-if (mesa_api == API_OPENGL_COMPAT
-&& ((ctx_config.major_version > 3)
-|| (ctx_config.major_version == 3 &&
-ctx_config.minor_version >= 2))) {
+if (mesa_api == API_OPENGL_COMPAT &&
+((ctx_config.major_version > 3) || (ctx_config.major_version == 3 &&
+ctx_config.minor_version >= 2)) &&
+!((ctx_config.major_version * 10 + ctx_config.minor_version) <=
+  screen->max_gl_compat_version)) {


Unless I'm misreading it - this seems does the opposite to what the
commit message says.
Namely it causes an error out when the major/minor (overridden or not)
is greater than the max supported one.

In other words the code 'restricts', while the summary implies 'allow'.


The existing code hard-codes a limit of 3.1. This change ignores the 
restriction if we have an environment var with a higher gl version.





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


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 1/3] mesa: actually support compat profile creation with MESA_GL_VERSION_OVERRIDE

[Emil Velikov]

On 2 May 2018 at 11:27, Timothy Arceri  wrote:

Since this has gone unnoticed for a while, it proves to be subtle. Add
some commit message elaborating on the issue/solution.

> ---
>  src/mesa/drivers/dri/common/dri_util.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/common/dri_util.c 
> b/src/mesa/drivers/dri/common/dri_util.c
> index 7cb6248b130..d72f72d0756 100644
> --- a/src/mesa/drivers/dri/common/dri_util.c
> +++ b/src/mesa/drivers/dri/common/dri_util.c
> @@ -389,10 +389,11 @@ driCreateContextAttribs(__DRIscreen *screen, int api,
>  screen->max_gl_compat_version < 31)
> mesa_api = API_OPENGL_CORE;
>
> -if (mesa_api == API_OPENGL_COMPAT
> -&& ((ctx_config.major_version > 3)
> -|| (ctx_config.major_version == 3 &&
> -ctx_config.minor_version >= 2))) {
> +if (mesa_api == API_OPENGL_COMPAT &&
> +((ctx_config.major_version > 3) || (ctx_config.major_version == 3 &&
> +ctx_config.minor_version >= 2)) 
> &&
> +!((ctx_config.major_version * 10 + ctx_config.minor_version) <=
> +  screen->max_gl_compat_version)) {

Unless I'm misreading it - this seems does the opposite to what the
commit message says.
Namely it causes an error out when the major/minor (overridden or not)
is greater than the max supported one.

In other words the code 'restricts', while the summary implies 'allow'.

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

Re: [Mesa-dev] [PATCH v2 1/3] mesa: actually support compat profile creation with MESA_GL_VERSION_OVERRIDE

[Grazvydas Ignotas]

On Wed, May 2, 2018 at 3:38 PM, Timothy Arceri  wrote:
> On 02/05/18 22:36, Timothy Arceri wrote:
>>
>> On 02/05/18 21:48, Grazvydas Ignotas wrote:
>>>
>>> On Wed, May 2, 2018 at 1:27 PM, Timothy Arceri 
>>> wrote:

 ---
   src/mesa/drivers/dri/common/dri_util.c | 9 +
   1 file changed, 5 insertions(+), 4 deletions(-)

 diff --git a/src/mesa/drivers/dri/common/dri_util.c
 b/src/mesa/drivers/dri/common/dri_util.c
 index 7cb6248b130..d72f72d0756 100644
 --- a/src/mesa/drivers/dri/common/dri_util.c
 +++ b/src/mesa/drivers/dri/common/dri_util.c
 @@ -389,10 +389,11 @@ driCreateContextAttribs(__DRIscreen *screen, int
 api,
   screen->max_gl_compat_version < 31)
  mesa_api = API_OPENGL_CORE;

 -if (mesa_api == API_OPENGL_COMPAT
 -&& ((ctx_config.major_version > 3)
 -|| (ctx_config.major_version == 3 &&
 -ctx_config.minor_version >= 2))) {
 +if (mesa_api == API_OPENGL_COMPAT &&
 +((ctx_config.major_version > 3) || (ctx_config.major_version ==
 3 &&
 +ctx_config.minor_version >=
 2)) &&
 +!((ctx_config.major_version * 10 + ctx_config.minor_version) <=
 +  screen->max_gl_compat_version)) {
>>>
>>>
>>> nit: what about using (a * 10 + b  > c) to make it easier to read than
>>> !(a * 10 + b <= c) ?
>>
>>
>> That will break 3.1 compat on radeonsi because max_gl_compat_version is -1
>> unless overridden.
>
>
> Ignore that I should have just gone to bed rather than replying. That should
> work ok.

Hmm, if max_gl_compat_version can be -1 then it is still a problem,
.major_version and .minor_version are unsigned, so the compiler does
an unsigned comparison in the end (I've checked the asm), so in both
cases (ctx_config.major_version * 10 + ctx_config.minor_version) needs
casting to int.

Gražvydas
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 1/3] mesa: actually support compat profile creation with MESA_GL_VERSION_OVERRIDE

[Timothy Arceri]

On 02/05/18 22:36, Timothy Arceri wrote:

On 02/05/18 21:48, Grazvydas Ignotas wrote:
On Wed, May 2, 2018 at 1:27 PM, Timothy Arceri  
wrote:

---
  src/mesa/drivers/dri/common/dri_util.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/common/dri_util.c 
b/src/mesa/drivers/dri/common/dri_util.c

index 7cb6248b130..d72f72d0756 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -389,10 +389,11 @@ driCreateContextAttribs(__DRIscreen *screen, 
int api,

  screen->max_gl_compat_version < 31)
 mesa_api = API_OPENGL_CORE;

-    if (mesa_api == API_OPENGL_COMPAT
-    && ((ctx_config.major_version > 3)
-    || (ctx_config.major_version == 3 &&
-    ctx_config.minor_version >= 2))) {
+    if (mesa_api == API_OPENGL_COMPAT &&
+    ((ctx_config.major_version > 3) || (ctx_config.major_version 
== 3 &&
+    ctx_config.minor_version 
>= 2)) &&

+    !((ctx_config.major_version * 10 + ctx_config.minor_version) <=
+  screen->max_gl_compat_version)) {


nit: what about using (a * 10 + b  > c) to make it easier to read than
!(a * 10 + b <= c) ?


That will break 3.1 compat on radeonsi because max_gl_compat_version is 
-1 unless overridden.


Ignore that I should have just gone to bed rather than replying. That 
should work ok.






Gražvydas
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 1/3] mesa: actually support compat profile creation with MESA_GL_VERSION_OVERRIDE

[Timothy Arceri]

On 02/05/18 21:48, Grazvydas Ignotas wrote:

On Wed, May 2, 2018 at 1:27 PM, Timothy Arceri  wrote:

---
  src/mesa/drivers/dri/common/dri_util.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/common/dri_util.c 
b/src/mesa/drivers/dri/common/dri_util.c
index 7cb6248b130..d72f72d0756 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -389,10 +389,11 @@ driCreateContextAttribs(__DRIscreen *screen, int api,
  screen->max_gl_compat_version < 31)
 mesa_api = API_OPENGL_CORE;

-if (mesa_api == API_OPENGL_COMPAT
-&& ((ctx_config.major_version > 3)
-|| (ctx_config.major_version == 3 &&
-ctx_config.minor_version >= 2))) {
+if (mesa_api == API_OPENGL_COMPAT &&
+((ctx_config.major_version > 3) || (ctx_config.major_version == 3 &&
+ctx_config.minor_version >= 2)) &&
+!((ctx_config.major_version * 10 + ctx_config.minor_version) <=
+  screen->max_gl_compat_version)) {


nit: what about using (a * 10 + b  > c) to make it easier to read than
!(a * 10 + b <= c) ?


That will break 3.1 compat on radeonsi because max_gl_compat_version is 
-1 unless overridden.




Gražvydas
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 1/3] mesa: actually support compat profile creation with MESA_GL_VERSION_OVERRIDE

[Grazvydas Ignotas]

On Wed, May 2, 2018 at 1:27 PM, Timothy Arceri  wrote:
> ---
>  src/mesa/drivers/dri/common/dri_util.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/common/dri_util.c 
> b/src/mesa/drivers/dri/common/dri_util.c
> index 7cb6248b130..d72f72d0756 100644
> --- a/src/mesa/drivers/dri/common/dri_util.c
> +++ b/src/mesa/drivers/dri/common/dri_util.c
> @@ -389,10 +389,11 @@ driCreateContextAttribs(__DRIscreen *screen, int api,
>  screen->max_gl_compat_version < 31)
> mesa_api = API_OPENGL_CORE;
>
> -if (mesa_api == API_OPENGL_COMPAT
> -&& ((ctx_config.major_version > 3)
> -|| (ctx_config.major_version == 3 &&
> -ctx_config.minor_version >= 2))) {
> +if (mesa_api == API_OPENGL_COMPAT &&
> +((ctx_config.major_version > 3) || (ctx_config.major_version == 3 &&
> +ctx_config.minor_version >= 2)) 
> &&
> +!((ctx_config.major_version * 10 + ctx_config.minor_version) <=
> +  screen->max_gl_compat_version)) {

nit: what about using (a * 10 + b  > c) to make it easier to read than
!(a * 10 + b <= c) ?

Gražvydas
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 1/3] mesa: actually support compat profile creation with MESA_GL_VERSION_OVERRIDE

[Timothy Arceri]

---
 src/mesa/drivers/dri/common/dri_util.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/common/dri_util.c 
b/src/mesa/drivers/dri/common/dri_util.c
index 7cb6248b130..d72f72d0756 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -389,10 +389,11 @@ driCreateContextAttribs(__DRIscreen *screen, int api,
 screen->max_gl_compat_version < 31)
mesa_api = API_OPENGL_CORE;
 
-if (mesa_api == API_OPENGL_COMPAT
-&& ((ctx_config.major_version > 3)
-|| (ctx_config.major_version == 3 &&
-ctx_config.minor_version >= 2))) {
+if (mesa_api == API_OPENGL_COMPAT &&
+((ctx_config.major_version > 3) || (ctx_config.major_version == 3 &&
+ctx_config.minor_version >= 2)) &&
+!((ctx_config.major_version * 10 + ctx_config.minor_version) <=
+  screen->max_gl_compat_version)) {
*error = __DRI_CTX_ERROR_BAD_API;
return NULL;
 }
-- 
2.17.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev