[Piglit] [PATCH v2] Add tests for GL_AMD_depth_clamp_separate
v2: 1) Use correct coding style (Ian Romanick) 2) Refactor code (Ian Romanick) 3) Merge DEPTH_CLAMP_NEAR_AMD and DEPTH_CLAMP_FAR_AMD state check in single file (Ian Romanick) 4) Add missing piglit_require_extension method (Marek Olask) Signed-off-by: Sagar Ghuge Tested-by: Marek Olšák --- tests/opengl.py | 7 + tests/spec/CMakeLists.txt | 1 + .../CMakeLists.gl.txt | 15 ++ .../amd_depth_clamp_separate/CMakeLists.txt | 1 + .../depth-clamp-range.c | 161 +++ .../depth-clamp-separate-status.c | 188 ++ 6 files changed, 373 insertions(+) create mode 100644 tests/spec/amd_depth_clamp_separate/CMakeLists.gl.txt create mode 100644 tests/spec/amd_depth_clamp_separate/CMakeLists.txt create mode 100644 tests/spec/amd_depth_clamp_separate/depth-clamp-range.c create mode 100644 tests/spec/amd_depth_clamp_separate/depth-clamp-separate-status.c diff --git a/tests/opengl.py b/tests/opengl.py index 9c5290f49..42e8b3666 100644 --- a/tests/opengl.py +++ b/tests/opengl.py @@ -1703,6 +1703,13 @@ with profile.test_list.group_manager( g(['depth-clamp-range']) g(['depth-clamp-status']) +# AMD_depth_clamp_separate +with profile.test_list.group_manager( +PiglitGLTest, grouptools.join('spec', 'AMD_depth_clamp_separate')) as g: +g(['amd_depth_clamp_separate_status']) +g(['amd_depth_clamp_separate_range']) + +# Group ARB_draw_elements_base_vertex # Group ARB_draw_elements_base_vertex with profile.test_list.group_manager( PiglitGLTest, diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt index bb3f02744..4df9d331d 100644 --- a/tests/spec/CMakeLists.txt +++ b/tests/spec/CMakeLists.txt @@ -1,4 +1,5 @@ add_subdirectory (amd_framebuffer_multisample_advanced) +add_subdirectory (amd_depth_clamp_separate) add_subdirectory (amd_performance_monitor) add_subdirectory (amd_pinned_memory) add_subdirectory (arb_arrays_of_arrays) diff --git a/tests/spec/amd_depth_clamp_separate/CMakeLists.gl.txt b/tests/spec/amd_depth_clamp_separate/CMakeLists.gl.txt new file mode 100644 index 0..4f4917262 --- /dev/null +++ b/tests/spec/amd_depth_clamp_separate/CMakeLists.gl.txt @@ -0,0 +1,15 @@ +include_directories( + ${GLEXT_INCLUDE_DIR} + ${OPENGL_INCLUDE_PATH} +) + +link_libraries ( + piglitutil_${piglit_target_api} + ${OPENGL_gl_LIBRARY} +) + +piglit_add_executable (amd_depth_clamp_separate_status + depth-clamp-separate-status.c) +piglit_add_executable (amd_depth_clamp_separate_range depth-clamp-range.c) + +# vim: ft=cmake: diff --git a/tests/spec/amd_depth_clamp_separate/CMakeLists.txt b/tests/spec/amd_depth_clamp_separate/CMakeLists.txt new file mode 100644 index 0..4a012b958 --- /dev/null +++ b/tests/spec/amd_depth_clamp_separate/CMakeLists.txt @@ -0,0 +1 @@ +piglit_include_target_api() \ No newline at end of file diff --git a/tests/spec/amd_depth_clamp_separate/depth-clamp-range.c b/tests/spec/amd_depth_clamp_separate/depth-clamp-range.c new file mode 100644 index 0..b499184d0 --- /dev/null +++ b/tests/spec/amd_depth_clamp_separate/depth-clamp-range.c @@ -0,0 +1,161 @@ +/* + * Copyright © 2018 Intel Corporation + * + * 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. + * + */ + +/** @file depth-clamp-range.c + * + * Tests that AMD_depth_clamp_separate enablement didn't break DepthRange + * functionality, and properly uses the min/max selection. + */ + +#include "piglit-util-gl.h" + +PIGLIT_GL_TEST_CONFIG_BEGIN + + config.supports_gl_core_version = 32; + config.supports_gl_compat_version = 32; + config.khr_no_error_support = PIGLIT_NO_ERRORS; + config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE + | PIGLIT_GL_VISUAL_DEPTH; +
Re: [Piglit] [PATCH piglit] util: stop overallocating shader memory
On Thu, Aug 23, 2018 at 05:50:44PM +0100, Eric Engestrom wrote: > Fixes: 606e40b2659ad7fc4ae8e "util: Add utilities to handle shader_test files" > Cc: Alejandro Piñeiro > Signed-off-by: Eric Engestrom > --- > tests/util/piglit-shader-test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/util/piglit-shader-test.c b/tests/util/piglit-shader-test.c > index f11ee8ab5383080ae090..1db6aa2eb203a50ca93e 100644 > --- a/tests/util/piglit-shader-test.c > +++ b/tests/util/piglit-shader-test.c > @@ -143,7 +143,7 @@ piglit_load_source_from_shader_test(const char *filename, > text_size = line - first_line + 1; > > if (output_source) { > - *output_source = malloc(sizeof(char*) * text_size); > + *output_source = malloc(sizeof(char) * text_size); > snprintf(*output_source, line - first_line + 1, "%s", > first_line); Also could reuse text_size here too. And I wonder if at this point, we could just call strndup with 'text_size - 1'. Haven't looked deeply but we might also have a off-by-one lurking with this text_size being returned. Thanks, Caio ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH piglit] util: stop overallocating shader memory
On Thu, Aug 23, 2018 at 05:50:44PM +0100, Eric Engestrom wrote: > Fixes: 606e40b2659ad7fc4ae8e "util: Add utilities to handle shader_test files" > Cc: Alejandro Piñeiro > Signed-off-by: Eric Engestrom > --- > tests/util/piglit-shader-test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/util/piglit-shader-test.c b/tests/util/piglit-shader-test.c > index f11ee8ab5383080ae090..1db6aa2eb203a50ca93e 100644 > --- a/tests/util/piglit-shader-test.c > +++ b/tests/util/piglit-shader-test.c > @@ -143,7 +143,7 @@ piglit_load_source_from_shader_test(const char *filename, > text_size = line - first_line + 1; > > if (output_source) { > - *output_source = malloc(sizeof(char*) * text_size); > + *output_source = malloc(sizeof(char) * text_size); > snprintf(*output_source, line - first_line + 1, "%s", > first_line); > } Good catch. Reviewed-by: Caio Marcelo de Oliveira Filho ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH piglit] util: stop overallocating shader memory
Fixes: 606e40b2659ad7fc4ae8e "util: Add utilities to handle shader_test files" Cc: Alejandro Piñeiro Signed-off-by: Eric Engestrom --- tests/util/piglit-shader-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/util/piglit-shader-test.c b/tests/util/piglit-shader-test.c index f11ee8ab5383080ae090..1db6aa2eb203a50ca93e 100644 --- a/tests/util/piglit-shader-test.c +++ b/tests/util/piglit-shader-test.c @@ -143,7 +143,7 @@ piglit_load_source_from_shader_test(const char *filename, text_size = line - first_line + 1; if (output_source) { - *output_source = malloc(sizeof(char*) * text_size); + *output_source = malloc(sizeof(char) * text_size); snprintf(*output_source, line - first_line + 1, "%s", first_line); } -- Cheers, Eric ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH piglit] util: avoid leaking memory when caller doesn't ask for it
Thanks, Eric. This patch is Reviewed-by: Caio Marcelo de Oliveira Filho On Thu, Aug 23, 2018 at 03:23:55PM +0100, Eric Engestrom wrote: > It doesn't happen anywhere right now, but a caller could say it doesn't > want the source, only its size, and in that case we would just leak that > memory. > Let's only actually allocate it when the caller wants it and will take > ownership of that memory. > > Suggested-by: Caio Marcelo de Oliveira Filho > Signed-off-by: Eric Engestrom > --- > tests/util/piglit-shader-test.c | 9 - > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/tests/util/piglit-shader-test.c b/tests/util/piglit-shader-test.c > index 4802983e56d1037be079..f11ee8ab5383080ae090 100644 > --- a/tests/util/piglit-shader-test.c > +++ b/tests/util/piglit-shader-test.c > @@ -101,7 +101,6 @@ piglit_load_source_from_shader_test(const char *filename, > unsigned *output_source_size) > { > char group_name[4096]; > - char *source = NULL; > unsigned text_size; > char *line = NULL; > char *first_line = NULL; > @@ -142,11 +141,11 @@ piglit_load_source_from_shader_test(const char > *filename, > } > > text_size = line - first_line + 1; > - source = malloc(sizeof(char*) * text_size); > - snprintf(source, line - first_line + 1, "%s", first_line); > > - if (output_source) > - *output_source = source; > + if (output_source) { > + *output_source = malloc(sizeof(char*) * text_size); > + snprintf(*output_source, line - first_line + 1, "%s", > first_line); > + } > > if (output_source_size) > *output_source_size = text_size; > -- > Cheers, > Eric > ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH piglit] util: avoid leaking memory when caller doesn't ask for it
It doesn't happen anywhere right now, but a caller could say it doesn't want the source, only its size, and in that case we would just leak that memory. Let's only actually allocate it when the caller wants it and will take ownership of that memory. Suggested-by: Caio Marcelo de Oliveira Filho Signed-off-by: Eric Engestrom --- tests/util/piglit-shader-test.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/util/piglit-shader-test.c b/tests/util/piglit-shader-test.c index 4802983e56d1037be079..f11ee8ab5383080ae090 100644 --- a/tests/util/piglit-shader-test.c +++ b/tests/util/piglit-shader-test.c @@ -101,7 +101,6 @@ piglit_load_source_from_shader_test(const char *filename, unsigned *output_source_size) { char group_name[4096]; - char *source = NULL; unsigned text_size; char *line = NULL; char *first_line = NULL; @@ -142,11 +141,11 @@ piglit_load_source_from_shader_test(const char *filename, } text_size = line - first_line + 1; - source = malloc(sizeof(char*) * text_size); - snprintf(source, line - first_line + 1, "%s", first_line); - if (output_source) - *output_source = source; + if (output_source) { + *output_source = malloc(sizeof(char*) * text_size); + snprintf(*output_source, line - first_line + 1, "%s", first_line); + } if (output_source_size) *output_source_size = text_size; -- Cheers, Eric ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH piglit 2/4] util: fix memory leak
On Wednesday, 2018-08-22 09:58:16 -0700, Caio Marcelo de Oliveira Filho wrote: > On Wed, Aug 22, 2018 at 12:38:40PM +0100, Eric Engestrom wrote: > > CovID: 1438469 (RESOURCE_LEAK) > > Fixes: 606e40b2659ad7fc4ae8e "util: Add utilities to handle shader_test > > files" > > Signed-off-by: Eric Engestrom > > --- > > tests/util/piglit-shader-test.c | 1 + > > 1 file changed, 1 insertion(+) > > This patch is > > Reviewed-by: Caio Marcelo de Oliveira Filho Thanks; I just pushed the series. > > > > > diff --git a/tests/util/piglit-shader-test.c > > b/tests/util/piglit-shader-test.c > > index 6aeb5a521a25b2d62301..4802983e56d1037be079 100644 > > --- a/tests/util/piglit-shader-test.c > > +++ b/tests/util/piglit-shader-test.c > > @@ -137,6 +137,7 @@ piglit_load_source_from_shader_test(const char > > *filename, > > if (first_line == NULL) { > > fprintf(stderr, "Could not find groupname \"%s\" on file > > \"%s\"\n", > > group_name, filename); > > + free(text); > > return false; > > } > > Since you are touching this file, a patch suggestion. A few lines > below there is > > text_size = line - first_line + 1; > source = malloc(sizeof(char*) * text_size); > snprintf(source, line - first_line + 1, "%s", first_line); > > if (output_source) > *output_source = source; > > There's a leak lurking there (we don't currently hit it, though) in > case output_source == NULL, source is ignored. Maybe just move the > malloc/snprintf to inside the block? Good catch, making that patch now ;) > > > Thanks, > Caio ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit