Re: [Piglit] [PATCH 1/2] fbo: Silence compiler warnings about const mismatches
This and the other patch are Reviewed-by: Caio Marcelo de Oliveira Filho On Tue, Jan 22, 2019 at 01:33:10PM -0800, Ian Romanick wrote: > From: Ian Romanick > > tests/fbo/fbo-blending-format-quirks.c: In function ‘piglit_display’: > tests/fbo/fbo-blending-format-quirks.c:157:40: warning: passing argument 2 of > ‘test_formats’ discards ‘const’ qualifier from pointer target type > [-Wdiscarded-qualifiers] >result = test_formats(cases[i].name, cases[i].formats, > ^ > tests/fbo/fbo-blending-format-quirks.c:45:27: note: expected ‘GLenum * {aka > unsigned int *}’ but argument is of type ‘const GLenum * {aka const unsigned > int *}’ > static enum piglit_result test_formats(const char *name, GLenum formats[2], >^~~~ > tests/fbo/fbo-blending-format-quirks.c:158:11: warning: passing argument 3 of > ‘test_formats’ discards ‘const’ qualifier from pointer target type > [-Wdiscarded-array-qualifiers] >cases[i].expect, cases[i].factors); >^ > tests/fbo/fbo-blending-format-quirks.c:45:27: note: expected ‘float (*)[4]’ > but argument is of type ‘const float (*)[4]’ > static enum piglit_result test_formats(const char *name, GLenum formats[2], >^~~~ > tests/fbo/fbo-blending-format-quirks.c:158:28: warning: passing argument 4 of > ‘test_formats’ discards ‘const’ qualifier from pointer target type > [-Wdiscarded-qualifiers] >cases[i].expect, cases[i].factors); > ^ > tests/fbo/fbo-blending-format-quirks.c:45:27: note: expected ‘GLenum * {aka > unsigned int *}’ but argument is of type ‘const GLenum * {aka const unsigned > int *}’ > static enum piglit_result test_formats(const char *name, GLenum formats[2], >^~~~ > --- > tests/fbo/fbo-blending-format-quirks.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tests/fbo/fbo-blending-format-quirks.c > b/tests/fbo/fbo-blending-format-quirks.c > index 1b71b3252..1e931e970 100644 > --- a/tests/fbo/fbo-blending-format-quirks.c > +++ b/tests/fbo/fbo-blending-format-quirks.c > @@ -42,8 +42,9 @@ PIGLIT_GL_TEST_CONFIG_BEGIN > > PIGLIT_GL_TEST_CONFIG_END > > -static enum piglit_result test_formats(const char *name, GLenum formats[2], > -float expect[2][4], GLenum factors[2]) > +static enum piglit_result > +test_formats(const char *name, const GLenum formats[2], > + const float expect[2][4], const GLenum factors[2]) > { > GLboolean pass = GL_TRUE; > GLuint tex[2], fb; > -- > 2.14.5 > > ___ > Piglit mailing list > Piglit@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/piglit Caio ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH 2/2] s3tc: Silence compiler warnings when building for GLES
From: Ian Romanick tests/texturing/s3tc-errors.c: In function ‘test_format’: tests/texturing/s3tc-errors.c:175:9: warning: unused variable ‘format2’ [-Wunused-variable] GLenum format2; ^~~ At top level: tests/texturing/s3tc-errors.c:155:1: warning: ‘check_gl_error2_’ defined but not used [-Wunused-function] check_gl_error2_(GLenum err1, GLenum err2, int line) ^~~~ --- tests/texturing/s3tc-errors.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/texturing/s3tc-errors.c b/tests/texturing/s3tc-errors.c index ccc0be9a7..509bff980 100644 --- a/tests/texturing/s3tc-errors.c +++ b/tests/texturing/s3tc-errors.c @@ -147,6 +147,7 @@ check_rendering_(int width, int height, int line) #define check_rendering(w, h) check_rendering_(w, h, __LINE__) +#ifdef PIGLIT_USE_OPENGL /** * Check for either of two expected GL errors. * XXX this could be a piglit util function @@ -164,6 +165,7 @@ check_gl_error2_(GLenum err1, GLenum err2, int line) } #define check_gl_error2(err1, err2) check_gl_error2_(err1, err2, __LINE__) +#endif static bool @@ -172,7 +174,6 @@ test_format(int width, int height, GLfloat *image, GLenum requested_format) #ifdef PIGLIT_USE_OPENGL GLubyte *compressed_image; #endif - GLenum format2; int x, y, w, h; GLuint tex; bool pass = true; @@ -350,10 +351,10 @@ test_format(int width, int height, GLfloat *image, GLenum requested_format) pass = piglit_check_gl_error(GL_INVALID_VALUE) && pass; /* Try compressed subimage with different format - should not work */ - if (format == GL_COMPRESSED_RGB_S3TC_DXT1_EXT) - format2 = GL_COMPRESSED_RGBA_S3TC_DXT5_EXT; - else - format2 = GL_COMPRESSED_RGB_S3TC_DXT1_EXT; + const GLenum format2 = format == GL_COMPRESSED_RGB_S3TC_DXT1_EXT + ? GL_COMPRESSED_RGBA_S3TC_DXT5_EXT + : GL_COMPRESSED_RGB_S3TC_DXT1_EXT; + x = 4; y = 4; w = 4; -- 2.14.5 ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH 1/2] fbo: Silence compiler warnings about const mismatches
From: Ian Romanick tests/fbo/fbo-blending-format-quirks.c: In function ‘piglit_display’: tests/fbo/fbo-blending-format-quirks.c:157:40: warning: passing argument 2 of ‘test_formats’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] result = test_formats(cases[i].name, cases[i].formats, ^ tests/fbo/fbo-blending-format-quirks.c:45:27: note: expected ‘GLenum * {aka unsigned int *}’ but argument is of type ‘const GLenum * {aka const unsigned int *}’ static enum piglit_result test_formats(const char *name, GLenum formats[2], ^~~~ tests/fbo/fbo-blending-format-quirks.c:158:11: warning: passing argument 3 of ‘test_formats’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-array-qualifiers] cases[i].expect, cases[i].factors); ^ tests/fbo/fbo-blending-format-quirks.c:45:27: note: expected ‘float (*)[4]’ but argument is of type ‘const float (*)[4]’ static enum piglit_result test_formats(const char *name, GLenum formats[2], ^~~~ tests/fbo/fbo-blending-format-quirks.c:158:28: warning: passing argument 4 of ‘test_formats’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] cases[i].expect, cases[i].factors); ^ tests/fbo/fbo-blending-format-quirks.c:45:27: note: expected ‘GLenum * {aka unsigned int *}’ but argument is of type ‘const GLenum * {aka const unsigned int *}’ static enum piglit_result test_formats(const char *name, GLenum formats[2], ^~~~ --- tests/fbo/fbo-blending-format-quirks.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/fbo/fbo-blending-format-quirks.c b/tests/fbo/fbo-blending-format-quirks.c index 1b71b3252..1e931e970 100644 --- a/tests/fbo/fbo-blending-format-quirks.c +++ b/tests/fbo/fbo-blending-format-quirks.c @@ -42,8 +42,9 @@ PIGLIT_GL_TEST_CONFIG_BEGIN PIGLIT_GL_TEST_CONFIG_END -static enum piglit_result test_formats(const char *name, GLenum formats[2], - float expect[2][4], GLenum factors[2]) +static enum piglit_result +test_formats(const char *name, const GLenum formats[2], +const float expect[2][4], const GLenum factors[2]) { GLboolean pass = GL_TRUE; GLuint tex[2], fb; -- 2.14.5 ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 0/5] modernize html
The series is Reviewed-by: Caio Marcelo de Oliveira Filho On Tue, Jan 22, 2019 at 11:41:06AM +0100, Erik Faye-Lund wrote: > It's been a while since XHTML was really a thing, so let's modernize our > HTML to follow HTML5 conventions. > > This series might not fix *everything*, mostly because I'm not that > fluent at HTML. > > Erik Faye-Lund (5): > summary/html: use html5 instead of xhtml > summary/html: drop type from css-links > summary/html: specify lang="en" in html tag > summary/html: drop trailing slash in > summary/html: drop trailing slash in > > templates/empty_status.mako | 10 -- > templates/feature.mako | 13 + > templates/index.mako| 13 + > templates/test_result.mako | 14 ++ > templates/testrun_info.mako | 10 -- > 5 files changed, 24 insertions(+), 36 deletions(-) > > -- > 2.20.1 > > ___ > Piglit mailing list > Piglit@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/piglit Caio ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH piglit v2] egl: add test for EGL_MESA_query_driver
On Tue, 22 Jan 2019 at 15:32, Eric Engestrom wrote: > > Cc: Veluri Mithun > Cc: Emil Velikov > Cc: Rob Clark > Cc: Nicolai Hähnle > Signed-off-by: Eric Engestrom > --- > The extension is currently in development in this MR: > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/47 > > Veluri will send updated versions of this test if the spec changes. > --- > tests/egl/spec/CMakeLists.txt | 1 + > .../CMakeLists.no_api.txt | 7 ++ > .../spec/egl_mesa_query_driver/CMakeLists.txt | 1 + > .../egl_mesa_query_driver.c | 100 ++ > tests/opengl.py | 6 ++ > 5 files changed, 115 insertions(+) > create mode 100644 tests/egl/spec/egl_mesa_query_driver/CMakeLists.no_api.txt > create mode 100644 tests/egl/spec/egl_mesa_query_driver/CMakeLists.txt > create mode 100644 > tests/egl/spec/egl_mesa_query_driver/egl_mesa_query_driver.c > > diff --git a/tests/egl/spec/CMakeLists.txt b/tests/egl/spec/CMakeLists.txt > index f38a4f62b6d4139719fb..378c8d7257518a4cc773 100644 > --- a/tests/egl/spec/CMakeLists.txt > +++ b/tests/egl/spec/CMakeLists.txt > @@ -12,6 +12,7 @@ add_subdirectory (egl_khr_fence_sync) > add_subdirectory (egl_khr_surfaceless_context) > add_subdirectory (egl_mesa_device_software) > add_subdirectory (egl_mesa_platform_surfaceless) > +add_subdirectory (egl_mesa_query_driver) > > if (PIGLIT_HAS_X11) > add_subdirectory (egl_chromium_sync_control) > diff --git a/tests/egl/spec/egl_mesa_query_driver/CMakeLists.no_api.txt > b/tests/egl/spec/egl_mesa_query_driver/CMakeLists.no_api.txt > new file mode 100644 > index ..d6f97ba5d294e930d050 > --- /dev/null > +++ b/tests/egl/spec/egl_mesa_query_driver/CMakeLists.no_api.txt > @@ -0,0 +1,7 @@ > +link_libraries( > + piglitutil > +) > + > +piglit_add_executable(egl_mesa_query_driver egl_mesa_query_driver.c) > + > +# vim: ft=cmake: > diff --git a/tests/egl/spec/egl_mesa_query_driver/CMakeLists.txt > b/tests/egl/spec/egl_mesa_query_driver/CMakeLists.txt > new file mode 100644 > index ..144a306f4e7d38ba7da8 > --- /dev/null > +++ b/tests/egl/spec/egl_mesa_query_driver/CMakeLists.txt > @@ -0,0 +1 @@ > +piglit_include_target_api() > diff --git a/tests/egl/spec/egl_mesa_query_driver/egl_mesa_query_driver.c > b/tests/egl/spec/egl_mesa_query_driver/egl_mesa_query_driver.c > new file mode 100644 > index ..63d3217607ceac2516d5 > --- /dev/null > +++ b/tests/egl/spec/egl_mesa_query_driver/egl_mesa_query_driver.c > @@ -0,0 +1,100 @@ > +/* > + * Copyright © 2016 Red Hat, Inc. > + * Copyright 2015 Intel Corporation > + * Copyright 2018 Collabora, Ltd. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS > + * IN THE SOFTWARE. > + */ > + > +#include "piglit-util.h" > +#include "piglit-util-egl.h" > + > +#ifndef EGL_MESA_query_driver > +typedef char *PFNEGLGETDISPLAYDRIVERCONFIGPROC(EGLDisplay disp); > +typedef const char *PFNEGLGETDISPLAYDRIVERNAMEPROC(EGLDisplay disp); > +#endif > + > +int > +main(void) > +{ > + EGLDisplay egl_display = EGL_NO_DISPLAY; > + EGLint egl_major, egl_minor; > + EGLint egl_error; > + const char *driver_name; > + char *driver_config; > + > + egl_display = eglGetDisplay(EGL_DEFAULT_DISPLAY); > + > + piglit_require_egl_extension(egl_display, "EGL_MESA_query_driver"); I think that this will fail with "extension not found". As we get the extensions via eglQueryString() function will check the dpy (_eglCheckDisplay) which in turn will _eglError(EGL_NOT_INITIALIZED) -Emil ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH piglit] egl: add test for EGL_MESA_query_driver
On Tue, 22 Jan 2019 at 15:29, Eric Engestrom wrote: > > I'd add a "TODO: add basic xml config validation/printing based on the > > example in the spec." > > That would be good in theory, but: > - we'd need to write a proper DTD, which we don't have right now > - xml parsing is not trivial (we'd need a library, it's not something > one should do by hand), and I'm not sure we want to add that kind of > beast to piglit. > > If we decide to do this, I'm happy to write the DTD. I think we could have left this for a later stage - hence the TODO. If you prefer to do it now (and have time of course) sure. Thanks Emil ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH piglit] egl: add test for EGL_MESA_query_driver
On Tuesday, 2019-01-22 15:29:44 +, Eric Engestrom wrote: > On Tuesday, 2019-01-22 13:43:07 +, Emil Velikov wrote: > > Hi Eric, > > > > Thanks for writing this up. > > > > On Tue, 22 Jan 2019 at 12:43, Eric Engestrom > > wrote: > > > > > > Cc: Veluri Mithun > > > Cc: Emil Velikov > > > Cc: Rob Clark > > > Cc: Nicolai Hähnle > > > Signed-off-by: Eric Engestrom > > > --- > > > The extension is currently in development in this MR: > > > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/47 > > > > > > Veluri will send updated versions of this test if the spec changes. > > > > > + > > > + piglit_require_egl_extension(EGL_NO_DISPLAY, > > > "EGL_MESA_query_driver"); > > > + > > AFAICT we need an valid/initialized display here. EGL_NO_DISPLAY is > > for _client_ EGL extensions and EGL_MESA_query_driver is not one. > > Good point, indeed. > > > > > > > > + egl_error = eglGetError(); > > > + if (driver_name || egl_error != EGL_BAD_DISPLAY) { > > > + printf("eglGetDisplayDriverName() should have failed with > > > EGL_BAD_DISPLAY\n"); > > > + printf("Instead, it returned %s and with error %s\n", > > > + driver_name, piglit_get_egl_error_name(egl_error)); > > > + piglit_report_result(PIGLIT_FAIL); > > > + } > > > + > > This hunk seems to be an open-coded piglit_check_egl_error(), use the > > helper instead? > > Suggestion applies for the whole file. > > Yup, didn't know about the helper :) > > > > > > + printf("Driver name: %s\n", driver_name); > > > + printf("Driver config: %s\n", driver_config); > > > + free(driver_config); > > > + > > I'd add a "TODO: add basic xml config validation/printing based on the > > example in the spec." > > That would be good in theory, but: > - we'd need to write a proper DTD, which we don't have right now > - xml parsing is not trivial (we'd need a library, it's not something > one should do by hand), and I'm not sure we want to add that kind of > beast to piglit. > > If we decide to do this, I'm happy to write the DTD. Actually, after thinking about this a bit more, I think we really should have a DTD, and it should be part of the extension spec. I've started writing it, but I don't have much time left today. I'll send it once it's done. ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH piglit v2] egl: add test for EGL_MESA_query_driver
Cc: Veluri Mithun Cc: Emil Velikov Cc: Rob Clark Cc: Nicolai Hähnle Signed-off-by: Eric Engestrom --- The extension is currently in development in this MR: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/47 Veluri will send updated versions of this test if the spec changes. --- tests/egl/spec/CMakeLists.txt | 1 + .../CMakeLists.no_api.txt | 7 ++ .../spec/egl_mesa_query_driver/CMakeLists.txt | 1 + .../egl_mesa_query_driver.c | 100 ++ tests/opengl.py | 6 ++ 5 files changed, 115 insertions(+) create mode 100644 tests/egl/spec/egl_mesa_query_driver/CMakeLists.no_api.txt create mode 100644 tests/egl/spec/egl_mesa_query_driver/CMakeLists.txt create mode 100644 tests/egl/spec/egl_mesa_query_driver/egl_mesa_query_driver.c diff --git a/tests/egl/spec/CMakeLists.txt b/tests/egl/spec/CMakeLists.txt index f38a4f62b6d4139719fb..378c8d7257518a4cc773 100644 --- a/tests/egl/spec/CMakeLists.txt +++ b/tests/egl/spec/CMakeLists.txt @@ -12,6 +12,7 @@ add_subdirectory (egl_khr_fence_sync) add_subdirectory (egl_khr_surfaceless_context) add_subdirectory (egl_mesa_device_software) add_subdirectory (egl_mesa_platform_surfaceless) +add_subdirectory (egl_mesa_query_driver) if (PIGLIT_HAS_X11) add_subdirectory (egl_chromium_sync_control) diff --git a/tests/egl/spec/egl_mesa_query_driver/CMakeLists.no_api.txt b/tests/egl/spec/egl_mesa_query_driver/CMakeLists.no_api.txt new file mode 100644 index ..d6f97ba5d294e930d050 --- /dev/null +++ b/tests/egl/spec/egl_mesa_query_driver/CMakeLists.no_api.txt @@ -0,0 +1,7 @@ +link_libraries( + piglitutil +) + +piglit_add_executable(egl_mesa_query_driver egl_mesa_query_driver.c) + +# vim: ft=cmake: diff --git a/tests/egl/spec/egl_mesa_query_driver/CMakeLists.txt b/tests/egl/spec/egl_mesa_query_driver/CMakeLists.txt new file mode 100644 index ..144a306f4e7d38ba7da8 --- /dev/null +++ b/tests/egl/spec/egl_mesa_query_driver/CMakeLists.txt @@ -0,0 +1 @@ +piglit_include_target_api() diff --git a/tests/egl/spec/egl_mesa_query_driver/egl_mesa_query_driver.c b/tests/egl/spec/egl_mesa_query_driver/egl_mesa_query_driver.c new file mode 100644 index ..63d3217607ceac2516d5 --- /dev/null +++ b/tests/egl/spec/egl_mesa_query_driver/egl_mesa_query_driver.c @@ -0,0 +1,100 @@ +/* + * Copyright © 2016 Red Hat, Inc. + * Copyright 2015 Intel Corporation + * Copyright 2018 Collabora, Ltd. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include "piglit-util.h" +#include "piglit-util-egl.h" + +#ifndef EGL_MESA_query_driver +typedef char *PFNEGLGETDISPLAYDRIVERCONFIGPROC(EGLDisplay disp); +typedef const char *PFNEGLGETDISPLAYDRIVERNAMEPROC(EGLDisplay disp); +#endif + +int +main(void) +{ + EGLDisplay egl_display = EGL_NO_DISPLAY; + EGLint egl_major, egl_minor; + EGLint egl_error; + const char *driver_name; + char *driver_config; + + egl_display = eglGetDisplay(EGL_DEFAULT_DISPLAY); + + piglit_require_egl_extension(egl_display, "EGL_MESA_query_driver"); + + PFNEGLGETDISPLAYDRIVERNAMEPROC *GetDisplayDriverName = + (void *)eglGetProcAddress("eglGetDisplayDriverName"); + PFNEGLGETDISPLAYDRIVERCONFIGPROC *GetDisplayDriverconfig = + (void *)eglGetProcAddress("eglGetDisplayDriverconfig"); + + if (!GetDisplayDriverName || !GetDisplayDriverconfig) { + printf("Query driver entrypoints missing\n"); + piglit_report_result(PIGLIT_FAIL); + } + + driver_name = GetDisplayDriverName(EGL_NO_DISPLAY); + if (!piglit_check_egl_error(EGL_BAD_DISPLAY)) + piglit_report_result(PIGLIT_FAIL); + + driver_config = GetDisplayDriverconfig(EGL_NO_DISPLAY); + if (!piglit_check_egl_error(EGL_BAD_DISPLAY)) { +
Re: [Piglit] [PATCH piglit] egl: add test for EGL_MESA_query_driver
On Tuesday, 2019-01-22 13:43:07 +, Emil Velikov wrote: > Hi Eric, > > Thanks for writing this up. > > On Tue, 22 Jan 2019 at 12:43, Eric Engestrom wrote: > > > > Cc: Veluri Mithun > > Cc: Emil Velikov > > Cc: Rob Clark > > Cc: Nicolai Hähnle > > Signed-off-by: Eric Engestrom > > --- > > The extension is currently in development in this MR: > > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/47 > > > > Veluri will send updated versions of this test if the spec changes. > > > + > > + piglit_require_egl_extension(EGL_NO_DISPLAY, > > "EGL_MESA_query_driver"); > > + > AFAICT we need an valid/initialized display here. EGL_NO_DISPLAY is > for _client_ EGL extensions and EGL_MESA_query_driver is not one. Good point, indeed. > > > > + egl_error = eglGetError(); > > + if (driver_name || egl_error != EGL_BAD_DISPLAY) { > > + printf("eglGetDisplayDriverName() should have failed with > > EGL_BAD_DISPLAY\n"); > > + printf("Instead, it returned %s and with error %s\n", > > + driver_name, piglit_get_egl_error_name(egl_error)); > > + piglit_report_result(PIGLIT_FAIL); > > + } > > + > This hunk seems to be an open-coded piglit_check_egl_error(), use the > helper instead? > Suggestion applies for the whole file. Yup, didn't know about the helper :) > > > + printf("Driver name: %s\n", driver_name); > > + printf("Driver config: %s\n", driver_config); > > + free(driver_config); > > + > I'd add a "TODO: add basic xml config validation/printing based on the > example in the spec." That would be good in theory, but: - we'd need to write a proper DTD, which we don't have right now - xml parsing is not trivial (we'd need a library, it's not something one should do by hand), and I'm not sure we want to add that kind of beast to piglit. If we decide to do this, I'm happy to write the DTD. ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] all.py: add run_concurrent=False where needed
Huh, interesting. There might be more going on than meets the eye. config.requires_displayed_window = true; I don't know much about accumulators, so ... perhaps they don't work with fbo the way one might think they do. masked-clear appears to have the same config.requires_displayed_window=true and does do front-buffer stuff. So thinking about this a bit more, I don't think fbo's have a way of having an accum buffer, so implicitly this would still use the winsys visual (or plain not work with -fbo). So this is: Reviewed-by: Ilia Mirkin On Tue, Jan 22, 2019 at 9:46 AM Arthur Huillet wrote: > > If I remember correctly, these tests ignore -fbo when passed. At any > rate, they render to a real window. If you have two of these windows at > the same time because they are run concurrently, you run into the pixel > ownership problem which is present everywhere and potentially extra > painful on the NVIDIA driver. > > On 22.01.2019 15:39, Ilia Mirkin wrote: > > run_concurrent=False implies that the test will be run with "-fbo". > > Looking at clear-accum.c, I don't see anything that will fail to work > > when run with an fbo. If piglit_probe_rect_rgb doesn't work with -fbo, > > then there are a LOT more tests that will have this problem. > > > > -ilia > > > > On Tue, Jan 22, 2019 at 5:40 AM Arthur Huillet > > wrote: > >> > >> Ping. Pixel ownership test makes it impossible for Piglit to blindly > >> assume that it can read back all the pixels of its backbuffer, so > >> tests > >> that don't use an FBO can't be run concurrently unless it can be > >> ensured > >> that the windows do not overlap. > >> One reason this seems to only show up on NVIDIA driver is that we have > >> a > >> unified backbuffer under some circumstances. > >> > >> Thanks > >> -- > >> Arthur > >> > >> On 14.11.2018 13:41, arthur.huil...@free.fr wrote: > >> > From: Arthur Huillet > >> > > >> > Commit 57537d45b75218438716506594e16b91dade968f removed > >> > run_concurrent=False > >> > from a bunch of tests, saying there was no reason for them to be marked > >> > as > >> > such. This is wrong for at least two tests, which require a displayed > >> > window > >> > (cannot render to an FBO). As such, when run concurrently they are > >> > liable to > >> > write over each other's pixels if the windows overlap, which can happen > >> > on some > >> > setups. > >> > > >> > This change fixes the problem by marking clear-accum and masked-clear > >> > as > >> > run_concurrent=False. It is likely to be the correct thing to do for > >> > all tests > >> > that require a displayed window for the same reason, but these two are > >> > the only > >> > one that were so far identified by NVIDIA. > >> > --- > >> > tests/all.py | 4 ++-- > >> > 1 file changed, 2 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/tests/all.py b/tests/all.py > >> > index 4cd911fab..e1b182f15 100644 > >> > --- a/tests/all.py > >> > +++ b/tests/all.py > >> > @@ -779,7 +779,7 @@ with profile.test_list.group_manager( > >> > g(['gl-1.1-read-pixels-after-display-list']) > >> > g(['gl-1.1-set-vertex-color-after-draw']) > >> > g(['array-stride']) > >> > -g(['clear-accum']) > >> > +g(['clear-accum'], run_concurrent=False) > >> > g(['clipflat']) > >> > g(['copypixels-draw-sync']) > >> > g(['copypixels-sync']) > >> > @@ -813,7 +813,7 @@ with profile.test_list.group_manager( > >> > g(['lineloop', '-dlist'], 'lineloop-dlist') > >> > g(['linestipple'], run_concurrent=False) > >> > g(['longprim']) > >> > -g(['masked-clear']) > >> > +g(['masked-clear'], run_concurrent=False) > >> > g(['point-line-no-cull']) > >> > g(['polygon-mode']) > >> > g(['polygon-mode-facing']) > >> > >> -- > >> A. Huillet > >> ___ > >> Piglit mailing list > >> Piglit@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/piglit > > -- > A. Huillet ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] all.py: add run_concurrent=False where needed
If I remember correctly, these tests ignore -fbo when passed. At any rate, they render to a real window. If you have two of these windows at the same time because they are run concurrently, you run into the pixel ownership problem which is present everywhere and potentially extra painful on the NVIDIA driver. On 22.01.2019 15:39, Ilia Mirkin wrote: run_concurrent=False implies that the test will be run with "-fbo". Looking at clear-accum.c, I don't see anything that will fail to work when run with an fbo. If piglit_probe_rect_rgb doesn't work with -fbo, then there are a LOT more tests that will have this problem. -ilia On Tue, Jan 22, 2019 at 5:40 AM Arthur Huillet wrote: Ping. Pixel ownership test makes it impossible for Piglit to blindly assume that it can read back all the pixels of its backbuffer, so tests that don't use an FBO can't be run concurrently unless it can be ensured that the windows do not overlap. One reason this seems to only show up on NVIDIA driver is that we have a unified backbuffer under some circumstances. Thanks -- Arthur On 14.11.2018 13:41, arthur.huil...@free.fr wrote: > From: Arthur Huillet > > Commit 57537d45b75218438716506594e16b91dade968f removed > run_concurrent=False > from a bunch of tests, saying there was no reason for them to be marked > as > such. This is wrong for at least two tests, which require a displayed > window > (cannot render to an FBO). As such, when run concurrently they are > liable to > write over each other's pixels if the windows overlap, which can happen > on some > setups. > > This change fixes the problem by marking clear-accum and masked-clear > as > run_concurrent=False. It is likely to be the correct thing to do for > all tests > that require a displayed window for the same reason, but these two are > the only > one that were so far identified by NVIDIA. > --- > tests/all.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/all.py b/tests/all.py > index 4cd911fab..e1b182f15 100644 > --- a/tests/all.py > +++ b/tests/all.py > @@ -779,7 +779,7 @@ with profile.test_list.group_manager( > g(['gl-1.1-read-pixels-after-display-list']) > g(['gl-1.1-set-vertex-color-after-draw']) > g(['array-stride']) > -g(['clear-accum']) > +g(['clear-accum'], run_concurrent=False) > g(['clipflat']) > g(['copypixels-draw-sync']) > g(['copypixels-sync']) > @@ -813,7 +813,7 @@ with profile.test_list.group_manager( > g(['lineloop', '-dlist'], 'lineloop-dlist') > g(['linestipple'], run_concurrent=False) > g(['longprim']) > -g(['masked-clear']) > +g(['masked-clear'], run_concurrent=False) > g(['point-line-no-cull']) > g(['polygon-mode']) > g(['polygon-mode-facing']) -- A. Huillet ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit -- A. Huillet ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] all.py: add run_concurrent=False where needed
run_concurrent=False implies that the test will be run with "-fbo". Looking at clear-accum.c, I don't see anything that will fail to work when run with an fbo. If piglit_probe_rect_rgb doesn't work with -fbo, then there are a LOT more tests that will have this problem. -ilia On Tue, Jan 22, 2019 at 5:40 AM Arthur Huillet wrote: > > Ping. Pixel ownership test makes it impossible for Piglit to blindly > assume that it can read back all the pixels of its backbuffer, so tests > that don't use an FBO can't be run concurrently unless it can be ensured > that the windows do not overlap. > One reason this seems to only show up on NVIDIA driver is that we have a > unified backbuffer under some circumstances. > > Thanks > -- > Arthur > > On 14.11.2018 13:41, arthur.huil...@free.fr wrote: > > From: Arthur Huillet > > > > Commit 57537d45b75218438716506594e16b91dade968f removed > > run_concurrent=False > > from a bunch of tests, saying there was no reason for them to be marked > > as > > such. This is wrong for at least two tests, which require a displayed > > window > > (cannot render to an FBO). As such, when run concurrently they are > > liable to > > write over each other's pixels if the windows overlap, which can happen > > on some > > setups. > > > > This change fixes the problem by marking clear-accum and masked-clear > > as > > run_concurrent=False. It is likely to be the correct thing to do for > > all tests > > that require a displayed window for the same reason, but these two are > > the only > > one that were so far identified by NVIDIA. > > --- > > tests/all.py | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tests/all.py b/tests/all.py > > index 4cd911fab..e1b182f15 100644 > > --- a/tests/all.py > > +++ b/tests/all.py > > @@ -779,7 +779,7 @@ with profile.test_list.group_manager( > > g(['gl-1.1-read-pixels-after-display-list']) > > g(['gl-1.1-set-vertex-color-after-draw']) > > g(['array-stride']) > > -g(['clear-accum']) > > +g(['clear-accum'], run_concurrent=False) > > g(['clipflat']) > > g(['copypixels-draw-sync']) > > g(['copypixels-sync']) > > @@ -813,7 +813,7 @@ with profile.test_list.group_manager( > > g(['lineloop', '-dlist'], 'lineloop-dlist') > > g(['linestipple'], run_concurrent=False) > > g(['longprim']) > > -g(['masked-clear']) > > +g(['masked-clear'], run_concurrent=False) > > g(['point-line-no-cull']) > > g(['polygon-mode']) > > g(['polygon-mode-facing']) > > -- > A. Huillet > ___ > Piglit mailing list > Piglit@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/piglit ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH piglit] egl: add test for EGL_MESA_query_driver
Hi Eric, Thanks for writing this up. On Tue, 22 Jan 2019 at 12:43, Eric Engestrom wrote: > > Cc: Veluri Mithun > Cc: Emil Velikov > Cc: Rob Clark > Cc: Nicolai Hähnle > Signed-off-by: Eric Engestrom > --- > The extension is currently in development in this MR: > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/47 > > Veluri will send updated versions of this test if the spec changes. > + > + piglit_require_egl_extension(EGL_NO_DISPLAY, "EGL_MESA_query_driver"); > + AFAICT we need an valid/initialized display here. EGL_NO_DISPLAY is for _client_ EGL extensions and EGL_MESA_query_driver is not one. > + egl_error = eglGetError(); > + if (driver_name || egl_error != EGL_BAD_DISPLAY) { > + printf("eglGetDisplayDriverName() should have failed with > EGL_BAD_DISPLAY\n"); > + printf("Instead, it returned %s and with error %s\n", > + driver_name, piglit_get_egl_error_name(egl_error)); > + piglit_report_result(PIGLIT_FAIL); > + } > + This hunk seems to be an open-coded piglit_check_egl_error(), use the helper instead? Suggestion applies for the whole file. > + printf("Driver name: %s\n", driver_name); > + printf("Driver config: %s\n", driver_config); > + free(driver_config); > + I'd add a "TODO: add basic xml config validation/printing based on the example in the spec." Thanks -Emil ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 0/4] summary/html: html fixes
On Tue, 2019-01-22 at 10:46 +0100, Erik Faye-Lund wrote: > On Tue, 2019-01-22 at 08:00 +0200, Tapani Pälli wrote: > > On 1/21/19 10:44 PM, Ilia Mirkin wrote: > > > Series is > > > > > > Reviewed-by: Ilia Mirkin > > > > > > As an aside, it appears that we're using xhtml? That's pretty > > > much > > > dead nowadays. The recommendation tends to be to have > > html> > > > on the first line, and that's it. > > > > Was about to say the same .. > > > > > > > > and then set charset in the part: > > > > > > > > I agree, and I think it'd be nice to move to html5 like the rest of > the > world. But I would like to land this first, as it's kinda independent > (with the exception of moving the ""-line) and solves > actual bugs. > > So how about I push out these patches, and follow it up with a (tiny) > series to modernize the html? I ended up pushing out this series (it was reviewed after all), and sending a new modernizing-series. ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH 5/5] summary/html: drop trailing slash in
This is optional in HTML5. Signed-off-by: Erik Faye-Lund --- templates/feature.mako | 2 +- templates/index.mako | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/feature.mako b/templates/feature.mako index ae8ac6fdb..1a8eae1e1 100644 --- a/templates/feature.mako +++ b/templates/feature.mako @@ -48,7 +48,7 @@ % for res in results.results: - ${res.name}\ + ${res.name}\ (info) % endfor diff --git a/templates/index.mako b/templates/index.mako index a458ea112..f9b453e80 100644 --- a/templates/index.mako +++ b/templates/index.mako @@ -95,7 +95,7 @@ % for res in results.results: - ${res.name}\ + ${res.name}\ (info) % endfor -- 2.20.1 ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] all.py: add run_concurrent=False where needed
Ping. Pixel ownership test makes it impossible for Piglit to blindly assume that it can read back all the pixels of its backbuffer, so tests that don't use an FBO can't be run concurrently unless it can be ensured that the windows do not overlap. One reason this seems to only show up on NVIDIA driver is that we have a unified backbuffer under some circumstances. Thanks -- Arthur On 14.11.2018 13:41, arthur.huil...@free.fr wrote: From: Arthur Huillet Commit 57537d45b75218438716506594e16b91dade968f removed run_concurrent=False from a bunch of tests, saying there was no reason for them to be marked as such. This is wrong for at least two tests, which require a displayed window (cannot render to an FBO). As such, when run concurrently they are liable to write over each other's pixels if the windows overlap, which can happen on some setups. This change fixes the problem by marking clear-accum and masked-clear as run_concurrent=False. It is likely to be the correct thing to do for all tests that require a displayed window for the same reason, but these two are the only one that were so far identified by NVIDIA. --- tests/all.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/all.py b/tests/all.py index 4cd911fab..e1b182f15 100644 --- a/tests/all.py +++ b/tests/all.py @@ -779,7 +779,7 @@ with profile.test_list.group_manager( g(['gl-1.1-read-pixels-after-display-list']) g(['gl-1.1-set-vertex-color-after-draw']) g(['array-stride']) -g(['clear-accum']) +g(['clear-accum'], run_concurrent=False) g(['clipflat']) g(['copypixels-draw-sync']) g(['copypixels-sync']) @@ -813,7 +813,7 @@ with profile.test_list.group_manager( g(['lineloop', '-dlist'], 'lineloop-dlist') g(['linestipple'], run_concurrent=False) g(['longprim']) -g(['masked-clear']) +g(['masked-clear'], run_concurrent=False) g(['point-line-no-cull']) g(['polygon-mode']) g(['polygon-mode-facing']) -- A. Huillet ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH 1/5] summary/html: use html5 instead of xhtml
It's been a while since XHTML was a thing. Since the release of HTML5 it has more or less taken over. So let's get with the program. Signed-off-by: Erik Faye-Lund --- templates/empty_status.mako | 8 +++- templates/feature.mako | 9 +++-- templates/index.mako| 9 +++-- templates/test_result.mako | 8 +++- templates/testrun_info.mako | 8 +++- 5 files changed, 15 insertions(+), 27 deletions(-) diff --git a/templates/empty_status.mako b/templates/empty_status.mako index 353b08915..989a1402a 100644 --- a/templates/empty_status.mako +++ b/templates/empty_status.mako @@ -1,9 +1,7 @@ - -http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd;> -http://www.w3.org/1999/xhtml;> + + - + Result summary diff --git a/templates/feature.mako b/templates/feature.mako index 7d2c400d9..fa0b19d3e 100644 --- a/templates/feature.mako +++ b/templates/feature.mako @@ -1,5 +1,3 @@ - - <%! import posixpath # this must be posixpath, since we want /'s not \'s import re @@ -27,11 +25,10 @@ return href.replace('\\', '/') %> -http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd;> -http://www.w3.org/1999/xhtml;> + + - + Result summary diff --git a/templates/index.mako b/templates/index.mako index ba483fe9d..f3a027f69 100644 --- a/templates/index.mako +++ b/templates/index.mako @@ -1,5 +1,3 @@ - - <%! import os import posixpath # this must be posixpath, since we want /'s not \'s @@ -59,11 +57,10 @@ return href.replace('\\', '/') %> -http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd;> -http://www.w3.org/1999/xhtml;> + + - + Result summary diff --git a/templates/test_result.mako b/templates/test_result.mako index ad90b3ae5..9a6ffea7e 100644 --- a/templates/test_result.mako +++ b/templates/test_result.mako @@ -1,9 +1,7 @@ - -http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd;> -http://www.w3.org/1999/xhtml;> + + - + ${testname} - Details diff --git a/templates/testrun_info.mako b/templates/testrun_info.mako index f8d86f94a..0af4f1c43 100644 --- a/templates/testrun_info.mako +++ b/templates/testrun_info.mako @@ -1,12 +1,10 @@ <%! import six %> - -http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd;> -http://www.w3.org/1999/xhtml;> + + - + ${name} - System info -- 2.20.1 ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH 3/5] summary/html: specify lang="en" in html tag
We emit english-language logs in here, so let's also specify the document-language. This allows e.g translation-plugins to know the source-language, and automatically translate the content. Signed-off-by: Erik Faye-Lund --- templates/empty_status.mako | 2 +- templates/feature.mako | 2 +- templates/index.mako| 2 +- templates/test_result.mako | 2 +- templates/testrun_info.mako | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/templates/empty_status.mako b/templates/empty_status.mako index 023295d7e..5f31431a6 100644 --- a/templates/empty_status.mako +++ b/templates/empty_status.mako @@ -1,5 +1,5 @@ - + Result summary diff --git a/templates/feature.mako b/templates/feature.mako index 2a754baa9..ae8ac6fdb 100644 --- a/templates/feature.mako +++ b/templates/feature.mako @@ -26,7 +26,7 @@ %> - + Result summary diff --git a/templates/index.mako b/templates/index.mako index a9bc6268c..a458ea112 100644 --- a/templates/index.mako +++ b/templates/index.mako @@ -58,7 +58,7 @@ %> - + Result summary diff --git a/templates/test_result.mako b/templates/test_result.mako index 62f6989b5..766e6f338 100644 --- a/templates/test_result.mako +++ b/templates/test_result.mako @@ -1,5 +1,5 @@ - + ${testname} - Details diff --git a/templates/testrun_info.mako b/templates/testrun_info.mako index 61fb0ae4b..d18327bcc 100644 --- a/templates/testrun_info.mako +++ b/templates/testrun_info.mako @@ -2,7 +2,7 @@ import six %> - + ${name} - System info -- 2.20.1 ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH 0/5] modernize html
It's been a while since XHTML was really a thing, so let's modernize our HTML to follow HTML5 conventions. This series might not fix *everything*, mostly because I'm not that fluent at HTML. Erik Faye-Lund (5): summary/html: use html5 instead of xhtml summary/html: drop type from css-links summary/html: specify lang="en" in html tag summary/html: drop trailing slash in summary/html: drop trailing slash in templates/empty_status.mako | 10 -- templates/feature.mako | 13 + templates/index.mako| 13 + templates/test_result.mako | 14 ++ templates/testrun_info.mako | 10 -- 5 files changed, 24 insertions(+), 36 deletions(-) -- 2.20.1 ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH 4/5] summary/html: drop trailing slash in
This is optional in HTML5. Signed-off-by: Erik Faye-Lund --- templates/test_result.mako | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/test_result.mako b/templates/test_result.mako index 766e6f338..ba5f7fef7 100644 --- a/templates/test_result.mako +++ b/templates/test_result.mako @@ -39,8 +39,8 @@ % for image in images: ${image['image_desc']} - - + + % endfor -- 2.20.1 ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 0/4] summary/html: html fixes
On Tue, 2019-01-22 at 08:00 +0200, Tapani Pälli wrote: > > On 1/21/19 10:44 PM, Ilia Mirkin wrote: > > Series is > > > > Reviewed-by: Ilia Mirkin > > > > As an aside, it appears that we're using xhtml? That's pretty much > > dead nowadays. The recommendation tends to be to have > html> > > on the first line, and that's it. > > Was about to say the same .. > > > > and then set charset in the part: > > > I agree, and I think it'd be nice to move to html5 like the rest of the world. But I would like to land this first, as it's kinda independent (with the exception of moving the ""-line) and solves actual bugs. So how about I push out these patches, and follow it up with a (tiny) series to modernize the html? ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit