[Mesa-dev] [PATCH V2 14/15] vbo: map indirect buffer and extract params if doing sw primitive restart

[Brian Paul]

On 11/07/2013 11:29 AM, Eric Anholt wrote:
> Brian Paul <brianp at vmware.com> writes:
>
>> On 11/07/2013 10:23 AM, Eric Anholt wrote:
>>> Chris Forbes <chrisf at ijw.co.nz> writes:
>>>
>>>> V2: Check for mapping failure (thanks Brian)
>>>>
>>>> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
>>>> ---
>>>>    src/mesa/vbo/vbo_primitive_restart.c | 33 +++++++++++++++++++++++++++++++++
>>>>    1 file changed, 33 insertions(+)
>>>>
>>>> diff --git a/src/mesa/vbo/vbo_primitive_restart.c b/src/mesa/vbo/vbo_primitive_restart.c
>>>> index 48d04e1..ee84f10 100644
>>>> --- a/src/mesa/vbo/vbo_primitive_restart.c
>>>> +++ b/src/mesa/vbo/vbo_primitive_restart.c
>>>> @@ -180,6 +180,39 @@ vbo_sw_primitive_restart(struct gl_context *ctx,
>>>>       GLboolean map_ib = ib->obj->Name && !ib->obj->Pointer;
>>>>       void *ptr;
>>>>
>>>> +   /* If there is an indirect buffer, map it and extract the draw params */
>>>> +   if (indirect && prims[0].is_indirect) {
>>>> +      struct _mesa_prim new_prim = *prims;
>>>> +      struct _mesa_index_buffer new_ib = *ib;
>>>> +      const uint32_t *indirect_params;
>>>> +      if (!ctx->Driver.MapBufferRange(ctx, 0, indirect->Size, GL_MAP_READ_BIT,
>>>> +            (struct gl_buffer_object *)indirect)) {
>>>
>>> Are you just casting away the const here?  indirect should definitely
>>> not be const, because you're modifying it in calls like this.
>>
>> Yeah, he's casting away const, but the buffer's being mapped for
>> read-only.  An indirect drawing call should never modify the indirect
>> buffer's contents.
>
> But it sure does modify the indirect buffer object's struct, since it's
> getting used to do the call.  In the i965 case, the struct's "what bits
> have been referenced by the GPU since our last stall?" get modified both
> in the normal draw path and in the fallback path.
>
> Even state_tracker modifies the struct:
>
>     obj->Pointer = pipe_buffer_map_range(pipe,
>                                          st_obj->buffer,
>                                          offset, length,
>                                          flags,
>                                          &st_obj->transfer);
>

OK, we're just talking at different levels.  From the GL API point of 
view, the indirect buffer is not modified by the drawing call.  If/when 
I suggested/questioned making it const, that what I had in mind.

 From the implementation point of view, const qualifying it does seem to 
be too much trouble.  I realize that mapping a buffer, even for 
read-only, can involve modifying the gl_buffer_object.

Please remove the const qualifier then.  Sorry for the confusion.

-Brian