[Mesa-dev] [PATCH] egl: Fix the bad surface attributes combination checking for pbuffers.

2016-06-17 Thread Guillaume Charifi 
Fixes a regression induced by commit a0674ce5:
When EGL_TEXTURE_FORMAT and EGL_TEXTURE_TARGET were both specified (and
both != EGL_NO_TEXTURE), an error was instantly triggered, before the
other one had even a chance to be checked, which is obviously not the
intended behaviour.

Signed-off-by: Guillaume Charifi 
---
 src/egl/main/eglsurface.c | 34 +++---
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
index 99e24dd..61e7d47 100644
--- a/src/egl/main/eglsurface.c
+++ b/src/egl/main/eglsurface.c
@@ -73,6 +73,8 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint 
*attrib_list)
EGLint i, err = EGL_SUCCESS;
EGLint tex_target = -1;
EGLint tex_format = -1;
+   EGLint attr = EGL_NONE;
+   EGLint val = EGL_NONE;
 
if (!attrib_list)
   return EGL_SUCCESS;
@@ -81,8 +83,8 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint 
*attrib_list)
   texture_type |= EGL_PIXMAP_BIT;
 
for (i = 0; attrib_list[i] != EGL_NONE; i++) {
-  EGLint attr = attrib_list[i++];
-  EGLint val = attrib_list[i];
+  attr = attrib_list[i++];
+  val = attrib_list[i];
 
   switch (attr) {
   /* common attributes */
@@ -235,25 +237,27 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const 
EGLint *attrib_list)
  break;
   }
 
-  if (type == EGL_PBUFFER_BIT) {
- if (tex_target == -1)
-tex_target = surf->TextureTarget;
+  if (err != EGL_SUCCESS)
+ break;
+   }
 
- if (tex_format == -1)
-tex_format = surf->TextureFormat;
+   if (err == EGL_SUCCESS && type == EGL_PBUFFER_BIT) {
+  if (tex_target == -1)
+ tex_target = surf->TextureTarget;
 
- if ((tex_target == EGL_NO_TEXTURE && tex_format != EGL_NO_TEXTURE) ||
- (tex_format == EGL_NO_TEXTURE && tex_target != EGL_NO_TEXTURE)) {
-err = EGL_BAD_MATCH;
- }
-  }
+  if (tex_format == -1)
+ tex_format = surf->TextureFormat;
 
-  if (err != EGL_SUCCESS) {
- _eglLog(_EGL_WARNING, "bad surface attribute 0x%04x", attr);
- break;
+  if ((tex_target == EGL_NO_TEXTURE && tex_format != EGL_NO_TEXTURE) ||
+  (tex_format == EGL_NO_TEXTURE && tex_target != EGL_NO_TEXTURE)) {
+ attr = tex_target == EGL_NO_TEXTURE ? EGL_TEXTURE_TARGET : 
EGL_TEXTURE_FORMAT;
+ err = EGL_BAD_MATCH;
   }
}
 
+   if (err != EGL_SUCCESS)
+  _eglLog(_EGL_WARNING, "bad surface attribute 0x%04x", attr);
+
return err;
 }
 
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH] egl: Fix the bad surface attributes combination checking for pbuffers.

2016-06-20 Thread Frank Binns 

On 18/06/16 01:34, Guillaume Charifi wrote:

Fixes a regression induced by commit a0674ce5:

You should put the full hash as this shortened version won't necessarily be
unique in the future.

Also, one minor comment below.

With the hash fixed this is:
Reviewed-by: Frank Binns 


When EGL_TEXTURE_FORMAT and EGL_TEXTURE_TARGET were both specified (and
both != EGL_NO_TEXTURE), an error was instantly triggered, before the
other one had even a chance to be checked, which is obviously not the
intended behaviour.

Signed-off-by: Guillaume Charifi 
---
  src/egl/main/eglsurface.c | 34 +++---
  1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
index 99e24dd..61e7d47 100644
--- a/src/egl/main/eglsurface.c
+++ b/src/egl/main/eglsurface.c
@@ -73,6 +73,8 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint 
*attrib_list)
 EGLint i, err = EGL_SUCCESS;
 EGLint tex_target = -1;
 EGLint tex_format = -1;
+   EGLint attr = EGL_NONE;
+   EGLint val = EGL_NONE;
  
 if (!attrib_list)

return EGL_SUCCESS;
@@ -81,8 +83,8 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint 
*attrib_list)
texture_type |= EGL_PIXMAP_BIT;
  
 for (i = 0; attrib_list[i] != EGL_NONE; i++) {

-  EGLint attr = attrib_list[i++];
-  EGLint val = attrib_list[i];
+  attr = attrib_list[i++];
+  val = attrib_list[i];
  
switch (attr) {

/* common attributes */
@@ -235,25 +237,27 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const 
EGLint *attrib_list)
   break;
}
  
