<div class="gmail_quote">On Mon, Feb 14, 2011 at 6:58 PM, José Fonseca <<a href="mailto:jfonseca@vmware.com">jfonseca@vmware.com</a>> wrote: <blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> Marek, I'm OK with removing pipe_vertex_buffer::max_index but there is a bit more work involved, as they are not really equivalent in the guarantees. pipe_vertex_buffer::max_index is an attribute of the vertex buffer -- it describe the max index that can be fetch from the buffer without running into a buffer overflow. It is an hard limit -- it must be set accurately by the state tracker or crashes will occur. It can be removed because it can be derived from the vertex element size, vertex element stride, vertex buffer offset, and vertex buffer size. pipe_draw_info::max_index is an attribute of the index buffer: it describes the maximum index in the index buffer. It is an hint -- there may be higher indices in the index buffer, and if so it is OK for the driver to ignore those vertices, but it should not crash with a buffer overflow. Therefore, in order to safely remove pipe_vertex_buffer::max_index, we should compute the max_index inside the draw module / pipe drivers, and ensure vertices with higher indices will never be removed. There are a few places in this patch where you replace pipe_vertex_buffer::max_index with ~0 or no checks, which means that places which where previous robust to pipe_draw_info::max_index == ~0 and bogus indices will now start crashing. </blockquote></div> You're right in theory. In practice, pipe_vertex_buffer::max_index was really derived from the value which is put in pipe_draw_info::max_index and was correctly initialized for user buffers only. It was set to ~0 for hardware buffers. Moreover, it could also be computed with: pipe_vertex_buffer::max_index = (pipe_resource::width0 - pipe_vertex_buffer::buffer_offset) / pipe_vertex_buffer::stride - 1 So it was already redundant. Basically, pipe_resource::width0 is also redundant for user buffers, because it is actually computed from pipe_draw_info::max_index too. It's all logical because the index bounds are the only info we have about user buffers and we compute all the other properties from it. This is how width0 is computed: pipe_resource::width0 = (pipe_draw_info::max_index + 1) * pipe_vertex_buffer::stride; Now we substitute width0 in the first formula: pipe_vertex_buffer::max_index = ((pipe_draw_info::max_index + 1) * pipe_vertex_buffer::stride - pipe_vertex_buffer::buffer_offset) / pipe_vertex_buffer::stride - 1 If we examine the state tracker code, we'll notice that buffer_offset is always 0 for user buffers. After simplification, we get this: pipe_vertex_buffer::max_index = pipe_draw_info::max_index And that's the whole story. That said, I'd like not to call set_vertex_buffers only to update max_index if I can get the same info from pipe_draw_info. Because pipe_vertex_buffer::max_index was really the maximum index value in the index buffer, we don't need to clamp anything when we fetch vertices, the clamping would basically do nothing. That's why I removed the clamping in Draw and put ~0 in the translate::run parameter and in several other places. Does this explanation justify all of my changes in the patch to you? Marek