Re: [igt-dev] [PATCH i-g-t] tests/kms_cursor_crc: fix size change and alpha tests

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Oct 12, 2021 at 05:22:24PM +0300, Juha-Pekka Heikkila wrote:
> Unify and simplify cursor size change and cursor alpha tests.
> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
>  tests/kms_cursor_crc.c | 190 ++++++++++++-----------------------------
>  1 file changed, 56 insertions(+), 134 deletions(-)
> 
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index 9faba1a18..7d9ec1927 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -71,7 +71,7 @@ typedef struct {
>  	igt_plane_t *cursor;
>  	cairo_surface_t *surface;
>  	uint32_t devid;
> -	bool hwimageistestimage;
> +	double alpha;
>  } data_t;
>  
>  #define TEST_DPMS (1<<0)
> @@ -89,7 +89,7 @@ typedef struct {
>  	int height;
>  } cursorarea;
>  
> -static void draw_cursor(cairo_t *cr, cursorarea *cursor)
> +static void draw_cursor(cairo_t *cr, cursorarea *cursor, double alpha)
>  {
>  	int wl, wr, ht, hb;
>  
> @@ -104,13 +104,13 @@ static void draw_cursor(cairo_t *cr, cursorarea *cursor)
>  	    (cursor->y < SHRT_MIN) || (cursor->y > SHRT_MAX))
>  		return;
>  
> -	cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> +	cairo_set_operator(cr, CAIRO_OPERATOR_OVER);
>  	cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE);
>  	/* 4 color rectangles in the corners, RGBY */
> -	igt_paint_color(cr, cursor->x, cursor->y, wl, ht, RED);
> -	igt_paint_color(cr, cursor->x + wl, cursor->y, wr, ht, GREEN);
> -	igt_paint_color(cr, cursor->x, cursor->y + ht, wl, hb, BLUE);
> -	igt_paint_color(cr, cursor->x + wl, cursor->y + ht, wr, hb, WHITE);
> +	igt_paint_color_alpha(cr, cursor->x, cursor->y, wl, ht, RED, alpha);
> +	igt_paint_color_alpha(cr, cursor->x + wl, cursor->y, wr, ht, GREEN, alpha);
> +	igt_paint_color_alpha(cr, cursor->x, cursor->y + ht, wl, hb, BLUE, alpha);
> +	igt_paint_color_alpha(cr, cursor->x + wl, cursor->y + ht, wr, hb, WHITE, alpha);

Is this an actual bugfix? The alpha seems to have disappeared at some
point.

Hmm. test_cursor_alpha() is apparently creating its own fb currently,
and clobbering data->fb which was created by the top level fixture for
everyone else. Ugh.

Anyways, the diff is huge, can't really figure out what the heck
it's doing. There seems to be some code motion in there as well,
but maybe with other changes sprinkled in? Dunno. Can you split
it up a bit?

-- 
Ville Syrjälä
Intel