On Tue, 10 Jul 2018 12:06:00 +0300 Pekka Paalanen <ppaala...@gmail.com> wrote:
> On Mon, 9 Jul 2018 14:23:15 +0100
> Daniel Stone <dani...@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 <dani...@collabora.com>
> > Tested-by: Emre Ucan <eu...@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.paala...@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 <dan...@fooishbar.org> wrote:
>
> > Hi Pekka,
> >
> > On Fri, 26 Jan 2018 at 12:45, Pekka Paalanen <ppaala...@gmail.com> wrote:
> > > On Wed, 20 Dec 2017 12:26:51 +0000 Daniel Stone <dani...@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;
> > }
>
pgpPCIKT3_4B8.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
