Hello,
Thanks for the patch.
I notice that there are a lot of whitespace errors in this patch. I
would fix them myself when I commit, but I have some questions that
might require other changes. They should be pretty apparent.
On Thu, Oct 11, 2018 at 4:35 AM <asimiklit.w...@gmail.com> wrote:
>
> From: Andrii Simiklit <andrii.simik...@globallogic.com>
>
> EGL_KHR_create_context spec says:
> "The default values for EGL_CONTEXT_MAJOR_VERSION_KHR and
> EGL_CONTEXT_MINOR_VERSION_KHR are 1 and 0 respectively."
>
> requesting a forward-compatible context for OpenGL
> versions less than 3.0 will generate an error.
>
> "* If an OpenGL context is requested and the values for attributes
> EGL_CONTEXT_MAJOR_VERSION_KHR and EGL_CONTEXT_MINOR_VERSION_KHR,
> when considered together with the value for attribute
> EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR, specify an OpenGL
> version and feature set that are not defined, than an
> EGL_BAD_MATCH error is generated.
>
> The defined versions of OpenGL at the time of writing are OpenGL
> 1.0, 1.1, 1.2, 1.3, 1.4, 1.5, 2.0, 2.1, 3.0, 3.1, 3.2, 4.0, 4.1,
> 4.2, and 4.3. Feature deprecation was introduced with OpenGL
> 3.0, so forward-compatible contexts may only be requested for
> OpenGL 3.0 and above. Thus, examples of invalid combinations of
> attributes include:
>
> - Major version < 1 or > 4
> - Major version == 1 and minor version < 0 or > 5
> - Major version == 2 and minor version < 0 or > 1
> - Major version == 3 and minor version < 0 or > 2
> - Major version == 4 and minor version < 0 or > 3
> - Forward-compatible flag set and major version < 3
>
> Because the purpose of forward-compatible contexts is to allow
> application development on a specific OpenGL version with the
> knowledge that the app will run on a future version, context
> creation will fail if
> EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR is set and the
> context version returned cannot implement exactly the requested
> version."
>
> Additionally this patch checks and an independence of the attributes order in
> list.
>
> Signed-off-by: Andrii Simiklit <andrii.simik...@globallogic.com>
> ---
> .../valid-flag-forward-compatible-gl.c | 70 +++++++++++++++----
> 1 file changed, 57 insertions(+), 13 deletions(-)
>
> diff --git
> a/tests/egl/spec/egl_khr_create_context/valid-flag-forward-compatible-gl.c
> b/tests/egl/spec/egl_khr_create_context/valid-flag-forward-compatible-gl.c
> index 42feb54d6..ea00ef595 100644
> --- a/tests/egl/spec/egl_khr_create_context/valid-flag-forward-compatible-gl.c
> +++ b/tests/egl/spec/egl_khr_create_context/valid-flag-forward-compatible-gl.c
> @@ -23,16 +23,30 @@
> #include "piglit-util-egl.h"
> #include "common.h"
>
> -int gl_version;
> +int gl_version = 0;
>
> -static bool try_flag(int flag)
> +static bool try_flag(int req_version, int flag)
This function has become strange, both returning a bool and also
calling piglit_report_result(PIGLIT_FAIL).
> {
> - const EGLint attribs[] = {
> - EGL_CONTEXT_FLAGS_KHR, flag,
> - EGL_NONE
> - };
> + bool oresult = true;
What does the 'o' mean here? Why not just call it result?
> + const unsigned vidx = req_version < 0 ? 0 : (req_version == 0) ? 1 :
> 2;
> + const bool is_forward_compatible =
> + (0 != (flag &
> EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR));
>
> - ctx = eglCreateContext(egl_dpy, cfg, EGL_NO_CONTEXT, attribs);
> + const EGLint attribs[][5] = {
> + /*------------req_version-before-case------------*/
> + { EGL_CONTEXT_MAJOR_VERSION_KHR, abs(req_version),
> + EGL_CONTEXT_FLAGS_KHR, flag,
> + EGL_NONE },
> + /*------------no-req_version-case----------------*/
> + { EGL_CONTEXT_FLAGS_KHR, flag,
> + EGL_NONE },
> + /*------------req_version-after-case-------------*/
> + { EGL_CONTEXT_FLAGS_KHR, flag,
> + EGL_CONTEXT_MAJOR_VERSION_KHR, abs(req_version),
> + EGL_NONE }
> + };
> + assert(vidx < 3);
> + ctx = eglCreateContext(egl_dpy, cfg, EGL_NO_CONTEXT, attribs[vidx]);
> if (ctx != NULL) {
> /* Get GL version in order to know whether we can test
> * EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR.
> @@ -47,6 +61,18 @@ static bool try_flag(int flag)
>
> gl_version = piglit_get_gl_version();
> }
> + if (abs(req_version) < 3 && is_forward_compatible) {
> + /* The EGL_KHR_create_context spec says:
> + *
> + * requesting a forward-compatible context for OpenGL
> + * versions less than 3.0 will generate an error
> + *
> + * The default values for
> EGL_CONTEXT_MAJOR_VERSION_KHR and
> + * EGL_CONTEXT_MINOR_VERSION_KHR are 1 and 0 respectively.
> + */
> + piglit_report_result(PIGLIT_FAIL);
Shouldn't we verify that requesting a forward-compatible with a
version < GL 3.0 returns BAD_MATCH correctly?
> + oresult = false;
> + }
> eglDestroyContext(egl_dpy, ctx);
> } else if (!piglit_check_egl_error(EGL_BAD_MATCH)) {
> /* The EGL_KHR_create_context spec says:
> @@ -60,12 +86,13 @@ static bool try_flag(int flag)
> piglit_report_result(PIGLIT_FAIL);
> }
>
> - return true;
> + return oresult;
> }
>
> int main(int argc, char **argv)
> {
> bool pass = true;
> + int req_version;
>
> if (!EGL_KHR_create_context_setup(EGL_OPENGL_BIT)) {
> fprintf(stderr, "Desktop GL not available.\n");
> @@ -77,11 +104,28 @@ int main(int argc, char **argv)
> *
> * "The default value of EGL_CONTEXT_FLAGS_KHR is zero."
> */
> - pass = pass && try_flag(0);
> - if (gl_version >= 30) {
> - pass = pass &&
> try_flag(EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR);
> - } else {
> - piglit_report_result(PIGLIT_SKIP);
> + for(req_version = 0; req_version < 4 && pass; ++req_version) {
Space after for
> + pass = pass && try_flag(req_version, 0);
> + if (gl_version >= 30) {
> + pass = pass && try_flag(req_version,
> +
> EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR);
> + } else {
> + piglit_report_result(PIGLIT_SKIP);
> + break;
> + }
> + }
> + /* The sign of a req_version is used to specifay
> EGL_CONTEXT_MAJOR_VERSION_KHR
> + * flag before or after of EGL_CONTEXT_FLAGS_KHR
> + **/
> + for(req_version = 0; req_version > -4 && pass; --req_version) {
Space after for
> + pass = pass && try_flag(req_version, 0);
> + if (gl_version >= 30) {
> + pass = pass && try_flag(req_version,
> +
> EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR);
> + } else {
> + piglit_report_result(PIGLIT_SKIP);
> + break;
> + }
> }
>
> EGL_KHR_create_context_teardown();
> --
> 2.17.1
>
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
