Re: [Piglit] [PATCH piglit] egl_khr_create_context: Proper invalid attributes for EGL 1.5

2018-09-03 Thread Eric Engestrom
On Thursday, 2018-08-30 12:56:03 -0700, Miguel Angel Vico wrote:
> Any takers?
> 
> Thanks!
> 
> 
> On Wed, 22 Aug 2018 07:34:09 -0700
> "Miguel A. Vico"  wrote:
> 
> > When EGL_KHR_create_context was originally written,
> > EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR was an invalid
> > attribute for OpenGL ES.
> > 
> > After moving the extension to core EGL 1.5, the aforementioned
> > attribute was made valid for both OpenGL and OpenGL ES.
> > 
> > Check whether the EGL version is lower than 1.5 before checking
> > EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR is an invalid
> > attribute.
> > 
> > Signed-off-by: Miguel A Vico Moya 
> > ---
> >  .../egl/spec/egl_khr_create_context/common.c  |  5 ++--
> >  .../egl/spec/egl_khr_create_context/common.h  |  8 +++
> >  .../invalid-attribute-gles.c  | 24 +++
> >  3 files changed, 35 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/egl/spec/egl_khr_create_context/common.c 
> > b/tests/egl/spec/egl_khr_create_context/common.c
> > index a443ced97..ba0311bff 100644
> > --- a/tests/egl/spec/egl_khr_create_context/common.c
> > +++ b/tests/egl/spec/egl_khr_create_context/common.c
> > @@ -27,6 +27,8 @@
> >  
> >  static Display *dpy = NULL;
> >  EGLDisplay egl_dpy;
> > +EGLint egl_major;
> > +EGLint egl_minor;
> >  EGLConfig cfg;
> >  EGLContext ctx;
> >  
> > @@ -87,7 +89,6 @@ EGL_KHR_create_context_setup(EGLint renderable_type_mask)
> > EGL_NONE
> > };
> > EGLint count;
> > -   EGLint major, minor;
> >  
> > dpy = XOpenDisplay(NULL);
> > if (dpy == NULL) {
> > @@ -101,7 +102,7 @@ EGL_KHR_create_context_setup(EGLint 
> > renderable_type_mask)
> > piglit_report_result(PIGLIT_FAIL);
> > }
> >  
> > -   if (!eglInitialize(egl_dpy, , )) {
> > +   if (!eglInitialize(egl_dpy, _major, _minor)) {
> > fprintf(stderr, "eglInitialize() failed\n");
> > piglit_report_result(PIGLIT_FAIL);
> > }
> > diff --git a/tests/egl/spec/egl_khr_create_context/common.h 
> > b/tests/egl/spec/egl_khr_create_context/common.h
> > index f4f42760c..269610f01 100644
> > --- a/tests/egl/spec/egl_khr_create_context/common.h
> > +++ b/tests/egl/spec/egl_khr_create_context/common.h
> > @@ -51,6 +51,8 @@
> >  #endif
> >  
> >  extern EGLDisplay egl_dpy;
> > +extern EGLint egl_major;
> > +extern EGLint egl_minor;
> >  extern EGLConfig cfg;
> >  extern EGLContext ctx;
> >  
> > @@ -75,3 +77,9 @@ version_is_valid_for_context(int ctx_major, int major, 
> > int minor)
> > }
> > return false;
> >  }
> > +
> > +static inline bool
> > +check_egl_version(int major, int minor)

I would maybe add `_at_least` to the function name, but other than this
nit-pick, this looks all good to me:
Reviewed-by: Eric Engestrom 

