[Mesa-dev] [PATCH] meta/TexSubImage: Stash everything other than PIXEL_TRANSFER/store in meta_begin

[Neil Roberts]

This looks good to me. It does seem a shame to save everything though
when so little state affects glBlitFramebuffer. However I think even if
we changed it to just save the scissor state we'd still have a problem
with not saving the the GL_FRAMEBUFFER_SRGB state which I think we also
need to do. I guess it's more robust to just save everything in case
more state gets added one day. The meta stuff is really difficult
because every time you add new state you really need to check every use
of _mesa_meta_begin to decide whether that state should affect that
operation or not and it's really easy to get it wrong.

I think the patch could do with a small commit message mentioning that
the scissor and SRGB states were broken.

Either way,
Reviewed-by: Neil Roberts <neil at linux.intel.com>

Regards,
- Neil

Jason Ekstrand <jason at jlekstrand.net> writes:

> Cc: 10.5 <mesa-stable at lists.freedesktop.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89292
> ---
>  src/mesa/drivers/common/meta_tex_subimage.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c b/src/mesa/drivers/common/meta_tex_subimage.c
> index 5ae12c8..802430a 100644
> --- a/src/mesa/drivers/common/meta_tex_subimage.c
> +++ b/src/mesa/drivers/common/meta_tex_subimage.c
> @@ -192,8 +192,8 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims,
>     if (allocate_storage)
>        ctx->Driver.AllocTextureImageBuffer(ctx, tex_image);
>  
> -   /* Only stash the current FBO */
> -   _mesa_meta_begin(ctx, 0);
> +   _mesa_meta_begin(ctx, ~(MESA_META_PIXEL_TRANSFER |
> +                           MESA_META_PIXEL_STORE));
>  
>     _mesa_GenFramebuffers(2, fbos);
>     _mesa_BindFramebuffer(GL_READ_FRAMEBUFFER, fbos[0]);
> @@ -311,8 +311,8 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims,
>     if (!pbo_tex_image)
>        return false;
>  
> -   /* Only stash the current FBO */
> -   _mesa_meta_begin(ctx, 0);
> +   _mesa_meta_begin(ctx, ~(MESA_META_PIXEL_TRANSFER |
> +                           MESA_META_PIXEL_STORE));
>  
>     _mesa_GenFramebuffers(2, fbos);
>  
> -- 
> 2.3.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 472 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150302/fed2aea7/attachment.sig>