[Mesa-dev] [PATCH 2/6] st/mesa: overhaul vertex setup for clearing, glDrawPixels, glBitmap

[Brian Paul]

On 02/13/2016 01:03 PM, Ilia Mirkin wrote:
> On Fri, Feb 12, 2016 at 10:43 AM, Brian Paul <brianp at vmware.com> wrote:
>> diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c
>> index 9016846..cb2c390 100644
>> --- a/src/mesa/state_tracker/st_context.c
>> +++ b/src/mesa/state_tracker/st_context.c
>> @@ -241,16 +241,23 @@ st_create_context_priv( struct gl_context *ctx, struct pipe_context *pipe,
>>      else
>>         st->internal_target = PIPE_TEXTURE_RECT;
>>
>> -   /* Vertex element objects used for drawing rectangles for glBitmap,
>> -    * glDrawPixels, glClear, etc.
>> +   /* Setup vertex element info for 'struct st_util_vertex'.
>>       */
>> -   for (i = 0; i < ARRAY_SIZE(st->velems_util_draw); i++) {
>> -      memset(&st->velems_util_draw[i], 0, sizeof(struct pipe_vertex_element));
>> -      st->velems_util_draw[i].src_offset = i * 4 * sizeof(float);
>> -      st->velems_util_draw[i].instance_divisor = 0;
>> -      st->velems_util_draw[i].vertex_buffer_index =
>> -            cso_get_aux_vertex_buffer_slot(st->cso_context);
>> -      st->velems_util_draw[i].src_format = PIPE_FORMAT_R32G32B32A32_FLOAT;
>> +   {
>> +      const unsigned slot = cso_get_aux_vertex_buffer_slot(st->cso_context);
>
> Can the aux vertex buffer slot change over time? If so, you need some
> logic to update the vertex_buffer_index for these. From what I can
> tell it's always 0, not sure what the intention behind it is... seems
> like it'll be a very annoying problem to debug down the line should it
> ever change. Thoughts?

It's hard-wired to zero as you say but I imagine it could be computed by 
examining the current vertex buffer bindings state to find a free slot 
such that we might be able to avoid saving/restoring all the vertex 
buffer bindings.  I believe Marek wrote the code in question.

In any case, my patch doesn't change how/where the 
cso_get_aux_vertex_buffer_slot() function is used.  For now, I could add 
an assertion that slot==0 to help catch future issues.

Sound OK?

Does the series look OK to you overall?

-Brian


>
>> +
>> +      STATIC_ASSERT(sizeof(struct st_util_vertex) == 9 * sizeof(float));
>> +
>> +      memset(&st->util_velems, 0, sizeof(st->util_velems));
>> +      st->util_velems[0].src_offset = 0;
>> +      st->util_velems[0].vertex_buffer_index = slot;
>> +      st->util_velems[0].src_format = PIPE_FORMAT_R32G32B32_FLOAT;
>> +      st->util_velems[1].src_offset = 3 * sizeof(float);
>> +      st->util_velems[1].vertex_buffer_index = slot;
>> +      st->util_velems[1].src_format = PIPE_FORMAT_R32G32B32A32_FLOAT;
>> +      st->util_velems[2].src_offset = 7 * sizeof(float);
>> +      st->util_velems[2].vertex_buffer_index = slot;
>> +      st->util_velems[2].src_format = PIPE_FORMAT_R32G32_FLOAT;
>>      }
>>
>>      /* we want all vertex data to be placed in buffer objects */