Tobias Klausmann
2015-Jan-07 18:52 UTC
[Nouveau] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
If we capture transform feedback from n stream in (n-1) buffers we face a
NULL buffer, use the buffer (n-1) to capture the output of stream n.
This fixes one piglit test with nvc0:
arb_gpu_shader5-xfb-streams-without-invocations
Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de>
---
src/mesa/state_tracker/st_cb_xformfb.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/mesa/state_tracker/st_cb_xformfb.c
b/src/mesa/state_tracker/st_cb_xformfb.c
index 8f75eda..5a12da4 100644
--- a/src/mesa/state_tracker/st_cb_xformfb.c
+++ b/src/mesa/state_tracker/st_cb_xformfb.c
@@ -123,6 +123,11 @@ st_begin_transform_feedback(struct gl_context *ctx, GLenum
mode,
struct st_buffer_object *bo = st_buffer_object(sobj->base.Buffers[i]);
if (bo) {
+ if (!bo->buffer)
+ /* If we capture transform feedback from n streams into (n-1)
+ * buffers we have to write to buffer (n-1) for stream n.
+ */
+ bo = st_buffer_object(sobj->base.Buffers[i-1]);
/* Check whether we need to recreate the target. */
if (!sobj->targets[i] ||
sobj->targets[i] == sobj->draw_count ||
--
2.2.1
Ilia Mirkin
2015-Jan-11 05:05 UTC
[Nouveau] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
Can you elaborate a bit as to why that's the right thing to do? On Wed, Jan 7, 2015 at 1:52 PM, Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de> wrote:> If we capture transform feedback from n stream in (n-1) buffers we face a > NULL buffer, use the buffer (n-1) to capture the output of stream n. > > This fixes one piglit test with nvc0: > arb_gpu_shader5-xfb-streams-without-invocations > > Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de> > --- > src/mesa/state_tracker/st_cb_xformfb.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/src/mesa/state_tracker/st_cb_xformfb.c b/src/mesa/state_tracker/st_cb_xformfb.c > index 8f75eda..5a12da4 100644 > --- a/src/mesa/state_tracker/st_cb_xformfb.c > +++ b/src/mesa/state_tracker/st_cb_xformfb.c > @@ -123,6 +123,11 @@ st_begin_transform_feedback(struct gl_context *ctx, GLenum mode, > struct st_buffer_object *bo = st_buffer_object(sobj->base.Buffers[i]); > > if (bo) { > + if (!bo->buffer) > + /* If we capture transform feedback from n streams into (n-1) > + * buffers we have to write to buffer (n-1) for stream n. > + */ > + bo = st_buffer_object(sobj->base.Buffers[i-1]); > /* Check whether we need to recreate the target. */ > if (!sobj->targets[i] || > sobj->targets[i] == sobj->draw_count || > -- > 2.2.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/nouveau
Tobias Klausmann
2015-Jan-12 00:43 UTC
[Nouveau] Re: [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
On 11.01.2015 06:05, Ilia Mirkin wrote:> Can you elaborate a bit as to why that's the right thing to do? > > On Wed, Jan 7, 2015 at 1:52 PM, Tobias Klausmann > <tobias.johannes.klausmann at mni.thm.de> wrote: >> If we capture transform feedback from n stream in (n-1) buffers we face a >> NULL buffer, use the buffer (n-1) to capture the output of stream n. >> >> This fixes one piglit test with nvc0: >> arb_gpu_shader5-xfb-streams-without-invocations >> >> Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de> >> --- >> src/mesa/state_tracker/st_cb_xformfb.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/src/mesa/state_tracker/st_cb_xformfb.c b/src/mesa/state_tracker/st_cb_xformfb.c >> index 8f75eda..5a12da4 100644 >> --- a/src/mesa/state_tracker/st_cb_xformfb.c >> +++ b/src/mesa/state_tracker/st_cb_xformfb.c >> @@ -123,6 +123,11 @@ st_begin_transform_feedback(struct gl_context *ctx, GLenum mode, >> struct st_buffer_object *bo = st_buffer_object(sobj->base.Buffers[i]); >> >> if (bo) { >> + if (!bo->buffer) >> + /* If we capture transform feedback from n streams into (n-1) >> + * buffers we have to write to buffer (n-1) for stream n. >> + */ >> + bo = st_buffer_object(sobj->base.Buffers[i-1]); >> /* Check whether we need to recreate the target. */ >> if (!sobj->targets[i] || >> sobj->targets[i] == sobj->draw_count || >> -- >> 2.2.1Quoted from Ilia Mirkin, to specify what shall be elaborated: "Can you explain (on-list) why using buffer n - 1 is the right thing to do to capture output of stream n? I would have thought that the output for that stream should be discarded or something. Like with a spec quotation or some other justification. i.e. why is the code you wrote correct? Why is it better than, say, bo buffers[0], or some other thing entirely?" Yeah thats the most concerning point i see as well. The problem is that there is a interaction between arb_gpu_shader5 and arb_transform_feedback3, but after a bit of reading i think the patch is actually what we should do: From the arb_transfrom_feedback3 spec: " (3) How might you use transform feedback with geometry shaders and multiple vertex streams? RESOLVED: As a simple example, let's say you are processing triangles and capture both processed triangle vertices and some values that are computed per-primitive (e.g., facet normal). The geometry shader might declare its outputs like the following: layout(stream = 0) out vec4 position; layout(stream = 0) out vec4 texcoord; layout(stream = 1) out vec4 normal; "position" and "texcoord" would be per-vertex attributes written to vertex stream 0; "normal" would be a per-triangle facet normal. The geometry shader would emit three vertices to stream zero (the processed input vertices) and a single vertex to stream one (the per-triangle data). The transform feedback API usage for this case would be something like: // Set up buffer objects 21 and 22 to capture data for per-vertex and // per primitive values. glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, 21); glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 1, 22); // Set up XFB to capture position and texcoord to buffer binding // point 0 (buffer 21 bound), and normal to binding point 1 (buffer // 22 bound). char *strings[] = { "position", "texcoord", "gl_NextBuffer", "normal" }; " -> Especially the comments are enlightening as to where the outputs should go. Thats what happens with the "arb_gpu_shader5-xfb-streams-without-invocations" test, where two stream(outputs) are captured into one buffer. One might argue now if we have to count .Buffers[i-1] for all buffers after this... Comments and additional feedback is always appreciated! Greetings, Tobias
Marek Olšák
2015-Jan-12 12:14 UTC
[Nouveau] [Mesa-dev] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
NAK. NULL buffers are allowed. All transform feedback writes to NULL buffers should be discarded. Marek On Wed, Jan 7, 2015 at 7:52 PM, Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de> wrote:> If we capture transform feedback from n stream in (n-1) buffers we face a > NULL buffer, use the buffer (n-1) to capture the output of stream n. > > This fixes one piglit test with nvc0: > arb_gpu_shader5-xfb-streams-without-invocations > > Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de> > --- > src/mesa/state_tracker/st_cb_xformfb.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/src/mesa/state_tracker/st_cb_xformfb.c b/src/mesa/state_tracker/st_cb_xformfb.c > index 8f75eda..5a12da4 100644 > --- a/src/mesa/state_tracker/st_cb_xformfb.c > +++ b/src/mesa/state_tracker/st_cb_xformfb.c > @@ -123,6 +123,11 @@ st_begin_transform_feedback(struct gl_context *ctx, GLenum mode, > struct st_buffer_object *bo = st_buffer_object(sobj->base.Buffers[i]); > > if (bo) { > + if (!bo->buffer) > + /* If we capture transform feedback from n streams into (n-1) > + * buffers we have to write to buffer (n-1) for stream n. > + */ > + bo = st_buffer_object(sobj->base.Buffers[i-1]); > /* Check whether we need to recreate the target. */ > if (!sobj->targets[i] || > sobj->targets[i] == sobj->draw_count || > -- > 2.2.1 > > _______________________________________________ > mesa-dev mailing list > mesa-dev at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Possibly Parallel Threads
- Re: [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
- Re: [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
- [PATCH 1/2] st/mesa: treat resource-less xfb buffers as if they weren't there
- [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
- [RFC] mesa/st: Avoid passing a NULL buffer to the drivers