Ian Romanick <i...@freedesktop.org> writes: > On 06/25/2013 11:57 AM, Eric Anholt wrote: >> Ian Romanick <i...@freedesktop.org> writes: >> >>> From: Ian Romanick <ian.d.roman...@intel.com> >>> >>> The checks to determine when the data can be uploaded in an interleaved >>> fashion can be tricked by certain data layouts. For example, >>> >>> float data[...]; >>> >>> glVertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, 16, &data[0]); >>> glVertexAttribPointer(1, 4, GL_FLOAT, GL_FALSE, 16, &data[4]); >>> glDrawArrays(GL_POINTS, 0, 1); >>> >>> will hit the interleaved path with an incorrect size (16 bytes instead >>> of 32 bytes). As a result, the data for attribute 1 never gets >>> uploaded. The single element draw case is the only sensible case I can >>> think of for non-interleaved-that-looks-like-interleaved data, but there >>> may be others as well. >>> >>> To fix this, make sure that the end of the element in the array being >>> checked is within the stride "window." Previously the code would check >>> that the begining of the element was within the window. >>> >>> NOTE: This is a candidate for stable branches. >>> >>> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com> >>> Cc: Kenneth Graunke <kenn...@whitecape.org> >>> Cc: Eric Anholt <e...@anholt.net> >>> --- >>> src/mesa/drivers/dri/i965/brw_draw_upload.c | 16 +++++++++++++++- >>> 1 file changed, 15 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c >>> b/src/mesa/drivers/dri/i965/brw_draw_upload.c >>> index 2ded14b..d19250b 100644 >>> --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c >>> +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c >>> @@ -506,8 +506,22 @@ static void brw_prepare_vertices(struct brw_context >>> *brw) >>> ptr = glarray->Ptr; >>> } >>> else if (interleaved != glarray->StrideB || >>> - (uintptr_t)(glarray->Ptr - ptr) > interleaved) >>> + (uintptr_t)(glarray->Ptr - ptr) + glarray->_ElementSize >>> > interleaved) >> >> I think we should retain the previous check, too, to make sure that we >> don't do it if people have overlapping elements where attribute 1 starts >> just before and extending into the start of attribute 0. It sounds >> crazy, but it'll be hell to debug if anyone ever does. (I'm imagining a >> single-element attribute like the original bug report, where someone >> stacked up a vec2 before a vec4, but accidentally specified "16" as the >> stride on the vec2 because it shouldn't matter) >> > > git n00b failure. :( I have an uncommitted change in my tree that makes > the check actually work. > > + glarray->Ptr < ptr || > > Without this, piles of piglit tests fail because the data is interleaved > in the opposite order from what the fast path can handle... basically > making more instances of the original bug. > > I also think this addition covers the case you're concerned about. Yeah?
Yeah, that works.
pgpMa5t6OiEx0.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
