[Cogl] [PATCH 2/2] wayland: Don't delay resize if nothing is drawn since last swap

Robert Bragg robert at sixbynine.org
Tue May 28 05:53:02 PDT 2013


Since we now have the public api using the term "dirty" to describe
undefined buffer contents, using the term dirty here could be a bit
confusing down the line, since in this context dirty has an almost
opposite meaning. Dirty here means something has been drawn and from
the user's pov the buffer contents is now well defined.

Maybe we can use the term "damage" internally, or track a "mid_scene"
or "frame_started" flag or something along those lines?

+  /* Whether something has been drawn to the buffer since the last
+   * swap buffers. Note that cogl_onscreen_swap_region doesn't clear
+   * this flag */
+  CoglBool            dirty_since_swap;

I wasn't sure why this special cases the use of _swap_region()? It
seems like it would be generally useful for us to know when we are in
the middle of rendering a frame regardless of whether swap_region()
gets used. _swap_region() is just a different way of presenting at the
end of a frame by copying a specific region to the front-buffer, but
apart from that it is considered to be the end of a frame.

kind regards,
- Robert

On Fri, May 17, 2013 at 3:57 PM, Neil Roberts <neil at linux.intel.com> wrote:
> After discussing with Kristian Høgsberg it seems that the semantics of
> wl_egl_window_resize is meant to be that if nothing has been drawn to
> the framebuffer since the last swap then the resize will take effect
> immediately. Cogl was previously always delaying the call to
> wl_egl_window_resize until the next swap. That meant that if you
> wanted to resize the surface you would have to call
> cogl_wayland_onscreen_resize and then redundantly draw a frame at the
> old size so that you can swap to get the resize to occur before
> drawing again at the right size. Typically an application would decide
> to resize at the start of its paint sequence so it should be able to
> just resize immediately.
>
> It current Mesa master it seems that there is a bug which means that
> it won't actually delay a resize that is done mid-scene and instead it
> will just discard what came before. To get consistent behaviour in
> Cogl, the code to delay the call to wl_egl_window_resize is still used
> if it determines that the buffer is dirty. There is an existing
> _cogl_framebuffer_dirty call which was being used to track when the
> framebuffer becomes dirty since the last clear. This function is now
> also used to track a new flag to track whether it's dirty since the
> last swap.
>
> cogl_framebuffer_clear has been slightly altered to always call
> _cogl_framebuffer_dirty even if it determines that it doesn't need to
> clear because the framebuffer should still be considered dirty from
> the point of view of cogl_wayland_onscreen_resize.
>
> This also fixes a potential bug where it looks like pending_dx/dy were
> never cleared so they would always be accumulated even after the
> resize is flushed.
> ---
>  cogl/cogl-framebuffer-private.h       |  5 ++++
>  cogl/cogl-framebuffer.c               |  5 ++--
>  cogl/cogl-onscreen.c                  |  1 +
>  cogl/cogl-onscreen.h                  | 21 ++++++++++------
>  cogl/winsys/cogl-winsys-egl-wayland.c | 46 ++++++++++++++++++++++++++++-------
>  5 files changed, 59 insertions(+), 19 deletions(-)
>
> diff --git a/cogl/cogl-framebuffer-private.h b/cogl/cogl-framebuffer-private.h
> index cd2739a..19bb0d8 100644
> --- a/cogl/cogl-framebuffer-private.h
> +++ b/cogl/cogl-framebuffer-private.h
> @@ -170,6 +170,11 @@ struct _CoglFramebuffer
>    int                 clear_clip_y1;
>    CoglBool            clear_clip_dirty;
>
> +  /* Whether something has been drawn to the buffer since the last
> +   * swap buffers. Note that cogl_onscreen_swap_region doesn't clear
> +   * this flag */
> +  CoglBool            dirty_since_swap;
> +
>    /* driver specific */
>    CoglBool            dirty_bitmasks;
>    CoglFramebufferBits bits;
> diff --git a/cogl/cogl-framebuffer.c b/cogl/cogl-framebuffer.c
> index 3ac0e03..ed43adc 100644
> --- a/cogl/cogl-framebuffer.c
> +++ b/cogl/cogl-framebuffer.c
> @@ -226,6 +226,7 @@ void
>  _cogl_framebuffer_dirty (CoglFramebuffer *framebuffer)
>  {
>    framebuffer->clear_clip_dirty = TRUE;
> +  framebuffer->dirty_since_swap = TRUE;
>  }
>
>  void
> @@ -351,6 +352,8 @@ cogl_framebuffer_clear4f (CoglFramebuffer *framebuffer,
>
>  cleared:
>
> +  _cogl_framebuffer_dirty (framebuffer);
> +
>    if (buffers & COGL_BUFFER_BIT_COLOR && buffers & COGL_BUFFER_BIT_DEPTH)
>      {
>        /* For our fast-path for reading back a single pixel of simple
> @@ -378,8 +381,6 @@ cleared:
>            /* FIXME: set degenerate clip */
>          }
>      }
> -  else
> -    _cogl_framebuffer_dirty (framebuffer);
>  }
>
>  /* Note: the 'buffers' and 'color' arguments were switched around on
> diff --git a/cogl/cogl-onscreen.c b/cogl/cogl-onscreen.c
> index d19748f..9f39f59 100644
> --- a/cogl/cogl-onscreen.c
> +++ b/cogl/cogl-onscreen.c
> @@ -271,6 +271,7 @@ cogl_onscreen_swap_buffers_with_damage (CoglOnscreen *onscreen,
>      }
>
>    onscreen->frame_counter++;
> +  framebuffer->dirty_since_swap = FALSE;
>  }
>
>  void
> diff --git a/cogl/cogl-onscreen.h b/cogl/cogl-onscreen.h
> index 455d450..021486e 100644
> --- a/cogl/cogl-onscreen.h
> +++ b/cogl/cogl-onscreen.h
> @@ -212,12 +212,12 @@ cogl_wayland_onscreen_set_foreign_surface (CoglOnscreen *onscreen,
>   * @width: The desired width of the framebuffer
>   * @height: The desired height of the framebuffer
>   * @offset_x: A relative x offset for the new framebuffer
> - * @offset_y: A relative x offset for the new framebuffer
> + * @offset_y: A relative y offset for the new framebuffer
>   *
> - * Queues a resize of the given @onscreen framebuffer which will be applied
> - * during the next swap buffers request. Since a buffer is usually conceptually
> - * scaled with a center point the @offset_x and @offset_y arguments allow the
> - * newly allocated buffer to be positioned relative to the old buffer size.
> + * Resizes the backbuffer of the given @onscreen framebuffer to the
> + * given size. Since a buffer is usually conceptually scaled with a
> + * center point the @offset_x and @offset_y arguments allow the newly
> + * allocated buffer to be positioned relative to the old buffer size.
>   *
>   * For example a buffer that is being resized by moving the bottom right
>   * corner, and the top left corner is remaining static would use x and y
> @@ -228,9 +228,14 @@ cogl_wayland_onscreen_set_foreign_surface (CoglOnscreen *onscreen,
>   * x/y_size_increase is how many pixels bigger the buffer is on the x and y
>   * axis.
>   *
> - * If cogl_wayland_onscreen_resize() is called multiple times before the next
> - * swap buffers request then the relative x and y offsets accumulate instead of
> - * being replaced. The @width and @height values superseed the old values.
> + * Note that if some drawing commands have been applied to the
> + * framebuffer since the last swap buffers then the resize will be
> + * queued and will only take effect in the next swap buffers.
> + *
> + * If multiple calls to cogl_wayland_onscreen_resize() get queued
> + * before the next swap buffers request then the relative x and y
> + * offsets accumulate instead of being replaced. The @width and
> + * @height values superseed the old values.
>   *
>   * Since: 1.10
>   * Stability: unstable
> diff --git a/cogl/winsys/cogl-winsys-egl-wayland.c b/cogl/winsys/cogl-winsys-egl-wayland.c
> index 334ec1b..196752c 100644
> --- a/cogl/winsys/cogl-winsys-egl-wayland.c
> +++ b/cogl/winsys/cogl-winsys-egl-wayland.c
> @@ -416,15 +416,8 @@ _cogl_winsys_egl_onscreen_deinit (CoglOnscreen *onscreen)
>  }
>
>  static void
> -_cogl_winsys_onscreen_swap_buffers_with_damage (CoglOnscreen *onscreen,
> -                                                const int *rectangles,
> -                                                int n_rectangles)
> +flush_pending_resize (CoglOnscreen *onscreen)
>  {
> -  CoglFramebuffer *fb = COGL_FRAMEBUFFER (onscreen);
> -  CoglContext *context = fb->context;
> -  CoglRenderer *renderer = context->display->renderer;
> -  CoglRendererEGL *egl_renderer = renderer->winsys;
> -  CoglRendererWayland *wayland_renderer = egl_renderer->platform;
>    CoglOnscreenEGL *egl_onscreen = onscreen->winsys;
>    CoglOnscreenWayland *wayland_onscreen = egl_onscreen->platform;
>
> @@ -436,11 +429,28 @@ _cogl_winsys_onscreen_swap_buffers_with_damage (CoglOnscreen *onscreen,
>                              wayland_onscreen->pending_dx,
>                              wayland_onscreen->pending_dy);
>
> -      _cogl_framebuffer_winsys_update_size (fb,
> +      _cogl_framebuffer_winsys_update_size (COGL_FRAMEBUFFER (onscreen),
>                                              wayland_onscreen->pending_width,
>                                              wayland_onscreen->pending_height);
> +
> +      wayland_onscreen->pending_dx = 0;
> +      wayland_onscreen->pending_dy = 0;
>        wayland_onscreen->has_pending = FALSE;
>      }
> +}
> +
> +static void
> +_cogl_winsys_onscreen_swap_buffers_with_damage (CoglOnscreen *onscreen,
> +                                                const int *rectangles,
> +                                                int n_rectangles)
> +{
> +  CoglFramebuffer *fb = COGL_FRAMEBUFFER (onscreen);
> +  CoglContext *context = fb->context;
> +  CoglRenderer *renderer = context->display->renderer;
> +  CoglRendererEGL *egl_renderer = renderer->winsys;
> +  CoglRendererWayland *wayland_renderer = egl_renderer->platform;
> +
> +  flush_pending_resize (onscreen);
>
>    parent_vtable->onscreen_swap_buffers_with_damage (onscreen,
>                                                      rectangles,
> @@ -627,6 +637,15 @@ cogl_wayland_onscreen_resize (CoglOnscreen *onscreen,
>        CoglOnscreenEGL *egl_onscreen = onscreen->winsys;
>        CoglOnscreenWayland *wayland_onscreen = egl_onscreen->platform;
>
> +      /* We want to update the dirty_since_swap flag depending on
> +       * whether anything has drawn to the buffer. For things in the
> +       * journal this won't get updated until the journal is actually
> +       * flushed. The journal isn't exposed outside of the Cogl API so
> +       * drawing via the journal should be considered dirtying the
> +       * framebuffer the same as anything else. To make that work
> +       * we'll just flush the journal here. */
> +      _cogl_framebuffer_flush_journal (fb);
> +
>        if (cogl_framebuffer_get_width (fb) != width ||
>            cogl_framebuffer_get_height (fb) != height ||
>            offset_x ||
> @@ -637,6 +656,15 @@ cogl_wayland_onscreen_resize (CoglOnscreen *onscreen,
>            wayland_onscreen->pending_dx += offset_x;
>            wayland_onscreen->pending_dy += offset_y;
>            wayland_onscreen->has_pending = TRUE;
> +
> +          /* If nothing has been drawn to the framebuffer since the
> +           * last swap then wl_egl_window_resize will take effect
> +           * immediately. Otherwise it might not take effect until the
> +           * next swap, depending on the version of Mesa. To keep
> +           * consistent behaviour we'll delay the resize until the
> +           * next swap unless we're sure nothing has been drawn */
> +          if (!fb->dirty_since_swap)
> +            flush_pending_resize (onscreen);
>          }
>      }
>    else
> --
> 1.7.11.3.g3c3efa5
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl


More information about the Cogl mailing list