[Mesa-dev] [PATCH 2/2] st/mesa: fix GLSL uniform updates for glBitmap & glDrawPixels
Marek Olšák
maraeo at gmail.com
Wed Dec 9 14:03:59 PST 2015
On Mon, Dec 7, 2015 at 5:36 PM, Brian Paul <brianp at vmware.com> wrote:
> On 12/06/2015 04:34 PM, Marek Olšák wrote:
>>
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> Spotted by luck. The GLSL uniform storage is only associated once
>> in LinkShader and can't be reallocated afterwards, because that would
>> break the association.
>> ---
>> src/mesa/state_tracker/st_cb_bitmap.c | 6 +++++-
>> src/mesa/state_tracker/st_cb_drawpixels.c | 6 ------
>> src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 6 ++++++
>> src/mesa/state_tracker/st_program.c | 17 ++++-------------
>> src/mesa/state_tracker/st_program.h | 1 -
>> 5 files changed, 15 insertions(+), 21 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_cb_bitmap.c
>> b/src/mesa/state_tracker/st_cb_bitmap.c
>> index cbc6845..a4a48a6 100644
>> --- a/src/mesa/state_tracker/st_cb_bitmap.c
>> +++ b/src/mesa/state_tracker/st_cb_bitmap.c
>> @@ -287,7 +287,8 @@ draw_bitmap_quad(struct gl_context *ctx, GLint x,
>> GLint y, GLfloat z,
>> GLfloat colorSave[4];
>> COPY_4V(colorSave, ctx->Current.Attrib[VERT_ATTRIB_COLOR0]);
>> COPY_4V(ctx->Current.Attrib[VERT_ATTRIB_COLOR0], color);
>> - st_upload_constants(st, fpv->parameters, PIPE_SHADER_FRAGMENT);
>> + st_upload_constants(st, st->fp->Base.Base.Parameters,
>> + PIPE_SHADER_FRAGMENT);
>> COPY_4V(ctx->Current.Attrib[VERT_ATTRIB_COLOR0], colorSave);
>> }
>>
>> @@ -404,6 +405,9 @@ draw_bitmap_quad(struct gl_context *ctx, GLint x,
>> GLint y, GLfloat z,
>> cso_restore_stream_outputs(cso);
>>
>> pipe_resource_reference(&vbuf, NULL);
>> +
>> + /* We uploaded modified constants, need to invalidate them. */
>> + st->dirty.mesa |= _NEW_PROGRAM_CONSTANTS;
>> }
>>
>>
>> diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c
>> b/src/mesa/state_tracker/st_cb_drawpixels.c
>> index 262ad80..e295f54 100644
>> --- a/src/mesa/state_tracker/st_cb_drawpixels.c
>> +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
>> @@ -1109,9 +1109,6 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint
>> y,
>>
>> st->pixel_xfer.pixelmap_sampler_view);
>> num_sampler_view++;
>> }
>> -
>> - /* update fragment program constants */
>> - st_upload_constants(st, fpv->parameters, PIPE_SHADER_FRAGMENT);
>> }
>>
>> /* Put glDrawPixels image into a texture */
>> @@ -1462,9 +1459,6 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx,
>> GLint srcy,
>>
>> st->pixel_xfer.pixelmap_sampler_view);
>> num_sampler_view++;
>> }
>> -
>> - /* update fragment program constants */
>> - st_upload_constants(st, fpv->parameters, PIPE_SHADER_FRAGMENT);
>> }
>> else {
>> assert(type == GL_DEPTH);
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index 40c7725..a32c4cf 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -5640,6 +5640,12 @@ get_mesa_program(struct gl_context *ctx,
>>
>> _mesa_reference_program(ctx, &shader->Program, prog);
>>
>> + /* Avoid reallocation of the program parameter list, because the
>> uniform
>> + * storage is only associated with the original parameter list.
>> + * This should be enough for Bitmap and DrawPixels constants.
>> + */
>> + _mesa_reserve_parameter_storage(prog->Parameters, 8);
>> +
>> /* This has to be done last. Any operation the can cause
>> * prog->ParameterValues to get reallocated (e.g., anything that adds
>> a
>> * program constant) has to happen before creating this linkage.
>> diff --git a/src/mesa/state_tracker/st_program.c
>> b/src/mesa/state_tracker/st_program.c
>> index 75ccaf2..39c54c2 100644
>> --- a/src/mesa/state_tracker/st_program.c
>> +++ b/src/mesa/state_tracker/st_program.c
>> @@ -112,8 +112,6 @@ delete_fp_variant(struct st_context *st, struct
>> st_fp_variant *fpv)
>> {
>> if (fpv->driver_shader)
>> cso_delete_fragment_shader(st->cso_context, fpv->driver_shader);
>> - if (fpv->parameters)
>> - _mesa_free_parameter_list(fpv->parameters);
>> free(fpv);
>> }
>>
>> @@ -914,8 +912,6 @@ st_create_fp_variant(struct st_context *st,
>> if (tgsi.tokens != stfp->tgsi.tokens)
>> tgsi_free_tokens(tgsi.tokens);
>> tgsi.tokens = tokens;
>> - variant->parameters =
>> - _mesa_clone_parameter_list(stfp->Base.Base.Parameters);
>> } else
>> fprintf(stderr, "mesa: cannot create a shader for glBitmap\n");
>> }
>> @@ -924,6 +920,7 @@ st_create_fp_variant(struct st_context *st,
>> if (key->drawpixels) {
>> const struct tgsi_token *tokens;
>> unsigned scale_const = 0, bias_const = 0, texcoord_const = 0;
>> + struct gl_program_parameter_list *params =
>> stfp->Base.Base.Parameters;
>>
>> /* Find the first unused slot. */
>> variant->drawpix_sampler = ffs(~stfp->Base.Base.SamplersUsed) - 1;
>> @@ -935,27 +932,21 @@ st_create_fp_variant(struct st_context *st,
>> variant->pixelmap_sampler = ffs(~samplers_used) - 1;
>> }
>>
>> - variant->parameters =
>> - _mesa_clone_parameter_list(stfp->Base.Base.Parameters);
>> -
>> if (key->scaleAndBias) {
>> static const gl_state_index scale_state[STATE_LENGTH] =
>> { STATE_INTERNAL, STATE_PT_SCALE };
>> static const gl_state_index bias_state[STATE_LENGTH] =
>> { STATE_INTERNAL, STATE_PT_BIAS };
>>
>> - scale_const = _mesa_add_state_reference(variant->parameters,
>> - scale_state);
>> - bias_const = _mesa_add_state_reference(variant->parameters,
>> - bias_state);
>> + scale_const = _mesa_add_state_reference(params, scale_state);
>> + bias_const = _mesa_add_state_reference(params, bias_state);
>> }
>>
>> {
>> static const gl_state_index state[STATE_LENGTH] =
>> { STATE_INTERNAL, STATE_CURRENT_ATTRIB, VERT_ATTRIB_TEX0 };
>>
>> - texcoord_const = _mesa_add_state_reference(variant->parameters,
>> - state);
>> + texcoord_const = _mesa_add_state_reference(params, state);
>> }
>>
>> tokens = st_get_drawpix_shader(tgsi.tokens,
>> diff --git a/src/mesa/state_tracker/st_program.h
>> b/src/mesa/state_tracker/st_program.h
>> index d9b53ac..a8571f0 100644
>> --- a/src/mesa/state_tracker/st_program.h
>> +++ b/src/mesa/state_tracker/st_program.h
>> @@ -80,7 +80,6 @@ struct st_fp_variant
>> void *driver_shader;
>>
>> /** For glBitmap variants */
>> - struct gl_program_parameter_list *parameters;
>> uint bitmap_sampler;
>>
>> /** For glDrawPixels variants */
>>
>
> Reviewed-by: Brian Paul <brianp at vmware.com>
> Should this be tagged for the 11.1 / stable branches?
Yes, I'll tag them for stable.
Marek
More information about the mesa-dev
mailing list