[PATCH 1/2] compositor: Clear only the the damage that was actually repainted
Ander Conselvan de Oliveira
conselvan2 at gmail.com
Thu Aug 16 01:22:37 PDT 2012
Em 15-08-2012 23:02, Kristian Høgsberg escreveu:
> 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.
Yeah, that looks better. You did, however, remove one too many lines
from surface_accumulate_damage() so the opaque region is always empty.
> 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.
And with some refactoring to track this on a per plane basis, it can
also scale to the case where there are multiple surfaces in an ARGB overlay.
Ander
>
> 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
More information about the wayland-devel
mailing list