[Mesa-dev] [PATCH] mesa/state_tracker: Fix draw-pixel-with-texture piglit test.

Marek Olšák maraeo at gmail.com
Sun Oct 4 18:11:16 PDT 2015


Thank you for the patch, but it's not applicable on my (private)
branch where I'm reworking st/mesa's shader compilation, including
DrawPixels shaders. Your patch helped to fix it in the new
implementation at least.

Sorry for ignoring it in the first place.

Marek

On Sat, Mar 28, 2015 at 6:07 PM, Matthew Dawson <matthew at mjdsystems.ca> wrote:
> When glDrawPixels was used with an external texture, the pixels passed in
> were sampled instead of the texture.  Change gallium to instead move the user
> texture to a new sampler below the glDrawPixels samplers and use the texture
> coordinates from the raster position.
>
> This uses a uniform for the texture coordinates instead passing it through
> the vertex shader as the texture coordinates are constant for the entire
> operation.  While working the vertex shader would be possible, implementing
> that solution would break several assumptions throughout the glDrawPixels
> implementation as well as helper functions used by other code paths, increasing
> the chance for breakage.
>
> Tested on llvmpipe, r600, and radeonsi.
>
> V2: Complete everything missing from V1.
>
> v3:
>  - Fix style issues.
>  - Add comments about how the texture references are fixed up to be valid.
> ---
>  src/mesa/state_tracker/st_cb_drawpixels.c  | 39 ++++++++++++++++++++++++++---
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 40 ++++++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c
> index 3edf31b..47abd16 100644
> --- a/src/mesa/state_tracker/st_cb_drawpixels.c
> +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
> @@ -44,6 +44,7 @@
>  #include "main/texstore.h"
>  #include "main/glformats.h"
>  #include "program/program.h"
> +#include "program/prog_parameter.h"
>  #include "program/prog_print.h"
>  #include "program/prog_instruction.h"
>
> @@ -676,6 +677,8 @@ draw_textured_quad(struct gl_context *ctx, GLint x, GLint y, GLfloat z,
>     GLfloat x0, y0, x1, y1;
>     GLsizei maxSize;
>     boolean normalized = sv[0]->texture->target != PIPE_TEXTURE_RECT;
> +   GLuint num_user_samplers = st->state.num_samplers[PIPE_SHADER_FRAGMENT];
> +   unsigned int i;
>
>     /* limit checks */
>     /* XXX if DrawPixels image is larger than max texture size, break
> @@ -765,6 +768,10 @@ draw_textured_quad(struct gl_context *ctx, GLint x, GLint y, GLfloat z,
>        if (num_sampler_view > 1) {
>           cso_single_sampler(cso, PIPE_SHADER_FRAGMENT, 1, &sampler);
>        }
> +      for (i = 0; i < num_user_samplers; ++i) {
> +         cso_single_sampler(cso, PIPE_SHADER_FRAGMENT, i+num_sampler_view,
> +                            &st->state.samplers[PIPE_SHADER_FRAGMENT][i]);
> +      }
>        cso_single_sampler_done(cso, PIPE_SHADER_FRAGMENT);
>     }
>
> @@ -786,7 +793,14 @@ draw_textured_quad(struct gl_context *ctx, GLint x, GLint y, GLfloat z,
>     cso_set_stream_outputs(st->cso_context, 0, NULL, NULL);
>
>     /* texture state: */
> -   cso_set_sampler_views(cso, PIPE_SHADER_FRAGMENT, num_sampler_view, sv);
> +   {
> +      struct pipe_sampler_view *lsv[PIPE_MAX_SAMPLERS];
> +      memcpy(lsv, sv, num_sampler_view*sizeof(struct pipe_sampler_view*));
> +      memcpy(lsv+num_sampler_view, st->state.sampler_views[PIPE_SHADER_FRAGMENT],
> +             num_user_samplers*sizeof(struct pipe_sampler_view*));
> +      cso_set_sampler_views(cso, PIPE_SHADER_FRAGMENT,
> +                            num_sampler_view+num_user_samplers, lsv);
> +   }
>
>     /* Compute Gallium window coords (y=0=top) with pixel zoom.
>      * Recall that these coords are transformed by the current
> @@ -1160,8 +1174,27 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y,
>        }
>     }
>
> -   /* update fragment program constants */
> -   st_upload_constants(st, fpv->parameters, PIPE_SHADER_FRAGMENT);
> +   /* updated texture coordinates and fragment program constants. */
> +   {
> +      int i;
> +      struct gl_program_parameter_list *parameters = fpv->parameters;
> +      /* Update the texture coordinates from all the used texture units, using
> +       * the values associated with the raster position */
> +      for (i = 0; i < parameters->NumParameters; ++i) {
> +         const char *name = parameters->Parameters[i].Name;
> +         /* Any texture coordinate will use the parameter name texcoord_, where
> +          * _ is the texture units id added to the ascii character 'A'.  This
> +          * checks if a parameter matches this patterns, and updates the
> +          * parameter's value with the appropriate raster position texture
> +          * coordinate. */
> +         if (strncmp("texcoord", name, 8) == 0 && strlen(name) == 9) {
> +            memcpy(parameters->ParameterValues[i],
> +                   st->ctx->Current.RasterTexCoords[name[8] - 'A'],
> +                   sizeof(GL_FLOAT) * 4);
> +         }
> +      }
> +      st_upload_constants(st, parameters, PIPE_SHADER_FRAGMENT);
> +   }
>
>     /* draw with textured quad */
>     {
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index e97ab83..b2f6e6e 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -4256,6 +4256,7 @@ get_pixel_transfer_visitor(struct st_fragment_program *fp,
>     st_src_reg coord, src0;
>     st_dst_reg dst0;
>     glsl_to_tgsi_instruction *inst;
> +   unsigned int count_samplers_used = 0;
>
>     /* Copy attributes of the glsl_to_tgsi_visitor in the original shader. */
>     v->ctx = original->ctx;
> @@ -4286,6 +4287,7 @@ get_pixel_transfer_visitor(struct st_fragment_program *fp,
>     prog->InputsRead |= VARYING_BIT_TEX0;
>     prog->SamplersUsed |= (1 << 0); /* mark sampler 0 as used */
>     v->samplers_used |= (1 << 0);
> +   count_samplers_used++;
>
>     if (scale_and_bias) {
>        static const gl_state_index scale_state[STATE_LENGTH] =
> @@ -4333,6 +4335,7 @@ get_pixel_transfer_visitor(struct st_fragment_program *fp,
>
>        prog->SamplersUsed |= (1 << 1); /* mark sampler 1 as used */
>        v->samplers_used |= (1 << 1);
> +      count_samplers_used++;
>
>        /* MOV colorTemp, temp; */
>        inst = v->emit(NULL, TGSI_OPCODE_MOV, dst0, temp);
> @@ -4361,6 +4364,43 @@ get_pixel_transfer_visitor(struct st_fragment_program *fp,
>        newinst = v->emit(NULL, inst->op, inst->dst[0], src_regs[0], src_regs[1], src_regs[2]);
>        newinst->tex_target = inst->tex_target;
>        newinst->sampler_array_size = inst->sampler_array_size;
> +
> +      /* If the original fragment shader (either user supplied, or generated from
> +       * the fixed function pipeline) references a texture, fix up the instruction
> +       * here to not conflict with the DrawPixels shader. */
> +      if (newinst->tex_target != 0) {
> +         int new_sampler_index;
> +         char name[10];
> +         GLint param;
> +
> +         /* Grab the original sampler, and move it up by the number of samplers
> +          * used by the DrawPixels shader. */
> +         newinst->sampler = inst->sampler;
> +         new_sampler_index = newinst->sampler.index + count_samplers_used;
> +
> +         /* This name records what texture coordinates are need for this parameter.
> +         * This is used later to update the texture coordinates as appropriate. */
> +         memcpy(name, "texcoord_", 10);
> +         name[8] = 'A' + newinst->sampler.index; // Replaces the underscore from above, while keeping the null byte.
> +
> +         if ((prog->SamplersUsed & (1 << new_sampler_index)) == 0) {
> +            /* If this is the first instruction referencing it, create a uniform
> +             * to store its texture coordinate, which will be a constant for the
> +             * whole drawn area */
> +            param = _mesa_add_parameter(params, PROGRAM_UNIFORM, name, 4, GL_FLOAT, (const gl_constant_value*)st->ctx->Current.RasterTexCoords[newinst->sampler.index], 0);
> +         } else {
> +            /* Otherwise get its parameter index to use below */
> +            param = _mesa_lookup_parameter_index(params, -1, name);
> +         }
> +         /* Update the new instruction with the new sampler index and texture
> +          * coordinate parameter. */
> +         newinst->src[0] = st_src_reg(PROGRAM_UNIFORM, param, GLSL_TYPE_FLOAT);
> +         newinst->sampler.index = new_sampler_index;
> +
> +         /* And mark the sampler as used */
> +         prog->SamplersUsed |= (1 << new_sampler_index);
> +         v->samplers_used |= (1 << new_sampler_index);
> +      }
>     }
>
>     /* Make modifications to fragment program info. */
> --
> 2.0.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list