[Cogl] [PATCH 2/3] Fix flushing the stencil viewport clipping workaround

[Robert Bragg]

This looks good to me:

Reviewed-by: Robert Bragg <robert at linux.intel.com>

thanks,
- Robert

On Tue, Nov 20, 2012 at 6:53 PM, Neil Roberts <neil at linux.intel.com> wrote:

> There were two problems with the stencil viewport clip workaround
> introduced in afc5daab8:
>
> • When the viewport is changed the current clip state is not marked as
>   dirty. That means that when the framebuffer state is next flushed it
>   would continue to use the stencil from the previous viewport.
>
> • When the viewport is automatically updated due to the window being
>   resized the viewport age was not incremented so the clip state
>   wouldn't be flushed.
>
> I noticed the bugs by running cogl-sdl2-hello.
>
> This patch makes it so that the clip state is dirtied in
> cogl_framebuffer_set_viewport if the workaround is enabled.
>
> The automatic viewport changing code now just calls
> cogl_framebuffer_set_viewport instead of directly prodding the
> viewport values. This has the side-effect that it will also cause the
> journal to be flushed. This seems like the right thing to do anyway
> and presumably there would have been a bug before where it wouldn't
> have flushed the journal, although presumably this is extremely
> unlikely because it would have to have done a resize in the middle of
> painting the scene.
> ---
>  cogl/cogl-framebuffer.c | 12 +++++++++---
>  cogl/cogl-onscreen.c    | 12 +-----------
>  2 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/cogl/cogl-framebuffer.c b/cogl/cogl-framebuffer.c
> index 49d4364..54b7797 100644
> --- a/cogl/cogl-framebuffer.c
> +++ b/cogl/cogl-framebuffer.c
> @@ -448,6 +448,8 @@ cogl_framebuffer_set_viewport (CoglFramebuffer
> *framebuffer,
>                                 float width,
>                                 float height)
>  {
> +  CoglContext *context = framebuffer->context;
> +
>    _COGL_RETURN_IF_FAIL (width > 0 && height > 0);
>
>    if (framebuffer->viewport_x == x &&
> @@ -464,9 +466,13 @@ cogl_framebuffer_set_viewport (CoglFramebuffer
> *framebuffer,
>    framebuffer->viewport_height = height;
>    framebuffer->viewport_age++;
>
> -  if (framebuffer->context->current_draw_buffer == framebuffer)
> -    framebuffer->context->current_draw_buffer_changes |=
> -      COGL_FRAMEBUFFER_STATE_VIEWPORT;
> +  if (context->current_draw_buffer == framebuffer)
> +    {
> +      context->current_draw_buffer_changes |=
> COGL_FRAMEBUFFER_STATE_VIEWPORT;
> +
> +      if (context->needs_viewport_scissor_workaround)
> +        context->current_draw_buffer_changes |=
> COGL_FRAMEBUFFER_STATE_CLIP;
> +    }
>  }
>
>  float
> diff --git a/cogl/cogl-onscreen.c b/cogl/cogl-onscreen.c
> index 4aaa5de..3c1e148 100644
> --- a/cogl/cogl-onscreen.c
> +++ b/cogl/cogl-onscreen.c
> @@ -387,17 +387,7 @@ _cogl_framebuffer_winsys_update_size (CoglFramebuffer
> *framebuffer,
>    framebuffer->width = width;
>    framebuffer->height = height;
>
> -  framebuffer->viewport_x = 0;
> -  framebuffer->viewport_y = 0;
> -  framebuffer->viewport_width = width;
> -  framebuffer->viewport_height = height;
> -
> -  /* If the framebuffer being updated is the current framebuffer we
> -   * mark the viewport state as changed so it will be updated the next
> -   * time _cogl_framebuffer_flush_state() is called. */
> -  if (framebuffer->context->current_draw_buffer == framebuffer)
> -    framebuffer->context->current_draw_buffer_changes |=
> -      COGL_FRAMEBUFFER_STATE_VIEWPORT;
> +  cogl_framebuffer_set_viewport (framebuffer, 0, 0, width, height);
>  }
>
>  void
> --
> 1.7.11.3.g3c3efa5
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/cogl/attachments/20121120/323d2c9c/attachment-0001.html>