Hi Brian,<br><br>The main reason for removal of pipe_vertex_buffer::max_index was its direct dependence on pipe_draw_info. The vertex buffer field was directly computed from the parameters of pipe_draw_info instead of being a completely independent state. So we ended up doing this in st/mesa:<br>
<br>set_vertex_buffer<br>draw_vbo<br>set_vertex_buffer<br>
draw_vbo<br>set_vertex_buffer<br>
draw_vbo<br><br>And this was completely impossible due to lack of info from GL when setting vertex pointers:<br><br>set_vertex_buffer<br>
draw_vbo<br>draw_vbo<br>
draw_vbo<br>
<br>Adding pipe_vertex_buffer::max_index back would be no different from putting vertex buffers in pipe_draw_info, because there is a direct dependency between them. In u_vbuf_mgr when draw_vbo is called, we try to guess the index bounds the same way we do it for GL (simply because there is no other way).<br>
<br>Let's decompose the whole problem of computing max_index into several cases:<br><br>1) Is the vertex buffer a non-user buffer?<br>- If stride == 0, max_index = 1.<br>- Otherwise, max_index = (width0 - buffer_offset - src_offset) / stride - 1.<br>
<br>That should work for user vertex buffers too, but there is an alternative way without having to use width0, it's what st/mesa is doing (though it might be wrong for instanced arrays, I didn't check):<br><br>2) Is the vertex buffer a user buffer?<br>
- If stride == 0, max_index = 1.<br>
- If instance_divisor != 0, max_index = start_instance + instance_count - 1.<br>- If !indexed, max_index = start + count - 1.<br>- Otherwise, st/mesa must go through the index buffer and obtain max_index from there (which it does).<br>
<br>There is no other way to compute max_index from the info we have from GL.<br><br>So that's all for the max_index issue.<br><br>On a slightly different note, we can't completely decouple the set_vertex_buffer and draw_vbo calls due to some trickery in the vbo module, so there is still a long way to go to make the two really independent of each other, i.e. several glDrawElements calls shouldn't cause calling set_vertex_buffer each time, but they sometimes do.<br>
<br>Marek<br><br><br><div class="gmail_quote">2011/4/15 Brian Paul <span dir="ltr"><<a href="mailto:brianp@vmware.com">brianp@vmware.com</a>></span><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<br>
Hi Marek,<br>
<br>
Back on Jan 10 you removed the per-vertex array max_index field (commit cdca3c58aa2d9549f5188910e2a77b438516714f). I believe this was a mistake.<br>
<br>
I noticed today that the piglit draw-instanced-divisor test is failing. A bisection points to Jose's commit 17bbc1f0425b3768e26473eccea5f2570dcb26d3. But that's a red herring. If I disable the SSE code paths, the regression goes back to the Jan 10 change.<br>
<br>
With the GL_ARB_instanced_arrays extension vertex array indexing changes quite a bit. Specifically, some arrays (those with divisors != 0) are now indexed by instance ID, not the primitive's vertex index. The draw_info::max_index field doesn't let us take this into account. I believe that we really need a per-array max_index value.<br>
<br>
As an example, suppose we're drawing a star field with 1 million instanced stars, each star drawn as a 4-vertex quad. We might use a vertex array indexed by the instance ID to color the stars.<br>
<br>
The draw call would look like:<br>
<br>
glDrawArraysInstanced(GL_QUADS, 0, 4, 1000*1000);<br>
<br>
In this case we'd have two vertex arrays. The first array is the quad vertex positions with four elements. The second array is the star colors with 1 million elements. As it is now, we're setting draw_info::max_index = 4 and we errantly clamp the index into the second array to 4 instead of 1 million.<br>
<br>
As a temporary work around we can disable clamping of array indexes for instance arrays. But I think we need to revert the Jan 10 commit and then rework some of Jose's related changes.<br><font color="#888888">
<br>
-Brian<br>
<br>
</font></blockquote></div><br>