[Mesa-dev] [PATCH] meta: Clip src/dest rects in BlitFramebuffer, using the scissor

[Anuj Phogat]

On Tue, Apr 15, 2014 at 7:15 PM, Chris Forbes <chrisf at ijw.co.nz> wrote:
> Fixes piglit's fbo-blit-stretch test on drivers which use the meta path.
> (i965: should fix Broadwell, but also fixes Sandybridge/Ivybridge/Haswell
> since this test falls off the blorp path now due to format conversion)
>
> V2: Use scissor instead of just mangling the rects, to avoid texcoord
> rounding problems. (Thanks Marek)
>
> V3: Rebase on Eric's CTSI meta changes; re-add _mesa_update_state in the
> CTSI path so that _mesa_clip_blit sees the correct bounds.
>
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77414
> Cc: Eric Anholt <eric at anholt.net>
> ---
>  src/mesa/drivers/common/meta.c      |  7 +++++++
>  src/mesa/drivers/common/meta_blit.c | 38 +++++++++++++++++++++++++++++++++++--
>  2 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index ddd0b1a..ac27abb 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -2783,6 +2783,13 @@ copytexsubimage_using_blit_framebuffer(struct gl_context *ctx, GLuint dims,
>        goto out;
>
>     ctx->Meta->Blit.no_ctsi_fallback = true;
> +
> +   /* Since we've bound a new draw framebuffer, we need to update
> +    * its derived state -- _Xmin, etc -- for BlitFramebuffer's clipping to
> +    * be correct.
> +    */
> +   _mesa_update_state(ctx);
> +
>     /* We skip the core BlitFramebuffer checks for format consistency, which
>      * are too strict for CopyTexImage.  We know meta will be fine with format
>      * changes.
> diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c
> index 69a600c..5d72dd2 100644
> --- a/src/mesa/drivers/common/meta_blit.c
> +++ b/src/mesa/drivers/common/meta_blit.c
> @@ -33,11 +33,13 @@
>  #include "main/enable.h"
>  #include "main/enums.h"
>  #include "main/fbobject.h"
> +#include "main/image.h"
>  #include "main/macros.h"
>  #include "main/matrix.h"
>  #include "main/multisample.h"
>  #include "main/objectlabel.h"
>  #include "main/readpix.h"
> +#include "main/scissor.h"
>  #include "main/shaderapi.h"
>  #include "main/texobj.h"
>  #include "main/texenv.h"
> @@ -627,6 +629,15 @@ _mesa_meta_BlitFramebuffer(struct gl_context *ctx,
>     const GLint dstH = abs(dstY1 - dstY0);
>     const GLint dstFlipX = (dstX1 - dstX0) / dstW;
>     const GLint dstFlipY = (dstY1 - dstY0) / dstH;
> +
> +   struct {
> +      GLint srcX0, srcY0, srcX1, srcY1;
> +      GLint dstX0, dstY0, dstX1, dstY1;
> +   } clip = {
> +      srcX0, srcY0, srcX1, srcY1,
> +      dstX0, dstY0, dstX1, dstY1
> +   };
> +
>     const GLboolean use_glsl_version = ctx->Extensions.ARB_vertex_shader &&
>                                        ctx->Extensions.ARB_fragment_shader;
>
> @@ -636,8 +647,31 @@ _mesa_meta_BlitFramebuffer(struct gl_context *ctx,
>        goto fallback;
>     }
>
> -   /* only scissor effects blit so save/clear all other relevant state */
> -   _mesa_meta_begin(ctx, ~MESA_META_SCISSOR);
> +   /* Clip a copy of the blit coordinates. If these differ from the input
> +    * coordinates, then we'll set the scissor.
> +    */
> +   if (!_mesa_clip_blit(ctx, &clip.srcX0, &clip.srcY0, &clip.srcX1, &clip.srcY1,
> +                        &clip.dstX0, &clip.dstY0, &clip.dstX1, &clip.dstY1)) {
> +      /* clipped/scissored everything away */
> +      return;
> +   }
> +
> +   /* Only scissor affects blit, but we're doing to set a custom scissor if
> +    * necessary anyway, so save/clear state.
> +    */
> +   _mesa_meta_begin(ctx, MESA_META_ALL);
> +
> +   /* If the clipping earlier changed the destination rect at all, then
> +    * enable the scissor to clip to it.
> +    */
> +   if (clip.dstX0 != dstX0 || clip.dstY0 != dstY0 ||
> +       clip.dstX1 != dstX1 || clip.dstY1 != dstY1) {
> +      _mesa_set_enable(ctx, GL_SCISSOR_TEST, GL_TRUE);
> +      _mesa_Scissor(MIN2(clip.dstX0, clip.dstX1),
> +                    MIN2(clip.dstY0, clip.dstY1),
> +                    abs(clip.dstX0 - clip.dstX1),
> +                    abs(clip.dstY0 - clip.dstY1));
> +   }
>
>     /* Try faster, direct texture approach first */
>     if (mask & GL_COLOR_BUFFER_BIT) {
> --

Thanks for fixing it Chris. I spent some time on this earlier but got
distracted.
It is little tricky to notice that clipping was affecting the test.

Reviewed-by and Tested-by: Anuj Phogat <anuj.phogat at gmail.com>

> 1.9.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev