On Tue, Oct 1, 2013 at 3:06 PM, Chad Versace <[email protected]> wrote: > On 10/01/2013 02:49 PM, Matt Turner wrote: >> >> On Fri, Sep 20, 2013 at 7:02 PM, Chad Versace >> <[email protected]> wrote: >>> >>> According to version 15 of the EGL_KHR_create_context spec, >>> EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR is a valid flag for OpenGL ES contexts. >>> But the test "EGL_KHR_create_context/invalid flag GLES" verifies that it >>> is an *invalid* flag. >>> >>> Signed-off-by: Chad Versace <[email protected]> >>> --- >>> .../spec/egl_khr_create_context/invalid-flag-gles.c | 19 >>> ++++++++++--------- >>> 1 file changed, 10 insertions(+), 9 deletions(-) >>> >>> diff --git a/tests/egl/spec/egl_khr_create_context/invalid-flag-gles.c >>> b/tests/egl/spec/egl_khr_create_context/invalid-flag-gles.c >>> index b12db20..5af773b 100644 >>> --- a/tests/egl/spec/egl_khr_create_context/invalid-flag-gles.c >>> +++ b/tests/egl/spec/egl_khr_create_context/invalid-flag-gles.c >>> @@ -60,19 +60,20 @@ int main(int argc, char **argv) >>> uint32_t flag = 0x80000000; >>> bool ran_test = false; >>> >>> - /* The EGL_KHR_create_context spec says: >>> + /* According to the EGL_KHR_create_context spec, version 15, >>> there >>> + * exists exactly one valid flag for OpenGL ES contexts: the >>> debug >>> + * flag. >>> * >>> - * "The value for attribute EGL_CONTEXT_FLAGS_KHR specifies a >>> set of >>> - * flag bits affecting the context. Flags are only defined >>> for OpenGL >>> - * context creation, and specifying a flags value other than >>> zero for >>> - * other types of contexts, including OpenGL ES contexts, >>> will generate >>> - * an error." >>> + * If the EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR flag bit is set in >>> + * EGL_CONTEXT_FLAGS_KHR, then a <debug context> will be >>> created. >>> + * [...] This bit is supported for OpenGL and OpenGL ES >>> contexts. >>> */ >>> - uint32_t first_valid_flag = 0; >>> + const EGLint valid_flags = EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR; >>> + const EGLint invalid_flags = ~valid_flags; >>> >>> if (EGL_KHR_create_context_setup(EGL_OPENGL_ES_BIT)) { >>> ran_test = true; >>> - while (flag != first_valid_flag) { >>> + while (flag & invalid_flags) { >>> pass = pass && try_flag(flag); >>> flag >>= 1; >>> } >>> @@ -83,7 +84,7 @@ int main(int argc, char **argv) >>> if (EGL_KHR_create_context_setup(EGL_OPENGL_ES2_BIT)) { >>> ran_test = true; >>> flag = 0x80000000; >>> - while (flag != first_valid_flag) { >>> + while (flag & invalid_flags) { >>> pass = pass && try_flag(flag); >>> flag >>= 1; >>> } >>> -- >>> 1.8.3.1 >> >> >> Although the code seems a bit silly doing this while (flag != >> first_valid_flag) when first_valid_flag is const 0, I did it that way >> to match the invalid-flag-gl.c test. I like what you're doing better >> though. >> >> This patch is >> >> Reviewed-by: Matt Turner <[email protected]> >> >> I'd be nice to make the invalid-flag-gl.c test behave the same way as >> this one. Maybe do a follow-on patch? > > > Sure, I'll do a follow-on patch. > > Hmm... I now see something better than my while-loop. > > while (flag) { > if (flag & invalid_flag) { > > pass = pass && try_flag(flag); > } > > flag >>= 1; > } > > With this idiom, if Khronos ever adds more valid flags such that > the invalid_flags mask has discontinuities in it, then the new loop > will correctly jump over discontinuities and continue. The loop > currently in the patch would fail to test all invalid flags below > the first valid one. :( And that was exactly the behavior I was trying > to fix. > > If I use the new proposed loop idiom in this patch, do I still have your > rb? Do you see any improvements that could be made? > }
Looks good to me. Have a R-b. I don't see any improvements to it. _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
