[Piglit] [PATCH v2] Add tests for GL_AMD_depth_clamp_separate

2018-08-23 Thread Sagar Ghuge
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

2018-08-23 Thread Caio Marcelo de Oliveira Filho
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

2018-08-23 Thread Caio Marcelo de Oliveira Filho
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

2018-08-23 Thread Eric Engestrom
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

2018-08-23 Thread Caio Marcelo de Oliveira Filho
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

2018-08-23 Thread Eric Engestrom
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

2018-08-23 Thread Eric Engestrom
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