[igt-dev] [PATCH i-g-t v2] tests/kms_cursor_crc: refactoring cursor-alpha subtests
Juha-Pekka Heikkila
juhapekka.heikkila at gmail.com
Sun Oct 25 22:13:56 UTC 2020
On 23.10.2020 14.47, Melissa Wen wrote:
> On 10/22, Juha-Pekka Heikkila wrote:
>> On 22.10.2020 13.37, Ville Syrjälä wrote:
>>> On Wed, Oct 21, 2020 at 05:59:56AM -0300, Melissa Wen wrote:
>>>> Considering just a fully opaque or fully transparent cursor is not enough
>>>> to check the composition of an ARGB cursor plane. For example, the cairo
>>>> ARGB32 format uses pre-multiplied alpha, and this representation may only
>>>> be evident when testing a translucent cursor.
>>>>
>>>> Therefore, this patch refactors the cursor-alpha-opaque and
>>>> cursor-alpha-transparent subtests into just one subtest (cursor-alpha)
>>>> that checks the alpha blending of a white cursor, with different alpha
>>>> values, in the primary plane. This refactoring also generates some setup
>>>> stuffs savings.
>>>
>>> Black background means you can't actually tell the
>>> difference between premultiplied blending and no
>>> blending.
>>>
>>> Oh, I guess restore_image() does end up painting
>>> the test pattern into the primary plane's fb? Are
>>> we guaranteed to position the cursor over some non-black
>>> parts?
>>>
>>> The other obvious concern is whether hardware and software
>>> blending will produce the exact same results or not. I do see
>>> you use 1/4 steps for the alpha, which I guess can make that
>>> more likely.
>>
>> I don't think verifying those blending levels will be possible when
>> comparing hw vs sw image, to me it seemed different gens behave different
>> with cursor alpha. On this test to make it fail reliably even with alpha
>> value 1.0 one just need to change from cursor image white sub block into
>> grey level 0.5.
>>
> Hi, Ville and Juha-Pekka
>
> Thanks for feedback.
>
> To be honest, I'm a little confused by the issues you raised.
> Maybe I'm not getting it right.
>
> (1) We currently have two test cases to check the composition of the
> cursor plane with the primary: alpha = 0 and alpha = 1, both with solid
> black background and white cursor. For these cases, the premultiplied or
> straight alpha blending behave equally, since the cursor is just all 00
> or all FF.
>
> but for any other non-extreme alpha value, if the alpha blending is
> straight, it will end up "applying alpha twice". For example, if I
> remember correctly, cairo draws a white cursor alpha 0.5 = 80808080; and
> doing a *straight* alpha blending on black background results in
> FF404040. And this is why this patch checks the composition for extreme
> and non-extreme alpha values.
> Another way could be just to check a third alpha value, without all
> these alpha levels... I mean, if you think checking non-extreme values
> makes sense.
>
> (2) On the other hand, I guess that the "CRC mismatch for alpha = 0.5"
> for IGTPW_5080 is not really related to the proper composition, but it
> is a rounding issue (225 * 0.5 = 127.5). Because it didn't fail for any
> other non-extreme alpha levels and to get 128 without rounding issue,
> alpha should be 0.502.
>
> Therefore, if the problem here is rounding, would be better to take this
> problematic case off, right?
>
..just some random thoughts..
All cursor tests are running with alpha enabled, most of them just set
it to maximum value. This is because cursor plane did support only pixel
formats with alpha. As for those alphas, this test for example
kms_cursor_crc at pipe-*-cursor-*x*-onscreen is quite fragile across
platforms with alphas even if it explicitly doesn't try to test alpha,
from this I draw those alphas generally are not behaving same in all
platforms.
For those differences on gens with alpha behavior on kernel side you'll
find in intel_display.c icl_set_pipe_chicken(..) where is set display wa
#1153 which was put in place just to make cursor tests happy for icl
back in time. I was checking same bit for tgl but didn't find one hence
I was correcting colors some time ago on this test to 'safe' colors (see
43fa6db in igt git log). Earlier there was in use colors which had 0.5
value failing the test, this sound really familiar to your 0.5 alpha
problem. TBH it is interesting when you run test on tgl with 0.5 alpha
and it _doesn't_ fail because using those 0.5 colors on tgl did fail
this test.
As for your conclusion I think you are spot on this issue is about
rounding but I don't know was there a promise on hw side 0.5 means same
as what Cairo does. It was on test results now failing only on glk for
you? With those terms knowing alpha subtest is as fragile as everything
else on kms_cursor_crc that could be pushed upstream knowing there is
history baggage to drag on. IGT anyway is already committed to idea of
what some alpha/color/.. value mean for cairo will mean same thing for
display hw, there is kms_color struggling with similar problems.
I wonder what does Ville think about that Cairo vs hw issue?
>>>
>>> Side note: data->primary_fb[RESTOREBUFFER] looks to be dead weight
>>
>> Seems it was left when rendercopy was taken out from this test.
>>
>>>
>>>>
>>>> v2:
>>>> - debug log states the alpha whenever CRC mismatch (Arek)
>>>> - checking all alpha values before asserting the result (Arek)
>>>>
>>>> Signed-off-by: Melissa Wen <melissa.srw at gmail.com>
>>>> ---
>>>> tests/kms_cursor_crc.c | 87 ++++++++++++++++++++++--------------------
>>>> 1 file changed, 45 insertions(+), 42 deletions(-)
>>>>
>>>> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
>>>> index 0be8f7f8..83c74a69 100644
>>>> --- a/tests/kms_cursor_crc.c
>>>> +++ b/tests/kms_cursor_crc.c
>>>> @@ -428,65 +428,73 @@ static void prepare_crtc(data_t *data, igt_output_t *output,
>>>> igt_pipe_crc_start(data->pipe_crc);
>>>> }
>>>> -static void test_cursor_alpha(data_t *data, double a)
>>>> +static void test_cursor_alpha(data_t *data)
>>>> {
>>>> igt_display_t *display = &data->display;
>>>> igt_pipe_crc_t *pipe_crc = data->pipe_crc;
>>>> - igt_crc_t crc, ref_crc;
>>>> + igt_crc_t crc[5], ref_crc;
>>>> cairo_t *cr;
>>>> uint32_t fb_id;
>>>> int curw = data->curw;
>>>> int curh = data->curh;
>>>> + int i, index;
>>>> + double alpha;
>>>> + bool mismatch = false;
>>>> - /* Alpha cursor fb with white color */
>>>> + /* Alpha cursor fb */
>>>> fb_id = igt_create_fb(data->drm_fd, curw, curh,
>>>> DRM_FORMAT_ARGB8888,
>>>> LOCAL_DRM_FORMAT_MOD_NONE,
>>>> &data->fb);
>>>> igt_assert(fb_id);
>>>> +
>>>> + /*
>>>> + * Hardware Test
>>>> + * With the cursor enabled, get the PF CRCs from the composition with a
>>>> + * white cursor with different alpha values.
>>>> + */
>>>> cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
>>>> - igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, a);
>>>> - igt_put_cairo_ctx(cr);
>>>> + cairo_set_operator (cr, CAIRO_OPERATOR_SOURCE);
>>>> - /* Hardware Test - enable cursor and get PF CRC */
>>>> cursor_enable(data);
>>>> - igt_display_commit(display);
>>>> - igt_wait_for_vblank(data->drm_fd,
>>>> - display->pipes[data->pipe].crtc_offset);
>>>> - igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &crc);
>>>> + for (i = 0, alpha = 1.0; alpha >= 0.0; alpha -= 0.25, i++) {
>>>> + igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, alpha);
>>>> +
>>>> + igt_display_commit(display);
>>>> + igt_wait_for_vblank(data->drm_fd,
>>>> + display->pipes[data->pipe].crtc_offset);
>>>> + igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &crc[i]);
>>>> + }
>>>> + igt_put_cairo_ctx(cr);
>>>> cursor_disable(data);
>>>> igt_remove_fb(data->drm_fd, &data->fb);
>>>> - /* Software Test - render cursor in software, drawn it directly on PF */
>>>> + /* Software Test - render cursor in software, drawn it directly on PF*/
>>>> cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb[FRONTBUFFER]);
>>>> - igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, a);
>>>> - igt_put_cairo_ctx(cr);
>>>> -
>>>> - igt_display_commit(display);
>>>> - igt_wait_for_vblank(data->drm_fd,
>>>> - display->pipes[data->pipe].crtc_offset);
>>>> - igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &ref_crc);
>>>> - /* Compare CRC from Hardware/Software tests */
>>>> - igt_assert_crc_equal(&crc, &ref_crc);
>>>> + for (i = 0, alpha = 1.0; alpha >= 0.0; alpha -= 0.25, i++) {
>>>> + igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, alpha);
>>>> - /*Clear Screen*/
>>>> - cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb[FRONTBUFFER]);
>>>> - igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
>>>> - 0.0, 0.0, 0.0);
>>>> - igt_put_cairo_ctx(cr);
>>>> -}
>>>> + igt_display_commit(display);
>>>> + igt_wait_for_vblank(data->drm_fd,
>>>> + display->pipes[data->pipe].crtc_offset);
>>>> + igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &ref_crc);
>>>> -static void test_cursor_transparent(data_t *data)
>>>> -{
>>>> - test_cursor_alpha(data, 0.0);
>>>> + /* Compare CRC from Hardware/Software tests */
>>>> + if (igt_find_crc_mismatch(&crc[i], &ref_crc, &index)) {
>>>> + mismatch = true;
>>>> + igt_debug("CRC mismatch for alpha %.2f: 0x%x != 0x%x\n",
>>>> + alpha, crc[i].crc[index], ref_crc.crc[index]);
>>>> + }
>>>> -}
>>>> + /*Clear Screen*/
>>>> + igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
>>>> + 0.0, 0.0, 0.0);
>>>> + }
>>>> + igt_put_cairo_ctx(cr);
>>>> -static void test_cursor_opaque(data_t *data)
>>>> -{
>>>> - test_cursor_alpha(data, 1.0);
>>>> + igt_assert_f(!mismatch, "At least one CRC mismatch detected\n");
>>>> }
>>>> static void run_test(data_t *data, void (*testfunc)(data_t *), int cursor_w, int cursor_h)
>>>> @@ -655,15 +663,10 @@ static void run_tests_on_pipe(data_t *data, enum pipe pipe)
>>>> run_test(data, test_cursor_size,
>>>> data->cursor_max_w, data->cursor_max_h);
>>>> - igt_describe("Validates the composition of a fully opaque cursor "
>>>> - "plane, i.e., alpha channel equal to 1.0.");
>>>> - igt_subtest_f("pipe-%s-cursor-alpha-opaque", kmstest_pipe_name(pipe))
>>>> - run_test(data, test_cursor_opaque, data->cursor_max_w, data->cursor_max_h);
>>>> -
>>>> - igt_describe("Validates the composition of a fully transparent cursor "
>>>> - "plane, i.e., alpha channel equal to 0.0.");
>>>> - igt_subtest_f("pipe-%s-cursor-alpha-transparent", kmstest_pipe_name(pipe))
>>>> - run_test(data, test_cursor_transparent, data->cursor_max_w, data->cursor_max_h);
>>>> + igt_describe("Validates the composition of alpha cursor plane "
>>>> + "from fully opaque to fully transparent alpha range");
>>>> + igt_subtest_f("pipe-%s-cursor-alpha", kmstest_pipe_name(pipe))
>>>> + run_test(data, test_cursor_alpha, data->cursor_max_w, data->cursor_max_h);
>>>> igt_fixture
>>>> create_cursor_fb(data, data->cursor_max_w, data->cursor_max_h);
>>>> --
>>>> 2.28.0
>>>>
>>>> _______________________________________________
>>>> igt-dev mailing list
>>>> igt-dev at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>>>
>>
More information about the igt-dev
mailing list