arb_blend_func_extended: Test dual src blending without second color output

Submitted by Danylo Piliaiev on July 6, 2018, 8:51 a.m.

Details

Message ID 20180706085153.5707-1-danylo.piliaiev@globallogic.com
State New
Headers show
Series "arb_blend_func_extended: Test dual src blending without second color output" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Danylo Piliaiev July 6, 2018, 8:51 a.m.
Using fragment shader without second color output should not hang gpu
when dual source blending is enabled.
It hanged Intel gen8+ GPUs when discarding fragments and depth test
being enabled.
There is also safeguard against lack of second color output in radeonsi.

Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
---
To not hang hang on Intel gen8+ GPUs this patch depends on
https://patchwork.freedesktop.org/patch/235939/
The hang itself is around 2 - 5 seconds and test produces FAIL.

 tests/opengl.py                               |   2 +
 .../execution/CMakeLists.gl.txt               |   1 +
 .../execution/CMakeLists.gles3.txt            |   1 +
 .../dual-src-blending-discard-without-src1.c  | 120 ++++++++++++++++++
 4 files changed, 124 insertions(+)
 create mode 100644 tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c

Patch hide | download patch | download mbox

diff --git a/tests/opengl.py b/tests/opengl.py
index 02110ff86..61aa197b7 100644
--- a/tests/opengl.py
+++ b/tests/opengl.py
@@ -4114,6 +4114,7 @@  with profile.test_list.group_manager(
     g(['arb_blend_func_extended-fbo-extended-blend'])
     g(['arb_blend_func_extended-fbo-extended-blend-explicit'])
     g(['arb_blend_func_extended-fbo-extended-blend-pattern'])
+    g(['arb_blend_func_extended-dual-src-blending-discard-without-src1'], run_concurrent=False)
     g(['arb_blend_func_extended-blend-api_gles2'])
     g(['arb_blend_func_extended-builtins_gles2'])
     g(['arb_blend_func_extended-bindfragdataindexed-invalid-parameters_gles3'])
@@ -4123,6 +4124,7 @@  with profile.test_list.group_manager(
     g(['arb_blend_func_extended-fbo-extended-blend-pattern_gles3'])
     g(['arb_blend_func_extended-fbo-extended-blend_gles3'])
     g(['arb_blend_func_extended-fbo-extended-blend-explicit_gles3'])
+    g(['arb_blend_func_extended-dual-src-blending-discard-without-src1_gles3'], run_concurrent=False)
 
 with profile.test_list.group_manager(
         PiglitGLTest,
diff --git a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt
index f48c352e1..09d45b72c 100644
--- a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt
+++ b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt
@@ -12,4 +12,5 @@  link_libraries (
 piglit_add_executable (arb_blend_func_extended-fbo-extended-blend fbo-extended-blend.c)
 piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-explicit fbo-extended-blend-explicit.c)
 piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-pattern fbo-extended-blend-pattern.c)
+piglit_add_executable (arb_blend_func_extended-dual-src-blending-discard-without-src1 dual-src-blending-discard-without-src1.c)
 # vim: ft=cmake:
diff --git a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt
index a70e9fa5e..fd41622bd 100644
--- a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt
+++ b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt
@@ -3,4 +3,5 @@  link_libraries(piglitutil_${piglit_target_api})
 piglit_add_executable (arb_blend_func_extended-fbo-extended-blend_${piglit_target_api} fbo-extended-blend.c)
 piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-explicit_${piglit_target_api} fbo-extended-blend-explicit.c)
 piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-pattern_${piglit_target_api} fbo-extended-blend-pattern.c)
+piglit_add_executable (arb_blend_func_extended-dual-src-blending-discard-without-src1_${piglit_target_api} dual-src-blending-discard-without-src1.c)
 # vim: ft=cmake:
diff --git a/tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c b/tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c
new file mode 100644
index 000000000..881a21421
--- /dev/null
+++ b/tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c
@@ -0,0 +1,120 @@ 
+/* Copyright © 2018 Danylo Piliaiev
+ *
+ * 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.
+ */
+
+/**
+ * Drawing with dual source blending enabled while fragment shader
+ * doesn't write into src1 is undefined but it should not hang GPU.
+ * It hanged Intel gen8+ GPUs with depth test enabled.
+ *
+ * https://bugs.freedesktop.org/show_bug.cgi?id=107088
+ */
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+#ifdef PIGLIT_USE_OPENGL
+    config.supports_gl_core_version = 31;
+#else // PIGLIT_USE_OPENGLES3
+    config.supports_gl_es_version = 30;
+#endif
+    config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DEPTH;
+    config.khr_no_error_support = PIGLIT_NO_ERRORS;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+#ifdef PIGLIT_USE_OPENGL
+static const char *vs_text =
+    "#version 130\n"
+    "in vec4 vertex;\n"
+    "void main() { gl_Position = vertex; }\n"
+    ;
+
+static const char *fs_text =
+    "#version 130\n"
+    "void main() {\n"
+    "    discard;\n"
+    "}\n"
+    ;
+#else // PIGLIT_USE_OPENGLES3
+static const char *vs_text =
+    "#version 300 es\n"
+    "in vec4 vertex;\n"
+    "void main() { gl_Position = vertex; }\n"
+    ;
+
+static const char *fs_text =
+    "#version 300 es\n"
+    "void main() {\n"
+    "    discard;\n"
+    "}\n"
+    ;
+#endif
+
+enum piglit_result
+piglit_display(void)
+{
+    return PIGLIT_FAIL;
+}
+
+void piglit_init(int argc, char **argv)
+{
+#ifdef PIGLIT_USE_OPENGL
+    piglit_require_extension("GL_ARB_blend_func_extended");
+#else // PIGLIT_USE_OPENGLES3
+    piglit_require_extension("GL_EXT_blend_func_extended");
+#endif
+
+    GLint max_dual_source;
+    glGetIntegerv(GL_MAX_DUAL_SOURCE_DRAW_BUFFERS, &max_dual_source);
+
+    if (max_dual_source < 1) {
+        fprintf(stderr,
+            "ARB_blend_func_extended requires "
+            "GL_MAX_DUAL_SOURCE_DRAW_BUFFERS >= 1. "
+            "Only got %d!\n",
+            max_dual_source);
+        piglit_report_result(PIGLIT_FAIL);
+    }
+
+    GLuint prog = piglit_build_simple_program(vs_text, fs_text);
+    glUseProgram(prog);
+
+    glEnable(GL_BLEND);
+    glBlendFuncSeparate(GL_ONE, GL_SRC1_COLOR, GL_ONE, GL_ZERO);
+
+    glEnable(GL_DEPTH_TEST);
+
+    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
+
+    piglit_draw_rect(0, 0, 1, 1);
+
+    bool pass = piglit_check_gl_error(GL_NO_ERROR);
+
+    glClearColor(1.0, 0.0, 0.0, 0.0);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    const float red[] = {1, 0, 0, 0};
+    pass = pass && piglit_check_gl_error(GL_NO_ERROR);
+    pass = pass && piglit_probe_pixel_rgb(1, 1, red);
+
+    piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
+}

Comments

