Re: [PATCH v17 09/14] compositor-drm: Ignore occluded views
Hi, On Tue, 10 Jul 2018 at 10:06, Pekka Paalanen wrote: > On Mon, 9 Jul 2018 14:23:15 +0100 Daniel Stone wrote: > > @@ -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; As you noted here, we only promote fully opaque views. > 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. At the moment the whole area must be opaque in order to be promoted to a view. There is another, later, stage where we can begin to promote non-opaque views, however that gets tricky. We need to make sure that DRM's plane blending is in line with what we expect (it might be, but I'm not immediately sure), and we also need to begin to take care of stacking order. At the moment, scanout/primary strictly stacks at the bottom (furthest from eye) and cursor at the top (closest to eye), with all the overlay planes in between. We currently ensure the stacking order is correct by only promoting views which do not overlap each other to overlay planes. If we started loosening that, so planes could overlap each other (opaque or not), we need to be setting the 'zpos' property of planes, or picking the plane in the correct zpos. This got rather difficult rather fast, so I decided it was probably best not to roll it into the series. Fabien and Benji from STM had an old patchset which did implement this, which should be checked and resurrected once this all lands. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v17 09/14] compositor-drm: Ignore occluded views
On Tue, 10 Jul 2018 12:06:00 +0300
Pekka Paalanen wrote:
> On Mon, 9 Jul 2018 14:23:15 +0100
> Daniel Stone 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
> > Tested-by: Emre Ucan
> > ---
> > 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
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 wrote:
>
> > Hi Pekka,
> >
> > On Fri, 26 Jan 2018 at 12:45, Pekka Paalanen wrote:
> > > On Wed, 20 Dec 2017 12:26:51 + Daniel Stone
> > > wrote:
> > > > @@ -3116,9 +3136,16 @@ drm
Re: [PATCH v17 09/14] compositor-drm: Ignore occluded views
On Mon, 9 Jul 2018 14:23:15 +0100
Daniel Stone 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
> Tested-by: Emre Ucan
> ---
> 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 wrote:
> Hi Pekka,
>
> On Fri, 26 Jan 2018 at 12:45, Pekka Paalanen wrote:
> > On Wed, 20 Dec 2017 12:26:51 + Daniel Stone
> > 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);
> > > +
[PATCH v17 09/14] compositor-drm: Ignore occluded views
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
Tested-by: Emre Ucan
---
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)
+ 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;
}
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
