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

[Chris Forbes]

Maybe it would be better to remove the const from the indirect
parameter back in 1/15.

This was added in v2 of Christoph's original patches in response to a
suggestion from Brian; but if everyone ends up casting it away, then
it's not useful.

-- Chris

On Fri, Nov 8, 2013 at 7:29 AM, Eric Anholt <eric at anholt.net> 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);
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>