Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator

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

On Fri, Feb 22, 2019 at 05:46:08PM +0000, Kazlauskas, Nicholas wrote:
> On 2/22/19 12:39 PM, Ville Syrjälä wrote:
> > On Fri, Feb 22, 2019 at 12:08:46PM -0500, Nicholas Kazlauskas wrote:
> >> Cairo defaults to using a blended fill, so when IGT tries to paint on
> >> top of a framebuffer in these tests with an alpha less than 1.0 the
> >> previous framebuffer contents will still be present underneath.
> >>
> >> On amdgpu this is an issue because the framebuffer is created in
> >> an unitialized state and can have contents from previous tests
> >> still within memory.
> > 
> > igt_fb more or less assumes all fbs start out black. Sounds like you
> > need a memset() or equivalent somewhere.
> 
> The buffer could also be painted to 0 implicitly using cairo. We also 
> need a patch at some point to not generate the suspicious CRC read 
> warning since our black screens are (0, 0, 0) and we rely on tests 
> leaving garbage in the framebuffer to avoid it.

I guess just 'if (is_i915) check_suspicious_crc()' should help.
Though I'm not 100% sure we can't get those on some i915 platforms too.

> 
> However, this patch would still be needed since the fill will still be 
> blended by default. When creating transparent or semi-transparent 
> buffers they should be filled in with the cairo source operator or the 
> alpha channel won't be set to the correct value.

AFAICS if the dest alpha and color are both 0.0 the only fb that is
wrong is argb_fb_0 since the current code will in fact give us
alpha of 1.0. draw_squares() with OVER should still give us the
same results as we'd get with SOURCE.

I guess this means any test which is currently using argb_fb_0
is definitely not testing per-pixel alpha. 

So argb_transparant() and basic_alpha() are simply useless atm?

constant_alpha_min() sets pixel_blend_mode to None anyway so
doesn't really matter what we have in the alpha channel.

Not sure why those tests aren't doing the gray_fb thing the other tests
are using...

Anyways, no point in blending when you don't have to so
the patch seems to make sense to me either way:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Nicholas Kazlauskas
> 
> > 
> >>
> >> This patch explicitly sets the cairo filling operator to SOURCE to
> >> overwrite the contents when performing fills with alpha < 1.0 in
> >> this test.
> >>
> >> It also fixes the creation of the transparent FB. My guess as for why
> >> this test previously passed on i915 is because the primary FB was
> >> also black, so the CRC would match even if the black overlay plane
> >> was partially transparent.
> >>
> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> >> ---
> >>   tests/kms_plane_alpha_blend.c | 4 +++-
> >>   1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tests/kms_plane_alpha_blend.c b/tests/kms_plane_alpha_blend.c
> >> index 1d9d8933..d7dc44ab 100644
> >> --- a/tests/kms_plane_alpha_blend.c
> >> +++ b/tests/kms_plane_alpha_blend.c
> >> @@ -83,6 +83,7 @@ static void draw_squares(struct igt_fb *fb, int w, int h, double a)
> >>   {
> >>   	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
> >>   
> >> +	cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> >>   	igt_paint_color_alpha(cr, 0, 0,         w / 2, h / 2, 1., 0., 0., a);
> >>   	igt_paint_color_alpha(cr, w / 2, 0,     w / 2, h / 2, 0., 1., 0., a);
> >>   	igt_paint_color_alpha(cr, 0, h / 2,     w / 2, h / 2, 0., 0., 1., a);
> >> @@ -197,7 +198,8 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
> >>   			      &data->argb_fb_0);
> >>   
> >>   		cr = igt_get_cairo_ctx(data->gfx_fd, &data->argb_fb_0);
> >> -		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 1.0);
> >> +		cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> >> +		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 0.0);
> >>   		igt_put_cairo_ctx(data->gfx_fd, &data->argb_fb_0, cr);
> >>   
> >>   		igt_create_fb(data->gfx_fd, w, h,
> >> -- 
> >> 2.17.1
> >>
> >> _______________________________________________
> >> igt-dev mailing list
> >> igt-dev@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > 
> 

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev