thr3ads.net - Nouveau - [Nouveau] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers [Jan 2015]

If this information is useful, please help other people find it:
Share via:

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

head link

[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

head link

[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

head link

[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

Reasonably Related Threads

Search for more possibly parallel threads

Nouveau - Jan 2015 - [RFC] mesa/st: Avoid passing a NULL buffer to the drivers

[Nouveau] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers

[Nouveau] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers

[Nouveau] Re: [RFC] mesa/st: Avoid passing a NULL buffer to the drivers

[Nouveau] [Mesa-dev] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers

Reasonably Related Threads