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.pilia...@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.pilia...@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
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
