[Mesa-dev] [PATCH 08/15] st/mesa: make generic CopyPixels path work with MSAA visuals

[Brian Paul]

On Sat, Jun 1, 2013 at 8:29 AM, Marek Olšák <maraeo at gmail.com> wrote:

> We have to use pipe->blit, not resource_copy_region, so that the read
> buffer
> is resolved if it's multisampled. I also removed the CPU-based copying,
> which just did format conversion (obsoleted by the blit).
>
> Also, the layer/slice/face of the read buffer is taken into account (this
> was
> ignored).
>
> Last but not least, the format choosing is improved to take float and
> integer
> read buffers into account.
> ---
>  src/mesa/state_tracker/st_cb_drawpixels.c |  162
> +++++++++++++----------------
>  1 file changed, 70 insertions(+), 92 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c
> b/src/mesa/state_tracker/st_cb_drawpixels.c
> index eb75500..1c26315 100644
> --- a/src/mesa/state_tracker/st_cb_drawpixels.c
> +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
> @@ -1474,10 +1474,9 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx,
> GLint srcy,
>     struct pipe_sampler_view *sv[2];
>     int num_sampler_view = 1;
>     GLfloat *color;
> -   enum pipe_format srcFormat, texFormat;
> +   enum pipe_format srcFormat;
>     GLboolean invertTex = GL_FALSE;
>     GLint readX, readY, readW, readH;
> -   GLuint sample_count;
>     struct gl_pixelstore_attrib pack = ctx->DefaultPacking;
>     struct st_fp_variant *fpv;
>
> @@ -1539,35 +1538,51 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx,
> GLint srcy,
>     /* update fragment program constants */
>     st_upload_constants(st, fpv->parameters, PIPE_SHADER_FRAGMENT);
>
> -   sample_count = rbRead->texture->nr_samples;
> -   /* I believe this would be legal, presumably would need to do a resolve
> -      for color, and for depth/stencil spec says to just use one of the
> -      depth/stencil samples per pixel? Need some transfer clarifications.
> */
> -   assert(sample_count < 2);
> -
> +   /* Choose the format for the temporary texture. */
>     srcFormat = rbRead->texture->format;
>
> -   if (screen->is_format_supported(screen, srcFormat, st->internal_target,
> -                                   sample_count,
> -                                   PIPE_BIND_SAMPLER_VIEW)) {
> -      texFormat = srcFormat;
> -   }
> -   else {
> -      /* srcFormat can't be used as a texture format */
> +   if (!screen->is_format_supported(screen, srcFormat,
> st->internal_target, 0,
> +                                    PIPE_BIND_SAMPLER_VIEW |
> +                                    (type == GL_COLOR ?
> PIPE_BIND_RENDER_TARGET
> +                                     : PIPE_BIND_DEPTH_STENCIL))) {
>        if (type == GL_DEPTH) {
> -         texFormat = st_choose_format(st, GL_DEPTH_COMPONENT,
> -                                      GL_NONE, GL_NONE,
> st->internal_target,
> -                                      sample_count,
> PIPE_BIND_DEPTH_STENCIL,
> -                                      FALSE);
> -         assert(texFormat != PIPE_FORMAT_NONE);
> +         srcFormat = st_choose_format(st, GL_DEPTH_COMPONENT, GL_NONE,
> +                                      GL_NONE, st->internal_target, 0,
> +                                      PIPE_BIND_SAMPLER_VIEW |
> +                                      PIPE_BIND_DEPTH_STENCIL, FALSE);
>        }
>        else {
> -         /* default color format */
> -         texFormat = st_choose_format(st, GL_RGBA,
> -                                      GL_NONE, GL_NONE,
> st->internal_target,
> -                                      sample_count,
> PIPE_BIND_SAMPLER_VIEW,
> -                                      FALSE);
> -         assert(texFormat != PIPE_FORMAT_NONE);
> +         assert(type == GL_COLOR);
> +
> +         if (util_format_is_float(srcFormat)) {
> +            srcFormat = st_choose_format(st, GL_RGBA32F, GL_NONE,
> +                                         GL_NONE, st->internal_target, 0,
> +                                         PIPE_BIND_SAMPLER_VIEW |
> +                                         PIPE_BIND_RENDER_TARGET, FALSE);
> +         }
> +         else if (util_format_is_pure_sint(srcFormat)) {
> +            srcFormat = st_choose_format(st, GL_RGBA32I, GL_NONE,
> +                                         GL_NONE, st->internal_target, 0,
> +                                         PIPE_BIND_SAMPLER_VIEW |
> +                                         PIPE_BIND_RENDER_TARGET, FALSE);
> +         }
> +         else if (util_format_is_pure_uint(srcFormat)) {
> +            srcFormat = st_choose_format(st, GL_RGBA32UI, GL_NONE,
> +                                         GL_NONE, st->internal_target, 0,
> +                                         PIPE_BIND_SAMPLER_VIEW |
> +                                         PIPE_BIND_RENDER_TARGET, FALSE);
> +         }
> +         else {
> +            srcFormat = st_choose_format(st, GL_RGBA, GL_NONE,
> +                                         GL_NONE, st->internal_target, 0,
> +                                         PIPE_BIND_SAMPLER_VIEW |
> +                                         PIPE_BIND_RENDER_TARGET, FALSE);
> +         }
> +      }
>

I seem to recall that this "choose a gallium format for a given GL
format/type" code appears elsewhere in the state tracker (but haven't
double-checked).  In any case, it would be nicer if this code was moved
into a separate function.



> +
> +      if (srcFormat == PIPE_FORMAT_NONE) {
> +         assert(0 && "cannot choose a format for src of CopyPixels");
> +         return;
>        }
>     }
>
>
Reviewed-by: Brian Paul <brianp at vmware.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130603/98f5a203/attachment.html>