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

Pekka Paalanen ppaalanen at gmail.com
Tue Jul 10 10:09:26 UTC 2018


On Tue, 10 Jul 2018 12:06:00 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

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

Sorry, it seems I missed this condition. With it:

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

Even if it does seem to produce less optimal results than my
suggestion, it should be good for now.


Thanks,
pq

> > +
> >  		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/84c08d5b/attachment.sig>


More information about the wayland-devel mailing list