[PATCH 1/2] compositor: Clear only the the damage that was actually repainted
Kristian Høgsberg
hoegsberg at gmail.com
Wed Aug 15 13:02:11 PDT 2012
On Tue, Aug 14, 2012 at 03:26:49PM +0300, Ander Conselvan de Oliveira wrote:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
>
> Instead of clearing the whole output region after a repaint, clear
> only the regions that were actually painted. This way, the damage
> added when a surface moves from the primary plane to another one is
> kept while this region is obscured by the opaque region. This allows
> the contents below an overlaid surface to be culled, but to make this
> work properly, it is also necessary to change the way previous damage
> is drawn.
>
> Consider the following scenario: a surface is moved to an overlay plane
> leaving some damage in the primary plane. On the following frame, the
> surface on the overlay moves, revealing part of the damaged region on
> the primary plane. On the frame after that, the overlaid surface moves
> back to its previous position obscuring the region of the primary plane
> repainted before. At this point, the repainted region was added to the
> output's previous damage so that it is draw to both buffers. But since
> this region is now obscured, the redrawing is skipped. If the overlaid
> surface moves again revealing this region, one of the buffers actually
> contains the wrong content.
>
> To fix this problem, this patch ensures that any previous damage that
> would be lost is actually drawn, by subtracting it from the opaque
> region.
Ah, hm, ok... that's really subtle. Good analysis. Now with your
fix, we still repaint when we move the opaque overlay back over a
region with "old damage" even though it's not visible. How about
unioning output->previous_damage into the primary plane damage just
before repainting? That way, if any old damage doesn't get repainted,
we add it back to the primary plane damage. The only drawback here is
that we move the overlay away again, we could end up repainting that
area again. But once all buffers has repainted the area, the residual
damage will disappear.
I've tweaked your patch to do that and attached it. I think it's a
slight improvement, in that we probably typically end up painting less
and it's a little simpler. However, either approach is a workaround
how we should track the damage per buffer. When we repaint an output,
we add the new damage (from primary_plane.damage) to all buffers
damage, then render the damage (clipped by opaque regions) in current
back buffers damage region, subtracting what we render. This also
scales to triple buffering.
Kristian
> ---
> src/compositor.c | 26 ++++++++++++++++++--------
> 1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/src/compositor.c b/src/compositor.c
> index 7370435..26c57b9 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -928,6 +928,9 @@ weston_surface_draw(struct weston_surface *es, struct weston_output *output,
> if (!pixman_region32_not_empty(&repaint))
> goto out;
>
> + pixman_region32_subtract(&ec->primary_plane.damage,
> + &ec->primary_plane.damage, &repaint);
> +
> glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA);
> if (es->blend || es->alpha < 1.0)
> glEnable(GL_BLEND);
> @@ -1095,7 +1098,8 @@ update_shm_texture(struct weston_surface *surface)
>
> static void
> surface_accumulate_damage(struct weston_surface *surface,
> - pixman_region32_t *opaque)
> + pixman_region32_t *opaque,
> + pixman_region32_t *exclude)
> {
> if (surface->buffer && wl_buffer_is_shm(surface->buffer))
> update_shm_texture(surface);
> @@ -1121,9 +1125,8 @@ surface_accumulate_damage(struct weston_surface *surface,
> pixman_region32_union(&surface->plane->damage,
> &surface->plane->damage, &surface->damage);
> empty_region(&surface->damage);
> - pixman_region32_copy(&surface->clip, opaque);
> - if (surface->plane == &surface->compositor->primary_plane)
> - pixman_region32_union(opaque, opaque, &surface->transform.opaque);
> + pixman_region32_subtract(&surface->clip, opaque, exclude);
> + pixman_region32_union(opaque, opaque, &surface->transform.opaque);
> }
>
> static void
> @@ -1135,7 +1138,7 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
> struct weston_animation *animation, *next;
> struct weston_frame_callback *cb, *cnext;
> struct wl_list frame_callback_list;
> - pixman_region32_t opaque, output_damage;
> + pixman_region32_t opaque, output_damage, previous;
> int32_t width, height;
>
> weston_compositor_update_drag_surfaces(ec);
> @@ -1170,8 +1173,16 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
>
> pixman_region32_init(&opaque);
>
> + /* Once we draw something on one of the back buffers it needs to be
> + * drawn on the other one, otherwise we might endup with artifacts.
> + * Make sure we draw any previous damage that would be lost otherwise
> + */
> + pixman_region32_init(&previous);
> + pixman_region32_subtract(&previous, &output->previous_damage,
> + &ec->primary_plane.damage);
> +
> wl_list_for_each(es, &ec->surface_list, link)
> - surface_accumulate_damage(es, &opaque);
> + surface_accumulate_damage(es, &opaque, &previous);
>
> pixman_region32_init(&output_damage);
> pixman_region32_union(&output_damage, &ec->primary_plane.damage,
> @@ -1180,10 +1191,9 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
> &ec->primary_plane.damage);
> pixman_region32_intersect(&output_damage,
> &output_damage, &output->region);
> - pixman_region32_subtract(&ec->primary_plane.damage,
> - &ec->primary_plane.damage, &output->region);
>
> pixman_region32_fini(&opaque);
> + pixman_region32_fini(&previous);
>
> if (output->dirty)
> weston_output_update_matrix(output);
> --
> 1.7.9.5
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
-------------- next part --------------
>From 7803a3d2ea4ce6551355327edc8f0431f63663f3 Mon Sep 17 00:00:00 2001
From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
Date: Wed, 15 Aug 2012 14:02:05 -0400
Subject: [PATCH] compositor: Clear only the the damage that was actually
repainted
Instead of clearing the whole output region after a repaint, clear
only the regions that were actually painted. This way, the damage
added when a surface moves from the primary plane to another one is
kept while this region is obscured by the opaque region. This allows
the contents below an overlaid surface to be culled, but to make this
work properly, it is also necessary to change the way previous damage
is drawn.
Consider the following scenario: a surface is moved to an overlay plane
leaving some damage in the primary plane. On the following frame, the
surface on the overlay moves, revealing part of the damaged region on
the primary plane. On the frame after that, the overlaid surface moves
back to its previous position obscuring the region of the primary plane
repainted before. At this point, the repainted region was added to the
output's previous damage so that it is draw to both buffers. But since
this region is now obscured, the redrawing is skipped. If the overlaid
surface moves again revealing this region, one of the buffers actually
contains the wrong content.
To fix this problem, this patch ensures that any previous damage that
would be lost is actually preserved by folding it back into the
primary plane damage just before repainting.
---
src/compositor.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/src/compositor.c b/src/compositor.c
index 2efcacc..be07394 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -930,6 +930,9 @@ weston_surface_draw(struct weston_surface *es, struct weston_output *output,
if (!pixman_region32_not_empty(&repaint))
goto out;
+ pixman_region32_subtract(&ec->primary_plane.damage,
+ &ec->primary_plane.damage, &repaint);
+
glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA);
if (es->blend || es->alpha < 1.0)
glEnable(GL_BLEND);
@@ -1124,8 +1127,6 @@ surface_accumulate_damage(struct weston_surface *surface,
&surface->plane->damage, &surface->damage);
empty_region(&surface->damage);
pixman_region32_copy(&surface->clip, opaque);
- if (surface->plane == &surface->compositor->primary_plane)
- pixman_region32_union(opaque, opaque, &surface->transform.opaque);
}
static void
@@ -1137,7 +1138,7 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
struct weston_animation *animation, *next;
struct weston_frame_callback *cb, *cnext;
struct wl_list frame_callback_list;
- pixman_region32_t opaque, output_damage;
+ pixman_region32_t opaque, output_damage, new_damage;
int32_t width, height;
weston_compositor_update_drag_surfaces(ec);
@@ -1169,23 +1170,29 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
wl_list_for_each(es, &ec->surface_list, link)
weston_surface_move_to_plane(es, &ec->primary_plane);
-
pixman_region32_init(&opaque);
wl_list_for_each(es, &ec->surface_list, link)
surface_accumulate_damage(es, &opaque);
+ pixman_region32_fini(&opaque);
+
pixman_region32_init(&output_damage);
- pixman_region32_union(&output_damage, &ec->primary_plane.damage,
+
+ pixman_region32_init(&new_damage);
+ pixman_region32_copy(&new_damage, &ec->primary_plane.damage);
+
+ pixman_region32_union(&ec->primary_plane.damage,
+ &ec->primary_plane.damage,
&output->previous_damage);
- pixman_region32_copy(&output->previous_damage,
- &ec->primary_plane.damage);
+
+ pixman_region32_intersect(&output->previous_damage,
+ &new_damage, &output->region);
+
pixman_region32_intersect(&output_damage,
- &output_damage, &output->region);
- pixman_region32_subtract(&ec->primary_plane.damage,
- &ec->primary_plane.damage, &output->region);
+ &ec->primary_plane.damage, &output->region);
- pixman_region32_fini(&opaque);
+ pixman_region32_fini(&new_damage);
if (output->dirty)
weston_output_update_matrix(output);
--
1.7.10.2
More information about the wayland-devel
mailing list