> > +{
> > +   return ((egl_major > major) || ((egl_major == major) && (egl_minor >= 
> > minor)));
> > +}
> > diff --git a/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c 
> > b/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
> > index 7d23e5673..d2b98818c 100644
> > --- a/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
> > +++ b/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
> > @@ -79,6 +79,22 @@ int main(int argc, char **argv)
> >  *attribute is only meaningful for OpenGL contexts, and specifying 
> > it
> >  *for other types of contexts, including OpenGL ES contexts, will
> >  *generate an error."
> > +*
> > +* However, after making the extension part of core EGL 1.5,
> > +* EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR was made a valid
> > +* attribute for OpenGL ES contexts:
> > +*
> > +*"The attribute EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY
> > +*specifies reset notification behavior for a context supporting
> > +*robust buffer access.  The attribute value may be either
> > +*EGL_NO_RESET_NOTIFICATION or EGL_LOSE_CONTEXT_ON_RESET, which
> > +*respectively result in reset notification behavior of
> > +*GL_NO_RESET_NOTIFICATION_ARB and GL_LOSE_CONTEXT_ON_RESET_ARB, as
> > +*described by the OpenGL GL_ARB_robustness extension, or by
> > +*equivalent functionality.
> > +*
> > +*This attribute is supported only for OpenGL and OpenGL ES
> > +*contexts."
> >  */
> > EGLint bad_attributes[] = {
> > 0x,
> > @@ -97,6 +113,14 @@ int main(int argc, char **argv)
> > }
> >  
> > for (i = 0; i < ARRAY_SIZE(bad_attributes); i++) {
> > +   /* Skip EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR if
> > +* it's EGL 1.5+
> > +*/
> > +   if ((bad_attributes[i] == 
> > EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR) &&
> > +   check_egl_version(1, 5)) {
> > +   continue;
> > +   }
> > +
> > pass = try_attribute(bad_attributes[i]) && pass;
> > }
> >  
> > 

Re: [Piglit] [PATCH piglit] egl_khr_create_context: Proper invalid attributes for EGL 1.5

2018-08-30 Thread Miguel Angel Vico
(Explicitly adding Matt and Chad, since they seem to have touched this
file in the past.)

Matt, Chad, can you take a look, or otherwise point me to whomever
would be an appropriate reviewer?

Note that I don't have push permissions :-)

Thanks!!


On Thu, 30 Aug 2018 12:56:03 -0700
Miguel Angel Vico  wrote:

