[Mesa-dev] [PATCH] egl: Fix the bad surface attributes combination checking for pbuffers.
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.
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)
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)
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)
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)
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