On 03/13/2014 08:59 AM, Ian Romanick wrote:
> On 03/13/2014 01:57 AM, Kenneth Graunke wrote:
>> This function has three cases:
>> 1. Data is in user arrays
>> 2. Data is in a buffer object, but misaligned.
>> 3. Data is acceptable as is.
>>
>> Previously, there was a "Turn into a proper VBO" comment above all three
>> cases, even though it only applies to case 1, and maybe case 2 (with a
>> specific interpretation of "proper".) This patch replaces that with
>> more specific comments for each case.
>>
>> The expanded comments explain where the alignment restrictions come from
>> and give an example of how to produce misaligned data in OpenGL.
>>
>> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
>> ---
>> src/mesa/drivers/dri/i965/brw_draw_upload.c | 20 ++++++++++++++------
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
>> b/src/mesa/drivers/dri/i965/brw_draw_upload.c
>> index f2945c1..98a8277 100644
>> --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
>> +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
>> @@ -838,18 +838,25 @@ static void brw_upload_indices(struct brw_context *brw)
>> ib_size = ib_type_size * index_buffer->count;
>> bufferobj = index_buffer->obj;
>>
>> - /* Turn into a proper VBO:
>> - */
>> if (!_mesa_is_bufferobj(bufferobj)) {
>> - /* Get new bufferobj, offset:
>> - */
>> + /* Copy the index data from user arrays into a buffer object. */
>> intel_upload_data(brw, index_buffer->ptr, ib_size, ib_type_size,
>> &bo, &offset);
>> } else {
>> offset = (GLuint) (unsigned long) index_buffer->ptr;
>>
>> - /* If the index buffer isn't aligned to its element size, we have to
>> - * rebase it into a temporary.
>> + /* Even though the index data is in a buffer object, it may not meet
>> + * the hardware's alignment restrictions. From the
>> 3DSTATE_INDEX_BUFFER
>> + * documentation, DWord 1:
>> + *
>> + * "This field contains the size-aligned (as specified by Index
>> Format)
>> + * Graphics Address of the first element of interest within the index
>> + * buffer."
>> + *
>> + * OpenGL allows arbitrary byte offsets into the buffer. For example,
>> + * glDrawElements with index == 1 and a type of GL_UNSIGNED_SHORT.
>> + *
>> + * If the data isn't aligned to the element size, copy it to a
>> temporary.
>
> Have we actually encountered applications that do that? The GL spec
> does say:
>
> "Clients must align data elements consistent with the requirements
> of the client platform, with an additional base-level requirement
> that an offset within a buffer to a datum comprising N basic
> machine units be a multiple of N."
I sure don't know of any. Haihao added this code in 2007 - commit
ea07a0df9a2f689b8f5acaf92c40bbbd602cab3c, referencing bug #11910, where
someone (probably on the QA team at the time?) had written a test case
that did this, and used an offset of 1 when they probably meant 1 *
sizeof(GLuint).
Based on your spec citation, this sure sounds like it should never
happen. I don't see any code in api_validate.c or vbo_exec_array.c that
checks for misalignment, raises an error, and prevents us from reaching
the drawing function...but maybe I'm just blind. It seems like such
code ought to exist.
>> */
>> if ((ib_type_size - 1) & offset) {
>> perf_debug("copying index buffer to a temporary to work around "
>> @@ -866,6 +873,7 @@ static void brw_upload_indices(struct brw_context *brw)
>>
>> ctx->Driver.UnmapBuffer(ctx, bufferobj, MAP_INTERNAL);
>> } else {
>> + /* The index data is already in an acceptable BO. Use as is. */
>> bo = intel_bufferobj_buffer(brw, intel_buffer_object(bufferobj),
>> offset, ib_size);
>> drm_intel_bo_reference(bo);
>>
>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