> Any takers?
> 
> Thanks!
> 
> 
> On Wed, 22 Aug 2018 07:34:09 -0700
> "Miguel A. Vico"  wrote:
> 
> > When EGL_KHR_create_context was originally written,
> > EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR was an invalid
> > attribute for OpenGL ES.
> > 
> > After moving the extension to core EGL 1.5, the aforementioned
> > attribute was made valid for both OpenGL and OpenGL ES.
> > 
> > Check whether the EGL version is lower than 1.5 before checking
> > EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR is an invalid
> > attribute.
> > 
> > Signed-off-by: Miguel A Vico Moya 
> > ---
> >  .../egl/spec/egl_khr_create_context/common.c  |  5 ++--
> >  .../egl/spec/egl_khr_create_context/common.h  |  8 +++
> >  .../invalid-attribute-gles.c  | 24 +++
> >  3 files changed, 35 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/egl/spec/egl_khr_create_context/common.c 
> > b/tests/egl/spec/egl_khr_create_context/common.c
> > index a443ced97..ba0311bff 100644
> > --- a/tests/egl/spec/egl_khr_create_context/common.c
> > +++ b/tests/egl/spec/egl_khr_create_context/common.c
> > @@ -27,6 +27,8 @@
> >  
> >  static Display *dpy = NULL;
> >  EGLDisplay egl_dpy;
> > +EGLint egl_major;
> > +EGLint egl_minor;
> >  EGLConfig cfg;
> >  EGLContext ctx;
> >  
> > @@ -87,7 +89,6 @@ EGL_KHR_create_context_setup(EGLint renderable_type_mask)
> > EGL_NONE
> > };
> > EGLint count;
> > -   EGLint major, minor;
> >  
> > dpy = XOpenDisplay(NULL);
> > if (dpy == NULL) {
> > @@ -101,7 +102,7 @@ EGL_KHR_create_context_setup(EGLint 
> > renderable_type_mask)
> > piglit_report_result(PIGLIT_FAIL);
> > }
> >  
> > -   if (!eglInitialize(egl_dpy, , )) {
> > +   if (!eglInitialize(egl_dpy, _major, _minor)) {
> > fprintf(stderr, "eglInitialize() failed\n");
> > piglit_report_result(PIGLIT_FAIL);
> > }
> > diff --git a/tests/egl/spec/egl_khr_create_context/common.h 
> > b/tests/egl/spec/egl_khr_create_context/common.h
> > index f4f42760c..269610f01 100644
> > --- a/tests/egl/spec/egl_khr_create_context/common.h
> > +++ b/tests/egl/spec/egl_khr_create_context/common.h
> > @@ -51,6 +51,8 @@
> >  #endif
> >  
> >  extern EGLDisplay egl_dpy;
> > +extern EGLint egl_major;
> > +extern EGLint egl_minor;
> >  extern EGLConfig cfg;
> >  extern EGLContext ctx;
> >  
> > @@ -75,3 +77,9 @@ version_is_valid_for_context(int ctx_major, int major, 
> > int minor)
> > }
> > return false;
> >  }
> > +
> > +static inline bool
> > +check_egl_version(int major, int minor)
> > +{
> > +   return ((egl_major > major) || ((egl_major == major) && (egl_minor >= 
> > minor)));
> > +}
> > diff --git a/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c 
> > b/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
> > index 7d23e5673..d2b98818c 100644
> > --- a/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
> > +++ b/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
> > @@ -79,6 +79,22 @@ int main(int argc, char **argv)
> >  *attribute is only meaningful for OpenGL contexts, and specifying 
> > it
> >  *for other types of contexts, including OpenGL ES contexts, will
> >  *generate an error."
> > +*
> > +* However, after making the extension part of core EGL 1.5,
> > +* EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR was made a valid
> > +* attribute for OpenGL ES contexts:
> > +*
> > +*"The attribute EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY
> > +*specifies reset notification behavior for a context supporting
> > +*robust buffer access.  The attribute value may be either
> > +*EGL_NO_RESET_NOTIFICATION or EGL_LOSE_CONTEXT_ON_RESET, which
> > +*respectively result in reset notification behavior of
> > +*GL_NO_RESET_NOTIFICATION_ARB and GL_LOSE_CONTEXT_ON_RESET_ARB, as
> > +*described by the OpenGL GL_ARB_robustness extension, or by
> > +*equivalent functionality.
> > +*
> > +*This attribute is supported only for OpenGL and OpenGL ES
> > +*contexts."
> >  */
> > EGLint bad_attributes[] = {
> > 0x,
> > @@ -97,6 +113,14 @@ int main(int argc, char **argv)
> > }
> >  
> > for (i = 0; i < ARRAY_SIZE(bad_attributes); i++) {
> > +   /* Skip EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR if
> > +* it's EGL 1.5+
> > +*/
> > +   if ((bad_attributes[i] == 
> > EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR) &&
> > +   check_egl_version(1, 5)) {
> > +   continue;
> > 

Re: [Piglit] [PATCH piglit] egl_khr_create_context: Proper invalid attributes for EGL 1.5

2018-08-30 Thread Miguel Angel Vico
Any takers?

Thanks!


On Wed, 22 Aug 2018 07:34:09 -0700
"Miguel A. Vico"  wrote:

> When EGL_KHR_create_context was originally written,
> EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR was an invalid
> attribute for OpenGL ES.
> 
> After moving the extension to core EGL 1.5, the aforementioned
> attribute was made valid for both OpenGL and OpenGL ES.
> 
> Check whether the EGL version is lower than 1.5 before checking
> EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR is an invalid
> attribute.
> 
> Signed-off-by: Miguel A Vico Moya 
> ---
>  .../egl/spec/egl_khr_create_context/common.c  |  5 ++--
>  .../egl/spec/egl_khr_create_context/common.h  |  8 +++
>  .../invalid-attribute-gles.c  | 24 +++
>  3 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/egl/spec/egl_khr_create_context/common.c 
> b/tests/egl/spec/egl_khr_create_context/common.c
> index a443ced97..ba0311bff 100644
> --- a/tests/egl/spec/egl_khr_create_context/common.c
> +++ b/tests/egl/spec/egl_khr_create_context/common.c
> @@ -27,6 +27,8 @@
>  
>  static Display *dpy = NULL;
>  EGLDisplay egl_dpy;
> +EGLint egl_major;
> +EGLint egl_minor;
>  EGLConfig cfg;
>  EGLContext ctx;
>  
> @@ -87,7 +89,6 @@ EGL_KHR_create_context_setup(EGLint renderable_type_mask)
>   EGL_NONE
>   };
>   EGLint count;
> - EGLint major, minor;
>  
>   dpy = XOpenDisplay(NULL);
>   if (dpy == NULL) {
> @@ -101,7 +102,7 @@ EGL_KHR_create_context_setup(EGLint renderable_type_mask)
>   piglit_report_result(PIGLIT_FAIL);
>   }
>  
> - if (!eglInitialize(egl_dpy, , )) {
> + if (!eglInitialize(egl_dpy, _major, _minor)) {
>   fprintf(stderr, "eglInitialize() failed\n");
>   piglit_report_result(PIGLIT_FAIL);
>   }
> diff --git a/tests/egl/spec/egl_khr_create_context/common.h 
> b/tests/egl/spec/egl_khr_create_context/common.h
> index f4f42760c..269610f01 100644
> --- a/tests/egl/spec/egl_khr_create_context/common.h
> +++ b/tests/egl/spec/egl_khr_create_context/common.h
> @@ -51,6 +51,8 @@
>  #endif
>  
>  extern EGLDisplay egl_dpy;
> +extern EGLint egl_major;
> +extern EGLint egl_minor;
>  extern EGLConfig cfg;
>  extern EGLContext ctx;
>  
> @@ -75,3 +77,9 @@ version_is_valid_for_context(int ctx_major, int major, int 
> minor)
>   }
>   return false;
>  }
> +
> +static inline bool
> +check_egl_version(int major, int minor)
> +{
> + return ((egl_major > major) || ((egl_major == major) && (egl_minor >= 
> minor)));
> +}
> diff --git a/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c 
> b/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
> index 7d23e5673..d2b98818c 100644
> --- a/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
> +++ b/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
> @@ -79,6 +79,22 @@ int main(int argc, char **argv)
>*attribute is only meaningful for OpenGL contexts, and specifying 
> it
>*for other types of contexts, including OpenGL ES contexts, will
>*generate an error."
> +  *
> +  * However, after making the extension part of core EGL 1.5,
> +  * EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR was made a valid
> +  * attribute for OpenGL ES contexts:
> +  *
> +  *"The attribute EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY
> +  *specifies reset notification behavior for a context supporting
> +  *robust buffer access.  The attribute value may be either
> +  *EGL_NO_RESET_NOTIFICATION or EGL_LOSE_CONTEXT_ON_RESET, which
> +  *respectively result in reset notification behavior of
> +  *GL_NO_RESET_NOTIFICATION_ARB and GL_LOSE_CONTEXT_ON_RESET_ARB, as
> +  *described by the OpenGL GL_ARB_robustness extension, or by
> +  *equivalent functionality.
> +  *
> +  *This attribute is supported only for OpenGL and OpenGL ES
> +  *contexts."
>*/
>   EGLint bad_attributes[] = {
>   0x,
> @@ -97,6 +113,14 @@ int main(int argc, char **argv)
>   }
>  
>   for (i = 0; i < ARRAY_SIZE(bad_attributes); i++) {
> + /* Skip EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR if
> +  * it's EGL 1.5+
> +  */
> + if ((bad_attributes[i] == 
> EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR) &&
> + check_egl_version(1, 5)) {
> + continue;
> + }
> +
>   pass = try_attribute(bad_attributes[i]) && pass;
>   }
>  
> -- 
> 2.18.0
> 


