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

On 14.10.2021 11.42, Ville Syrjälä wrote:

On Tue, Oct 12, 2021 at 05:22:24PM +0300, Juha-Pekka Heikkila wrote:

quoted

Unify and simplify cursor size change and cursor alpha tests.

Signed-off-by: Juha-Pekka Heikkila <redacted>
---
  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.

There's been in use CAIRO_OPERATOR_SOURCE because here was always used 
only alpha 1.0 hence at some point I had dropped that alpha parameter. 
Now I used this function to draw with alpha in use so I put in place 
CAIRO_OPERATOR_OVER and set default alpha in 'data' to 1.0, though now 
that think about it I'll need to set it back to 1.0 after 
'alpha-transparent' subtest.

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?

Agree diff is quite big. That code moving is just because 
test_cursor_alpha started to use create_cursor_fb so create_cursor_fb 
need to be after test_cursor_alpha, I can separate that into its own 
patch but it probably will not change much in diff which does actual 
change. Most of diff is just about removing unnecessary code once 
started to use do_single_test also for size change and alpha tests.

I'll have second go to make the patch(es) to see what I get.

/Juha-Pekka