Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.
On Sat, Mar 13, 2010 at 4:33 AM, Luca Barbieri luca.barbi...@gmail.com wrote: Actually, why is the state tracker doing the min/max computation at all? If the driver does the index lookup itself, as opposed to using an hardware index buffer, (e.g. the nouveau drivers do this in some cases) this is unnecessary and slow. Would completely removing the call to vbo_get_minmax_index break anything? Also, how about removing the max_index field in pipe_vertex_buffer? This seems to be set to the same value for all vertex buffers, and the value is then passed to draw_range_elements too. Isn't the value passed to draw_range_elements sufficient? It's really only needed for the special (but common) case where the vertex data isn't in a GL VBO at all, but just sitting in regular memory. The state tracker currently has the task of wrapping those regions of memory in user_buffers, and needs this computation to figure out what memory to wrap in that way. User buffers are a useful concept for avoiding a data copy, and can be implemented one of three ways in the driver: a) for drivers doing swtnl, just access the wrapped memory directly b) for drivers with hwtnl but unsophisticated memory managers, upload the userbuffer contents to a real buffer, then use that. c) for drivers with sophisticated memory managers, instruct the kernel to allow the hardware to access these pages directly and do whatever memory management tricks necessary to avoid the contents being clobbered by subsequent CPU accesses. It's a difficult trick to avoid extra data copies in a layered stack like gallium, and user_buffers are an attempt to avoid one source of them. But for any such technique, the mesa state tracker will need to figure out what memory is being referred to by those non-VBO vertex buffers and to do that requires knowing the index min/max values. Keith -- Download Intel#174; Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.
But for any such technique, the mesa state tracker will need to figure out what memory is being referred to by those non-VBO vertex buffers and to do that requires knowing the index min/max values. Isn't the min/max value only required to compute a sensible value for the maximum user buffer length? (the base pointer is passed to gl*Pointer) The fact is, that we don't need to know how large the user buffer is if the CPU is accessing it (or if we have a very advanced driver that faults memory in the GPU VM on demand, and/or a mechanism to let the GPU share the process address space). As you said, this happens for instance with swtnl, but also with drivers that scan the index buffer and copy the referenced vertex for each index onto the GPU FIFO themselves (e.g. nv50 and experimental versions of nv30/nv40). So couldn't we pass ~0 or similar as the user buffer length, and have the driver use an auxiliary module on draw calls to determine the real length, if necessary? Of course, drivers that upload user buffers on creation (if any exists) would need to be changed to only do that on draw calls. -- Download Intel#174; Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.
On Sat, Mar 13, 2010 at 11:40 AM, Luca Barbieri luca.barbi...@gmail.com wrote: But for any such technique, the mesa state tracker will need to figure out what memory is being referred to by those non-VBO vertex buffers and to do that requires knowing the index min/max values. Isn't the min/max value only required to compute a sensible value for the maximum user buffer length? (the base pointer is passed to gl*Pointer) Yes, I think that's what I was trying to say. The fact is, that we don't need to know how large the user buffer is if the CPU is accessing it (or if we have a very advanced driver that faults memory in the GPU VM on demand, and/or a mechanism to let the GPU share the process address space). Even for software tl, it's pretty important, see below. As you said, this happens for instance with swtnl, but also with drivers that scan the index buffer and copy the referenced vertex for each index onto the GPU FIFO themselves (e.g. nv50 and experimental versions of nv30/nv40). Having user-buffers with undefined size establishes a connection inside the driver between two things which could previously be fully understood separately - the vertex buffer is now no longer fully defined without reference to an index buffer. Effectively the user buffer become just unqualified pointers and we are back to the GL world pre-VBOs. In your examples, scanning and uploading (or transforming) per-index is only something which is sensible in special cases - eg where there are very few indices or sparse access to a large vertex buffer that hasn't already been uploaded/transformed. But you can't even know if the vertex buffer is sparse until you know how big it is, ie. what max_index is... Typical usage is the opposite - vertices are referenced more than once in a mesh and the efficient thing to do is: - for software tnl, transform all the vertices in the vertex buffer (requires knowing max_index) and then apply the indices to the transformed vertices - for hardware tnl, upload the vertex buffer in entirety (or better still, the referenced subrange). So couldn't we pass ~0 or similar as the user buffer length, and have the driver use an auxiliary module on draw calls to determine the real length, if necessary? Of course, drivers that upload user buffers on creation (if any exists) would need to be changed to only do that on draw calls. My feeling is that user buffers are a pretty significant concession to legacy GL vertex arrays in the interface. If there was any change to them, it would be more along the lines of getting rid of them entirely and forcing the state trackers to use proper buffers for everything. They are a nice way to accomodate old-style GL arrays, but they don't have many other uses and are already on shaky ground semantically. In summary, I'm not 100% comfortable with the current userbuffer concept, and I'd be *really* uncomfortable with the idea of buffers who's size we don't know or which could change size when a new index buffer is bound... Keith -- Download Intel#174; Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.
On Fri, Mar 12, 2010 at 6:53 PM, Roland Scheidegger srol...@vmware.com wrote: On 13.03.2010 03:20, Corbin Simpson wrote: I've pushed a revert of the original patch, and an r300g patch that, while not perfect, covers the common case that Wine hits. I think I don't really understand that (and the fix you did for r300g). Why can't you simply clamp the maxIndex to the minimum of the submitted maxIndex and the vertex buffer max index? Now you have this: maxIndex = MIN3(maxIndex, r300-vertex_buffer_max_index, count - minIndex); This is then used to set the hardware max index clamp. However, for example count could be 3, min index 0, but the actual vertices fetched 0, 15, 30 - as long as the vertex buffers are large enough this is perfectly legal, but as far as I can tell your patch would force the hardware to just fetch the 0th vertex (3 times). Count really tells you nothing at all about the index range (would also be legal to have huge count but very small valid index range if you fetch same vertices repeatedly). That's why I said that it's not perfect. :3 r300-vertex_buffer_max_index is, on this particular rendering path, *also* bogus, which is too bad because it should be used instead of maxIndex/minIndex here. But, as I said before, if it's too big then it can't be used, and we need to err on the side of caution if we err at all. One misrendered draw call is better than dropping the entire packet of commands on the floor. -- Only fools are easily impressed by what is only barely beyond their reach. ~ Unknown Corbin Simpson mostawesomed...@gmail.com -- Download Intel#174; Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.
On Fri, 2010-03-12 at 02:54 -0800, Corbin Simpson wrote:
Module: Mesa
Branch: master
Commit: 50876ddaaff72a324ac45e255985e0f84e108594
URL:
http://cgit.freedesktop.org/mesa/mesa/commit/?id=50876ddaaff72a324ac45e255985e0f84e108594
Author: Corbin Simpson mostawesomed...@gmail.com
Date: Fri Mar 12 02:51:40 2010 -0800
st/mesa: Always recalculate invalid index bounds.
These should always be sanitized before heading towards the pipe driver,
and if the calling function explicitly marked them as invalid, we need
to regenerate them.
Allows r300g to properly pass a bit more of Wine's d3d9 testing without
dropping stuff on the floor.
---
src/mesa/state_tracker/st_draw.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/mesa/state_tracker/st_draw.c
b/src/mesa/state_tracker/st_draw.c
index 8b01272..d81b361 100644
--- a/src/mesa/state_tracker/st_draw.c
+++ b/src/mesa/state_tracker/st_draw.c
@@ -542,9 +542,9 @@ st_draw_vbo(GLcontext *ctx,
assert(ctx-NewState == 0x0);
/* Gallium probably doesn't want this in some cases. */
- if (!index_bounds_valid)
- if (!vbo_all_varyings_in_vbos(arrays))
- vbo_get_minmax_index(ctx, prims, ib, min_index, max_index);
+ if (index_bounds_valid != GL_TRUE) {
+ vbo_get_minmax_index(ctx, prims, ib, min_index, max_index);
+ }
Corbin,
This isn't really desirable. Ideally this range checking should be
pushed into pipe driver, because it's an expensive operation that is not
necessary on a lot of hardware. #
Specifically, vertex fetch hardware can often be set up with the min/max
permissible index bounds to avoid accessing vertex data outside of the
bound VB's. This can be achieved by examining the VBs, with min_index
== 0, max_index = vb.size / vb.stride.
The case where we need to calculate them internally is if some data is
not in a VB, meaning we can't guess what the legal min/max values are.
Also note that we need that min/max information to be able to upload the
data to a VB.
So, I think the code was probably correct in the original version -
defer the minmax scan to the hardware driver, which may or may not need
it.
But maybe there is a better way to let the driver know that we are not
providing this information.
Keith
--
Download Intel#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.
Resending to the right address.
Keith
On Fri, 2010-03-12 at 13:40 +, Keith Whitwell wrote:
On Fri, 2010-03-12 at 02:54 -0800, Corbin Simpson wrote:
Module: Mesa
Branch: master
Commit: 50876ddaaff72a324ac45e255985e0f84e108594
URL:
http://cgit.freedesktop.org/mesa/mesa/commit/?id=50876ddaaff72a324ac45e255985e0f84e108594
Author: Corbin Simpson mostawesomed...@gmail.com
Date: Fri Mar 12 02:51:40 2010 -0800
st/mesa: Always recalculate invalid index bounds.
These should always be sanitized before heading towards the pipe driver,
and if the calling function explicitly marked them as invalid, we need
to regenerate them.
Allows r300g to properly pass a bit more of Wine's d3d9 testing without
dropping stuff on the floor.
---
src/mesa/state_tracker/st_draw.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/mesa/state_tracker/st_draw.c
b/src/mesa/state_tracker/st_draw.c
index 8b01272..d81b361 100644
--- a/src/mesa/state_tracker/st_draw.c
+++ b/src/mesa/state_tracker/st_draw.c
@@ -542,9 +542,9 @@ st_draw_vbo(GLcontext *ctx,
assert(ctx-NewState == 0x0);
/* Gallium probably doesn't want this in some cases. */
- if (!index_bounds_valid)
- if (!vbo_all_varyings_in_vbos(arrays))
-vbo_get_minmax_index(ctx, prims, ib, min_index, max_index);
+ if (index_bounds_valid != GL_TRUE) {
+ vbo_get_minmax_index(ctx, prims, ib, min_index, max_index);
+ }
Corbin,
This isn't really desirable. Ideally this range checking should be
pushed into pipe driver, because it's an expensive operation that is not
necessary on a lot of hardware. #
Specifically, vertex fetch hardware can often be set up with the min/max
permissible index bounds to avoid accessing vertex data outside of the
bound VB's. This can be achieved by examining the VBs, with min_index
== 0, max_index = vb.size / vb.stride.
The case where we need to calculate them internally is if some data is
not in a VB, meaning we can't guess what the legal min/max values are.
Also note that we need that min/max information to be able to upload the
data to a VB.
So, I think the code was probably correct in the original version -
defer the minmax scan to the hardware driver, which may or may not need
it.
But maybe there is a better way to let the driver know that we are not
providing this information.
Keith
--
Download Intel#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.
The overriding goal is to avoid the expense of computing this in
software when the hardware can do the bounds check.
I guess we could add a PIPE_CAP query to ask the driver if it can do
bounds checking and if it can't, do it in the state tracker. But I
think Keith's interested in minimizing queries like this. Hence, try
to do it in the driver. I think it should be easy to write a
re-usable utility function to do the checks.
If you're seeing ~0, that means the Mesa VBO module has not computed
the bounds, and/or the user didn't provide them. I'd have to check the
code.
If vertex data is coming from regular memory and not a VBO we have no
idea what the legal bounds are.
-Brian
On Fri, Mar 12, 2010 at 5:06 PM, Corbin Simpson
mostawesomed...@gmail.com wrote:
This seems to be at odds with the idea that the pipe driver receives
pre-sanitized inputs.
If this patch is reverted:
- I can't trust the min and max elts, because they're set to ~0
- I can't just use the vertex buffer sizes, because they're set to ~0
So I have to do the maths myself, guessing just like st/mesa guesses. Maybe
this is specific to Radeons, but I *always* need those values.
I don't mind this, but IMO it *must* be doc'd, The minimum and maximum
index limits passed to draw_elements and draw_range_elements may be
invalid, and really, at that point, why are we even passing them? Seems
absurd to me. :3
Posting from a mobile, pardon my terseness. ~ C.
On Mar 12, 2010 5:40 AM, Keith Whitwell kei...@vmware.com wrote:
On Fri, 2010-03-12 at 02:54 -0800, Corbin Simpson wrote:
Module: Mesa
Branch: master
Commit: 50876ddaaff72a324ac45e255985e0f84e108594
URL:
http://cgit.freedesktop.org/mesa/mesa/commit/?id=50876ddaaff72a324ac45e255985e0f84e108594
Author: Corbin Simpson mostawesomed...@gmail.com
Date: Fri Mar 12 02:51:40 2010 -0800
st/mesa: Always recalculate invalid index bounds.
These should always be sanitized before heading towards the pipe driver,
and if the calling function explicitly marked them as invalid, we need
to regenerate them.
Allows r300g to properly pass a bit more of Wine's d3d9 testing without
dropping stuff on the floor.
---
src/mesa/state_tracker/st_draw.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/mesa/state_tracker/st_draw.c
b/src/mesa/state_tracker/st_draw.c
index 8b01272..d81b361 100644
--- a/src/mesa/state_tracker/st_draw.c
+++ b/src/mesa/state_tracker/st_draw.c
@@ -542,9 +542,9 @@ st_draw_vbo(GLcontext *ctx,
assert(ctx-NewState == 0x0);
/* Gallium probably doesn't want this in some cases. */
- if (!index_bounds_valid)
- if (!vbo_all_varyings_in_vbos(arrays))
- vbo_get_minmax_index(ctx, prims, ib, min_index, max_index);
+ if (index_bounds_valid != GL_TRUE) {
+ vbo_get_minmax_index(ctx, prims, ib, min_index, max_index);
+ }
Corbin,
This isn't really desirable. Ideally this range checking should be
pushed into pipe driver, because it's an expensive operation that is not
necessary on a lot of hardware. #
Specifically, vertex fetch hardware can often be set up with the min/max
permissible index bounds to avoid accessing vertex data outside of the
bound VB's. This can be achieved by examining the VBs, with min_index
== 0, max_index = vb.size / vb.stride.
The case where we need to calculate them internally is if some data is
not in a VB, meaning we can't guess what the legal min/max values are.
Also note that we need that min/max information to be able to upload the
data to a VB.
So, I think the code was probably correct in the original version -
defer the minmax scan to the hardware driver, which may or may not need
it.
But maybe there is a better way to let the driver know that we are not
providing this information.
Keith
--
Download Intel#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
--
Download Intel#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.
Isn't it possible to compute the maximum legal index by just taking the minimum of: (vb-buffer-size - vb-buffer_offset - ve-src_offset) / vb-stride over all vertex buffers/elements? Isn't the kernel checker doing something like this? -- Download Intel#174; Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.
Actually, why is the state tracker doing the min/max computation at all? If the driver does the index lookup itself, as opposed to using an hardware index buffer, (e.g. the nouveau drivers do this in some cases) this is unnecessary and slow. Would completely removing the call to vbo_get_minmax_index break anything? Also, how about removing the max_index field in pipe_vertex_buffer? This seems to be set to the same value for all vertex buffers, and the value is then passed to draw_range_elements too. Isn't the value passed to draw_range_elements sufficient? -- Download Intel#174; Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
