[PATCH v17 09/14] compositor-drm: Ignore occluded views

[Pekka Paalanen]

On Mon,  9 Jul 2018 14:23:15 +0100
Daniel Stone <daniels at collabora.com> wrote:

> When trying to assign planes, keep track of the areas which are
> already occluded, and ignore views which are completely occluded. This
> allows us to build a state using planes only, when there are occluded
> views which cannot go into a plane behind views which can.
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> Tested-by: Emre Ucan <eucan at de.adit-jv.com>
> ---
>  libweston/compositor-drm.c | 37 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index ca885f96e..b4a65209a 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -3252,7 +3252,7 @@ drm_output_propose_state(struct weston_output *output_base,
>  	struct drm_output *output = to_drm_output(output_base);
>  	struct drm_output_state *state;
>  	struct weston_view *ev;
> -	pixman_region32_t surface_overlap, renderer_region;
> +	pixman_region32_t surface_overlap, renderer_region, occluded_region;
>  	struct weston_plane *primary = &output_base->compositor->primary_plane;
>  
>  	assert(!output->state_last);
> @@ -3274,9 +3274,12 @@ drm_output_propose_state(struct weston_output *output_base,
>  	 * as we do for flipping full screen surfaces.
>  	 */
>  	pixman_region32_init(&renderer_region);
> +	pixman_region32_init(&occluded_region);
>  
>  	wl_list_for_each(ev, &output_base->compositor->view_list, link) {
>  		struct weston_plane *next_plane = NULL;
> +		pixman_region32_t clipped_view;
> +		bool occluded = false;
>  
>  		/* If this view doesn't touch our output at all, there's no
>  		 * reason to do anything with it. */
> @@ -3288,13 +3291,27 @@ drm_output_propose_state(struct weston_output *output_base,
>  		if (ev->output_mask != (1u << output->base.id))
>  			next_plane = primary;
>  
> +		/* Ignore views we know to be totally occluded. */
> +		pixman_region32_init(&clipped_view);
> +		pixman_region32_intersect(&clipped_view,
> +					  &ev->transform.boundingbox,
> +					  &output->base.region);
> +
> +		pixman_region32_init(&surface_overlap);
> +		pixman_region32_subtract(&surface_overlap, &clipped_view,
> +					 &occluded_region);
> +		occluded = !pixman_region32_not_empty(&surface_overlap);
> +		if (occluded) {
> +			pixman_region32_fini(&surface_overlap);
> +			pixman_region32_fini(&clipped_view);
> +			continue;
> +		}
> +
>  		/* Since we process views from top to bottom, we know that if
>  		 * the view intersects the calculated renderer region, it must
>  		 * be part of, or occluded by, it, and cannot go on a plane. */
> -		pixman_region32_init(&surface_overlap);
>  		pixman_region32_intersect(&surface_overlap, &renderer_region,
> -					  &ev->transform.boundingbox);
> -
> +					  &clipped_view);
>  		if (pixman_region32_not_empty(&surface_overlap))
>  			next_plane = primary;
>  		pixman_region32_fini(&surface_overlap);
> @@ -3302,6 +3319,9 @@ drm_output_propose_state(struct weston_output *output_base,
>  		if (next_plane == NULL)
>  			next_plane = drm_output_prepare_cursor_view(state, ev);
>  
> +		if (next_plane == NULL && !drm_view_is_opaque(ev))
> +			next_plane = primary;
> +
>  		if (next_plane == NULL)
>  			next_plane = drm_output_prepare_scanout_view(state, ev);
>  
> @@ -3314,9 +3334,16 @@ drm_output_propose_state(struct weston_output *output_base,
>  		if (next_plane == primary)
>  			pixman_region32_union(&renderer_region,
>  					      &renderer_region,
> -					      &ev->transform.boundingbox);
> +					      &clipped_view);
> +		else if (output->cursor_plane &&
> +			 next_plane != &output->cursor_plane->base)

On Mon, 9 Jul 2018 15:46:40 +0100
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi Pekka,
> 
> On Fri, 26 Jan 2018 at 12:45, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > On Wed, 20 Dec 2017 12:26:51 +0000 Daniel Stone <daniels at collabora.com> wrote:  
> > > @@ -3116,9 +3136,16 @@ drm_output_propose_state(struct weston_output *output_base,
> > >               if (next_plane == primary)
> > >                       pixman_region32_union(&renderer_region,
> > >                                             &renderer_region,
> > > -                                           &ev->transform.boundingbox);
> > > +                                           &clipped_view);
> > > +             else if (output->cursor_plane &&
> > > +                      next_plane != &output->cursor_plane->base)  
> >
> > Should this not be:
> >
> >         if (!output->cursor_plane || next_plane != &output->cursor_plane->base)
> >
> > so that lack of a cursor plane goes straight to occluded?  
> 
> Very right. I've force-pushed this fix

Yeah, that fix is good for its intention.

However, I noticed something else. drm_output_preprare_overlay_view()
may work also for non-opaque views. Therefore we need to handle
overlays as non-opaque or know if they actually are opaque.

Why do we even toggle opaqueness by the plane assignment? Why not
simply directly add the opaque region to occluded_region regardless of
the assigned plane?

weston_view::transform.opaque is there for that purpose.

drm_view_is_opaque() requires the whole view to be opaque, but we don't
actually need that for occluded_region. We should be able to do with
the proper opaque region in global coordinates, so that an opaque
sub-region of a view on a plane would be counted as occlusion.


Thanks,
pq


> +			pixman_region32_union(&occluded_region,
> +					      &occluded_region,
> +					      &clipped_view);
> +		pixman_region32_fini(&clipped_view);
>  	}
>  	pixman_region32_fini(&renderer_region);
> +	pixman_region32_fini(&occluded_region);
>  
>  	return state;
>  }

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180710/6681b9f4/attachment.sig>