-  if (type == EGL_PBUFFER_BIT) {

- if (tex_target == -1)
-tex_target = surf->TextureTarget;
+  if (err != EGL_SUCCESS)
+ break;
+   }
  
- if (tex_format == -1)

-tex_format = surf->TextureFormat;
+   if (err == EGL_SUCCESS && type == EGL_PBUFFER_BIT) {
+  if (tex_target == -1)
+ tex_target = surf->TextureTarget;
  
- if ((tex_target == EGL_NO_TEXTURE && tex_format != EGL_NO_TEXTURE) ||

- (tex_format == EGL_NO_TEXTURE && tex_target != EGL_NO_TEXTURE)) {
-err = EGL_BAD_MATCH;
- }
-  }
+  if (tex_format == -1)
+ tex_format = surf->TextureFormat;

tex_target and tex_format seem unnecessary as you can just test against
surf->TextureTarget and surf->TextureFormat directly. If you do want to 
continue
using them then, you can simplify things by declaring them inside this 
block and

setting them unconditionally.

  
-  if (err != EGL_SUCCESS) {

- _eglLog(_EGL_WARNING, "bad surface attribute 0x%04x", attr);
- break;
+  if ((tex_target == EGL_NO_TEXTURE && tex_format != EGL_NO_TEXTURE) ||
+  (tex_format == EGL_NO_TEXTURE && tex_target != EGL_NO_TEXTURE)) {
+ attr = tex_target == EGL_NO_TEXTURE ? EGL_TEXTURE_TARGET : 
EGL_TEXTURE_FORMAT;
+ err = EGL_BAD_MATCH;
}
 }
  
+   if (err != EGL_SUCCESS)

+  _eglLog(_EGL_WARNING, "bad surface attribute 0x%04x", attr);
+
 return err;
  }
  


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

[Mesa-dev] [PATCH] egl: Fix the bad surface attributes combination checking for pbuffers. (v2)

2016-06-20 Thread Guillaume Charifi 
Fixes a regression induced by commit a0674ce5c41903ccd161e89abb149621bfbc40d2:
When EGL_TEXTURE_FORMAT and EGL_TEXTURE_TARGET were both specified (and
both != EGL_NO_TEXTURE), an error was instantly triggered, before the
other one had even a chance to be checked, which is obviously not the
intended behaviour.

v2: Full commit hash, remove useless variables.

Signed-off-by: Guillaume Charifi 
Reviewed-by: Frank Binns 
---
 src/egl/main/eglsurface.c | 36 +++-
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
index 99e24dd..231a5f0 100644
--- a/src/egl/main/eglsurface.c
+++ b/src/egl/main/eglsurface.c
@@ -71,8 +71,8 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint 
*attrib_list)
EGLint type = surf->Type;
EGLint texture_type = EGL_PBUFFER_BIT;
EGLint i, err = EGL_SUCCESS;
-   EGLint tex_target = -1;
-   EGLint tex_format = -1;
+   EGLint attr = EGL_NONE;
+   EGLint val = EGL_NONE;
 
if (!attrib_list)
   return EGL_SUCCESS;
@@ -81,8 +81,8 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint 
*attrib_list)
   texture_type |= EGL_PIXMAP_BIT;
 
for (i = 0; attrib_list[i] != EGL_NONE; i++) {
-  EGLint attr = attrib_list[i++];
-  EGLint val = attrib_list[i];
+  attr = attrib_list[i++];
+  val = attrib_list[i];
 
   switch (attr) {
   /* common attributes */
@@ -189,7 +189,6 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint 
*attrib_list)
 break;
  }
 
- tex_format = val;
  switch (val) {
  case EGL_TEXTURE_RGB:
  case EGL_TEXTURE_RGBA:
@@ -209,7 +208,6 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint 
*attrib_list)
 break;
  }
 
- tex_target = val;
  switch (val) {
  case EGL_TEXTURE_2D:
  case EGL_NO_TEXTURE:
@@ -235,25 +233,21 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const 
EGLint *attrib_list)
  break;
   }
 
-  if (type == EGL_PBUFFER_BIT) {
- if (tex_target == -1)
-tex_target = surf->TextureTarget;
-
- if (tex_format == -1)
-tex_format = surf->TextureFormat;
-
- if ((tex_target == EGL_NO_TEXTURE && tex_format != EGL_NO_TEXTURE) ||
- (tex_format == EGL_NO_TEXTURE && tex_target != EGL_NO_TEXTURE)) {
-err = EGL_BAD_MATCH;
- }
-  }
-
-  if (err != EGL_SUCCESS) {
- _eglLog(_EGL_WARNING, "bad surface attribute 0x%04x", attr);
+  if (err != EGL_SUCCESS)
  break;
+   }
+
+   if (err == EGL_SUCCESS && type == EGL_PBUFFER_BIT) {
+  if ((surf->TextureTarget == EGL_NO_TEXTURE && surf->TextureFormat != 
EGL_NO_TEXTURE) ||
+  (surf->TextureFormat == EGL_NO_TEXTURE && surf->TextureTarget != 
EGL_NO_TEXTURE)) {
+ attr = surf->TextureTarget == EGL_NO_TEXTURE ? EGL_TEXTURE_TARGET : 
EGL_TEXTURE_FORMAT;
+ err = EGL_BAD_MATCH;
   }
}
 
+   if (err != EGL_SUCCESS)
+  _eglLog(_EGL_WARNING, "bad surface attribute 0x%04x", attr);
+
return err;
 }
 
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH] egl: Fix the bad surface attributes combination checking for pbuffers. (v2)

2016-07-06 Thread Chad Versace 
On Mon 20 Jun 2016, Guillaume Charifi wrote:
> Fixes a regression induced by commit a0674ce5c41903ccd161e89abb149621bfbc40d2:
> When EGL_TEXTURE_FORMAT and EGL_TEXTURE_TARGET were both specified (and
> both != EGL_NO_TEXTURE), an error was instantly triggered, before the
> other one had even a chance to be checked, which is obviously not the
> intended behaviour.
> 
> v2: Full commit hash, remove useless variables.
> 
> Signed-off-by: Guillaume Charifi 
> Reviewed-by: Frank Binns 
> ---
>  src/egl/main/eglsurface.c | 36 +++-
>  1 file changed, 15 insertions(+), 21 deletions(-)

This patch looks good to me. And I checked that it didn't regress
dEQP-EGL.functional.negative_api.create_pbuffer_surface.

But I want to run the patch through Intel's Jenkins before giving my
reviewed-and-tested-by. The Jenkins run should finish before morning
(I'm US West Coast).
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl: Fix the bad surface attributes combination checking for pbuffers. (v2)

2016-07-06 Thread Chad Versace 
On Wed 06 Jul 2016, Chad Versace wrote:
> On Mon 20 Jun 2016, Guillaume Charifi wrote:
> > Fixes a regression induced by commit 
> > a0674ce5c41903ccd161e89abb149621bfbc40d2:
> > When EGL_TEXTURE_FORMAT and EGL_TEXTURE_TARGET were both specified (and
> > both != EGL_NO_TEXTURE), an error was instantly triggered, before the
> > other one had even a chance to be checked, which is obviously not the
> > intended behaviour.
> > 
> > v2: Full commit hash, remove useless variables.
> > 
> > Signed-off-by: Guillaume Charifi 
> > Reviewed-by: Frank Binns 
> > ---
> >  src/egl/main/eglsurface.c | 36 +++-
> >  1 file changed, 15 insertions(+), 21 deletions(-)
> 
> This patch looks good to me. And I checked that it didn't regress
> dEQP-EGL.functional.negative_api.create_pbuffer_surface.
> 
> But I want to run the patch through Intel's Jenkins before giving my
> reviewed-and-tested-by. The Jenkins run should finish before morning
> (I'm US West Coast).

I verified that your patch regresses no tests in 'dEQP-EGL.*', at least
on Intel. So, it's looking good. I'm still waiting for the full Jenkins
run to complete, though.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl: Fix the bad surface attributes combination checking for pbuffers. (v2)

2016-07-07 Thread Chad Versace 
On Mon 20 Jun 2016, Guillaume Charifi wrote:
> Fixes a regression induced by commit a0674ce5c41903ccd161e89abb149621bfbc40d2:
> When EGL_TEXTURE_FORMAT and EGL_TEXTURE_TARGET were both specified (and
> both != EGL_NO_TEXTURE), an error was instantly triggered, before the
> other one had even a chance to be checked, which is obviously not the
> intended behaviour.
> 
> v2: Full commit hash, remove useless variables.
> 
> Signed-off-by: Guillaume Charifi 
> Reviewed-by: Frank Binns 

I added this snippet to the commit message:

Fixes: piglit "spec/egl 1.4/eglcreatepbuffersurface and then glclear"
Fixes: piglit "spec/egl 1.4/largest possible eglcreatepbuffersurface and 
then glclear"

I pushed and gave it my
Reviewed-by: Chad Versace 


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