[Piglit] [PATCH] arb_blend_func_extended: Test dual src blending without second color output
Danylo Piliaiev
danylo.piliaiev at gmail.com
Fri Jul 6 14:17:59 UTC 2018
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 at 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 at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/piglit
More information about the Piglit
mailing list