[igt-dev] [PATCH i-g-t v2] tests/kms_cursor_crc: refactoring cursor-alpha subtests

Melissa Wen melissa.srw at gmail.com
Fri Oct 23 11:47:25 UTC 2020


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?

> > 
> > 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