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 <[email protected]> >> --- >> 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 [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