On Fri, Jul 6, 2018 at 4:51 AM, Danylo Piliaiev
<danylo.piliaiev@gmail.com> wrote:
> Using fragment shader without second color output should not hang gpu
> when dual source blending is enabled.
> It hanged Intel gen8+ GPUs when discarding fragments and depth test
> being enabled.
> There is also safeguard against lack of second color output in radeonsi.
>
> Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
> ---
> To not hang hang on Intel gen8+ GPUs this patch depends on
> https://patchwork.freedesktop.org/patch/235939/
> The hang itself is around 2 - 5 seconds and test produces FAIL.
>
>  tests/opengl.py                               |   2 +
>  .../execution/CMakeLists.gl.txt               |   1 +
>  .../execution/CMakeLists.gles3.txt            |   1 +
>  .../dual-src-blending-discard-without-src1.c  | 120 ++++++++++++++++++
>  4 files changed, 124 insertions(+)
>  create mode 100644 tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c
>
> diff --git a/tests/opengl.py b/tests/opengl.py
> index 02110ff86..61aa197b7 100644
> --- a/tests/opengl.py
> +++ b/tests/opengl.py
> @@ -4114,6 +4114,7 @@ with profile.test_list.group_manager(
>      g(['arb_blend_func_extended-fbo-extended-blend'])
>      g(['arb_blend_func_extended-fbo-extended-blend-explicit'])
>      g(['arb_blend_func_extended-fbo-extended-blend-pattern'])
> +    g(['arb_blend_func_extended-dual-src-blending-discard-without-src1'], run_concurrent=False)

Why run_concurrent=False?

>      g(['arb_blend_func_extended-blend-api_gles2'])
>      g(['arb_blend_func_extended-builtins_gles2'])
>      g(['arb_blend_func_extended-bindfragdataindexed-invalid-parameters_gles3'])
> @@ -4123,6 +4124,7 @@ with profile.test_list.group_manager(
>      g(['arb_blend_func_extended-fbo-extended-blend-pattern_gles3'])
>      g(['arb_blend_func_extended-fbo-extended-blend_gles3'])
>      g(['arb_blend_func_extended-fbo-extended-blend-explicit_gles3'])
> +    g(['arb_blend_func_extended-dual-src-blending-discard-without-src1_gles3'], run_concurrent=False)
>
>  with profile.test_list.group_manager(
>          PiglitGLTest,
> diff --git a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt
> index f48c352e1..09d45b72c 100644
> --- a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt
> +++ b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt
> @@ -12,4 +12,5 @@ link_libraries (
>  piglit_add_executable (arb_blend_func_extended-fbo-extended-blend fbo-extended-blend.c)
>  piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-explicit fbo-extended-blend-explicit.c)
>  piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-pattern fbo-extended-blend-pattern.c)
> +piglit_add_executable (arb_blend_func_extended-dual-src-blending-discard-without-src1 dual-src-blending-discard-without-src1.c)
>  # vim: ft=cmake:
> diff --git a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt
> index a70e9fa5e..fd41622bd 100644
> --- a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt
> +++ b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt
> @@ -3,4 +3,5 @@ link_libraries(piglitutil_${piglit_target_api})
>  piglit_add_executable (arb_blend_func_extended-fbo-extended-blend_${piglit_target_api} fbo-extended-blend.c)
>  piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-explicit_${piglit_target_api} fbo-extended-blend-explicit.c)
>  piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-pattern_${piglit_target_api} fbo-extended-blend-pattern.c)
> +piglit_add_executable (arb_blend_func_extended-dual-src-blending-discard-without-src1_${piglit_target_api} dual-src-blending-discard-without-src1.c)
>  # vim: ft=cmake:
> diff --git a/tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c b/tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c
> new file mode 100644
> index 000000000..881a21421
> --- /dev/null
> +++ b/tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c
> @@ -0,0 +1,120 @@
> +/* Copyright © 2018 Danylo Piliaiev
> + *
> + * 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.
> + */
> +
> +/**
> + * Drawing with dual source blending enabled while fragment shader
> + * doesn't write into src1 is undefined but it should not hang GPU.
> + * It hanged Intel gen8+ GPUs with depth test enabled.
> + *
> + * https://bugs.freedesktop.org/show_bug.cgi?id=107088
> + */
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> +#ifdef PIGLIT_USE_OPENGL
> +    config.supports_gl_core_version = 31;

Sounds like gl_compat_version=30 would be enough, no?