-- 
Miguel


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


[Piglit] [PATCH piglit] egl_khr_create_context: Proper invalid attributes for EGL 1.5

2018-08-22 Thread Miguel A. Vico
When EGL_KHR_create_context was originally written,
EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR was an invalid
attribute for OpenGL ES.

After moving the extension to core EGL 1.5, the aforementioned
attribute was made valid for both OpenGL and OpenGL ES.

Check whether the EGL version is lower than 1.5 before checking
EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR is an invalid
attribute.

Signed-off-by: Miguel A Vico Moya 
---
 .../egl/spec/egl_khr_create_context/common.c  |  5 ++--
 .../egl/spec/egl_khr_create_context/common.h  |  8 +++
 .../invalid-attribute-gles.c  | 24 +++
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/tests/egl/spec/egl_khr_create_context/common.c 
b/tests/egl/spec/egl_khr_create_context/common.c
index a443ced97..ba0311bff 100644
--- a/tests/egl/spec/egl_khr_create_context/common.c
+++ b/tests/egl/spec/egl_khr_create_context/common.c
@@ -27,6 +27,8 @@
 
 static Display *dpy = NULL;
 EGLDisplay egl_dpy;
+EGLint egl_major;
+EGLint egl_minor;
 EGLConfig cfg;
 EGLContext ctx;
 
@@ -87,7 +89,6 @@ EGL_KHR_create_context_setup(EGLint renderable_type_mask)
EGL_NONE
};
EGLint count;
-   EGLint major, minor;
 
dpy = XOpenDisplay(NULL);
if (dpy == NULL) {
@@ -101,7 +102,7 @@ EGL_KHR_create_context_setup(EGLint renderable_type_mask)
piglit_report_result(PIGLIT_FAIL);
}
 
-   if (!eglInitialize(egl_dpy, , )) {
+   if (!eglInitialize(egl_dpy, _major, _minor)) {
fprintf(stderr, "eglInitialize() failed\n");
piglit_report_result(PIGLIT_FAIL);
}
diff --git a/tests/egl/spec/egl_khr_create_context/common.h 
b/tests/egl/spec/egl_khr_create_context/common.h
index f4f42760c..269610f01 100644
--- a/tests/egl/spec/egl_khr_create_context/common.h
+++ b/tests/egl/spec/egl_khr_create_context/common.h
@@ -51,6 +51,8 @@
 #endif
 
 extern EGLDisplay egl_dpy;
+extern EGLint egl_major;
+extern EGLint egl_minor;
 extern EGLConfig cfg;
 extern EGLContext ctx;
 
@@ -75,3 +77,9 @@ version_is_valid_for_context(int ctx_major, int major, int 
minor)
}
return false;
 }
+
+static inline bool
+check_egl_version(int major, int minor)
+{
+   return ((egl_major > major) || ((egl_major == major) && (egl_minor >= 
minor)));
+}
diff --git a/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c 
b/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
index 7d23e5673..d2b98818c 100644
--- a/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
+++ b/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
@@ -79,6 +79,22 @@ int main(int argc, char **argv)
 *attribute is only meaningful for OpenGL contexts, and specifying 
it
 *for other types of contexts, including OpenGL ES contexts, will
 *generate an error."
+*
+* However, after making the extension part of core EGL 1.5,
+* EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR was made a valid
+* attribute for OpenGL ES contexts:
+*
+*"The attribute EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY
+*specifies reset notification behavior for a context supporting
+*robust buffer access.  The attribute value may be either
+*EGL_NO_RESET_NOTIFICATION or EGL_LOSE_CONTEXT_ON_RESET, which
+*respectively result in reset notification behavior of
+*GL_NO_RESET_NOTIFICATION_ARB and GL_LOSE_CONTEXT_ON_RESET_ARB, as
+*described by the OpenGL GL_ARB_robustness extension, or by
+*equivalent functionality.
+*
+*This attribute is supported only for OpenGL and OpenGL ES
+*contexts."
 */
EGLint bad_attributes[] = {
0x,
@@ -97,6 +113,14 @@ int main(int argc, char **argv)
}
 
for (i = 0; i < ARRAY_SIZE(bad_attributes); i++) {
+   /* Skip EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR if
+* it's EGL 1.5+
+*/
+   if ((bad_attributes[i] == 
EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR) &&
+   check_egl_version(1, 5)) {
+   continue;
+   }
+
pass = try_attribute(bad_attributes[i]) && pass;
}
 
-- 
2.18.0

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