> +#else // PIGLIT_USE_OPENGLES3
> +    config.supports_gl_es_version = 30;
> +#endif
> +    config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DEPTH;
> +    config.khr_no_error_support = PIGLIT_NO_ERRORS;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +#ifdef PIGLIT_USE_OPENGL
> +static const char *vs_text =
> +    "#version 130\n"
> +    "in vec4 vertex;\n"
> +    "void main() { gl_Position = vertex; }\n"
> +    ;
> +
> +static const char *fs_text =
> +    "#version 130\n"
> +    "void main() {\n"
> +    "    discard;\n"
> +    "}\n"
> +    ;
> +#else // PIGLIT_USE_OPENGLES3
> +static const char *vs_text =
> +    "#version 300 es\n"
> +    "in vec4 vertex;\n"
> +    "void main() { gl_Position = vertex; }\n"
> +    ;
> +
> +static const char *fs_text =
> +    "#version 300 es\n"
> +    "void main() {\n"
> +    "    discard;\n"
> +    "}\n"
> +    ;
> +#endif
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +    return PIGLIT_FAIL;
> +}
> +
> +void piglit_init(int argc, char **argv)
> +{
> +#ifdef PIGLIT_USE_OPENGL
> +    piglit_require_extension("GL_ARB_blend_func_extended");
> +#else // PIGLIT_USE_OPENGLES3
> +    piglit_require_extension("GL_EXT_blend_func_extended");
> +#endif
> +
> +    GLint max_dual_source;
> +    glGetIntegerv(GL_MAX_DUAL_SOURCE_DRAW_BUFFERS, &max_dual_source);
> +
> +    if (max_dual_source < 1) {
> +        fprintf(stderr,
> +            "ARB_blend_func_extended requires "
> +            "GL_MAX_DUAL_SOURCE_DRAW_BUFFERS >= 1. "
> +            "Only got %d!\n",
> +            max_dual_source);
> +        piglit_report_result(PIGLIT_FAIL);
> +    }
> +
> +    GLuint prog = piglit_build_simple_program(vs_text, fs_text);
> +    glUseProgram(prog);
> +
> +    glEnable(GL_BLEND);
> +    glBlendFuncSeparate(GL_ONE, GL_SRC1_COLOR, GL_ONE, GL_ZERO);
> +
> +    glEnable(GL_DEPTH_TEST);

This should be the end of piglit_init. The remainder should go into
piglit_draw (piglit_display? I forget).

> +
> +    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

glClearColor/glClearDepth would make this more explicit here.

> +
> +    piglit_draw_rect(0, 0, 1, 1);

I think this draws only a fraction of

> +
> +    bool pass = piglit_check_gl_error(GL_NO_ERROR);
> +
> +    glClearColor(1.0, 0.0, 0.0, 0.0);
> +    glClear(GL_COLOR_BUFFER_BIT);

piglit_present_results() here, that way you see the output. Why do you
draw first and clear second? The other way around makes more sense.
Also piglit tends to be structured as green = good, red = bad. If the
issue is that the latter clear never happens, you could clear it to
red, do your draw, then clear to green.

> +
> +    const float red[] = {1, 0, 0, 0};
> +    pass = pass && piglit_check_gl_error(GL_NO_ERROR);
> +    pass = pass && piglit_probe_pixel_rgb(1, 1, red);
> +
> +    piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
> +}
> --
> 2.17.1
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
Thank you for the feedback, I'll send fixed version soon.


On 06.07.18 16:48, Ilia Mirkin wrote:
> On Fri, Jul 6, 2018 at 4:51 AM, Danylo Piliaiev
> <danylo.piliaiev@gmail.com> wrote:
>> Using fragment shader without second color output should not hang gpu
>> when dual source blending is enabled.
>> It hanged Intel gen8+ GPUs when discarding fragments and depth test
>> being enabled.
>> There is also safeguard against lack of second color output in radeonsi.
>>
>> Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
>> ---
>> To not hang hang on Intel gen8+ GPUs this patch depends on
>> https://patchwork.freedesktop.org/patch/235939/
>> The hang itself is around 2 - 5 seconds and test produces FAIL.
>>
>>   tests/opengl.py                               |   2 +
>>   .../execution/CMakeLists.gl.txt               |   1 +
>>   .../execution/CMakeLists.gles3.txt            |   1 +
>>   .../dual-src-blending-discard-without-src1.c  | 120 ++++++++++++++++++
>>   4 files changed, 124 insertions(+)
>>   create mode 100644 tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c
>>
>> diff --git a/tests/opengl.py b/tests/opengl.py
>> index 02110ff86..61aa197b7 100644
>> --- a/tests/opengl.py
>> +++ b/tests/opengl.py
>> @@ -4114,6 +4114,7 @@ with profile.test_list.group_manager(
>>       g(['arb_blend_func_extended-fbo-extended-blend'])
>>       g(['arb_blend_func_extended-fbo-extended-blend-explicit'])
>>       g(['arb_blend_func_extended-fbo-extended-blend-pattern'])
>> +    g(['arb_blend_func_extended-dual-src-blending-discard-without-src1'], run_concurrent=False)
> Why run_concurrent=False?
I set it because this test may hang which can affect other tests but if 
it won't an issue I can remove it.
>
>>       g(['arb_blend_func_extended-blend-api_gles2'])
>>       g(['arb_blend_func_extended-builtins_gles2'])
>>       g(['arb_blend_func_extended-bindfragdataindexed-invalid-parameters_gles3'])
>> @@ -4123,6 +4124,7 @@ with profile.test_list.group_manager(
>>       g(['arb_blend_func_extended-fbo-extended-blend-pattern_gles3'])
>>       g(['arb_blend_func_extended-fbo-extended-blend_gles3'])
>>       g(['arb_blend_func_extended-fbo-extended-blend-explicit_gles3'])
>> +    g(['arb_blend_func_extended-dual-src-blending-discard-without-src1_gles3'], run_concurrent=False)
>>
>>   with profile.test_list.group_manager(
>>           PiglitGLTest,
>> diff --git a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt
>> index f48c352e1..09d45b72c 100644
>> --- a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt
>> +++ b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt
>> @@ -12,4 +12,5 @@ link_libraries (
>>   piglit_add_executable (arb_blend_func_extended-fbo-extended-blend fbo-extended-blend.c)
>>   piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-explicit fbo-extended-blend-explicit.c)
>>   piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-pattern fbo-extended-blend-pattern.c)
>> +piglit_add_executable (arb_blend_func_extended-dual-src-blending-discard-without-src1 dual-src-blending-discard-without-src1.c)
>>   # vim: ft=cmake:
>> diff --git a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt
>> index a70e9fa5e..fd41622bd 100644
>> --- a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt
>> +++ b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gles3.txt
>> @@ -3,4 +3,5 @@ link_libraries(piglitutil_${piglit_target_api})
>>   piglit_add_executable (arb_blend_func_extended-fbo-extended-blend_${piglit_target_api} fbo-extended-blend.c)
>>   piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-explicit_${piglit_target_api} fbo-extended-blend-explicit.c)
>>   piglit_add_executable (arb_blend_func_extended-fbo-extended-blend-pattern_${piglit_target_api} fbo-extended-blend-pattern.c)
>> +piglit_add_executable (arb_blend_func_extended-dual-src-blending-discard-without-src1_${piglit_target_api} dual-src-blending-discard-without-src1.c)
>>   # vim: ft=cmake:
>> diff --git a/tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c b/tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c
>> new file mode 100644
>> index 000000000..881a21421
>> --- /dev/null
>> +++ b/tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c
>> @@ -0,0 +1,120 @@
>> +/* Copyright © 2018 Danylo Piliaiev
>> + *
>> + * 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.
>> + */
>> +
>> +/**
>> + * Drawing with dual source blending enabled while fragment shader
>> + * doesn't write into src1 is undefined but it should not hang GPU.
>> + * It hanged Intel gen8+ GPUs with depth test enabled.
>> + *
>> + * https://bugs.freedesktop.org/show_bug.cgi?id=107088
>> + */
>> +#include "piglit-util-gl.h"
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +
>> +#ifdef PIGLIT_USE_OPENGL
>> +    config.supports_gl_core_version = 31;
> Sounds like gl_compat_version=30 would be enough, no?
>
Indeed, it will be enough, thanks.
>> +#else // PIGLIT_USE_OPENGLES3
>> +    config.supports_gl_es_version = 30;
>> +#endif
>> +    config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DEPTH;
>> +    config.khr_no_error_support = PIGLIT_NO_ERRORS;
>> +
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +#ifdef PIGLIT_USE_OPENGL
>> +static const char *vs_text =
>> +    "#version 130\n"
>> +    "in vec4 vertex;\n"
>> +    "void main() { gl_Position = vertex; }\n"
>> +    ;
>> +
>> +static const char *fs_text =
>> +    "#version 130\n"
>> +    "void main() {\n"
>> +    "    discard;\n"
>> +    "}\n"
>> +    ;
>> +#else // PIGLIT_USE_OPENGLES3
>> +static const char *vs_text =
>> +    "#version 300 es\n"
>> +    "in vec4 vertex;\n"
>> +    "void main() { gl_Position = vertex; }\n"
>> +    ;
>> +
>> +static const char *fs_text =
>> +    "#version 300 es\n"
>> +    "void main() {\n"
>> +    "    discard;\n"
>> +    "}\n"
>> +    ;
>> +#endif
>> +
>> +enum piglit_result
>> +piglit_display(void)
>> +{
>> +    return PIGLIT_FAIL;
>> +}
>> +
>> +void piglit_init(int argc, char **argv)
>> +{
>> +#ifdef PIGLIT_USE_OPENGL
>> +    piglit_require_extension("GL_ARB_blend_func_extended");
>> +#else // PIGLIT_USE_OPENGLES3
>> +    piglit_require_extension("GL_EXT_blend_func_extended");
>> +#endif
>> +
>> +    GLint max_dual_source;
>> +    glGetIntegerv(GL_MAX_DUAL_SOURCE_DRAW_BUFFERS, &max_dual_source);
>> +
>> +    if (max_dual_source < 1) {
>> +        fprintf(stderr,
>> +            "ARB_blend_func_extended requires "
>> +            "GL_MAX_DUAL_SOURCE_DRAW_BUFFERS >= 1. "
>> +            "Only got %d!\n",
>> +            max_dual_source);
>> +        piglit_report_result(PIGLIT_FAIL);
>> +    }
>> +
>> +    GLuint prog = piglit_build_simple_program(vs_text, fs_text);
>> +    glUseProgram(prog);
>> +
>> +    glEnable(GL_BLEND);
>> +    glBlendFuncSeparate(GL_ONE, GL_SRC1_COLOR, GL_ONE, GL_ZERO);
>> +
>> +    glEnable(GL_DEPTH_TEST);
> This should be the end of piglit_init. The remainder should go into
> piglit_draw (piglit_display? I forget).
Noted
>> +
>> +    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
> glClearColor/glClearDepth would make this more explicit here.
>
Noted
>> +
>> +    piglit_draw_rect(0, 0, 1, 1);
> I think this draws only a fraction of
Yes, but It doesn't matter for the test, this draw call will not output 
anything
since there is a discard in fragment shader and for the hang to happen
even one fragment is enough.
>> +
>> +    bool pass = piglit_check_gl_error(GL_NO_ERROR);
>> +
>> +    glClearColor(1.0, 0.0, 0.0, 0.0);
>> +    glClear(GL_COLOR_BUFFER_BIT);
> piglit_present_results() here, that way you see the output. Why do you
> draw first and clear second? The other way around makes more sense.
> Also piglit tends to be structured as green = good, red = bad. If the
> issue is that the latter clear never happens, you could clear it to
> red, do your draw, then clear to green.
Later clear isn't expected to succeed in case of the hang.
So I will go with second suggestion: clear red -> draw -> clear green.
Didn't know about color code but it makes sense when you pointed it
>> +
>> +    const float red[] = {1, 0, 0, 0};
>> +    pass = pass && piglit_check_gl_error(GL_NO_ERROR);
>> +    pass = pass && piglit_probe_pixel_rgb(1, 1, red);
>> +
>> +    piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
>> +}
>> --
>> 2.17.1
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/piglit
Quoting Danylo Piliaiev (2018-07-06 07:17:59)
> Thank you for the feedback, I'll send fixed version soon.
> 
> 
> On 06.07.18 16:48, Ilia Mirkin wrote:
> > On Fri, Jul 6, 2018 at 4:51 AM, Danylo Piliaiev
> > <danylo.piliaiev@gmail.com> wrote:
> >> Using fragment shader without second color output should not hang gpu
> >> when dual source blending is enabled.
> >> It hanged Intel gen8+ GPUs when discarding fragments and depth test
> >> being enabled.
> >> There is also safeguard against lack of second color output in radeonsi.
> >>
> >> Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
> >> ---
> >> To not hang hang on Intel gen8+ GPUs this patch depends on
> >> https://patchwork.freedesktop.org/patch/235939/
> >> The hang itself is around 2 - 5 seconds and test produces FAIL.
> >>
> >>   tests/opengl.py                               |   2 +
> >>   .../execution/CMakeLists.gl.txt               |   1 +
> >>   .../execution/CMakeLists.gles3.txt            |   1 +
> >>   .../dual-src-blending-discard-without-src1.c  | 120 ++++++++++++++++++
> >>   4 files changed, 124 insertions(+)
> >>   create mode 100644 tests/spec/arb_blend_func_extended/execution/dual-src-blending-discard-without-src1.c
> >>
> >> diff --git a/tests/opengl.py b/tests/opengl.py
> >> index 02110ff86..61aa197b7 100644
> >> --- a/tests/opengl.py
> >> +++ b/tests/opengl.py
> >> @@ -4114,6 +4114,7 @@ with profile.test_list.group_manager(
> >>       g(['arb_blend_func_extended-fbo-extended-blend'])
> >>       g(['arb_blend_func_extended-fbo-extended-blend-explicit'])
> >>       g(['arb_blend_func_extended-fbo-extended-blend-pattern'])
> >> +    g(['arb_blend_func_extended-dual-src-blending-discard-without-src1'], run_concurrent=False)
> > Why run_concurrent=False?

> I set it because this test may hang which can affect other tests but if 
> it won't an issue I can remove it.

Please remove it, we disable concurrency for tests which will actively interfere
with other tests such as tests for frontbuffer rendering or tests like
arb_timer_query which are sensitive to busy systems only.

Dylan