* [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
@ 2015-01-07 18:52 Tobias Klausmann
2015-01-11 5:05 ` [Nouveau] " Ilia Mirkin
[not found] ` <1420656766-7342-1-git-send-email-tobias.johannes.klausmann-AqjdNwhu20eELgA04lAiVw@public.gmane.org>
0 siblings, 2 replies; 7+ messages in thread
From: Tobias Klausmann @ 2015-01-07 18:52 UTC (permalink / raw)
To: mesa-dev-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
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@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Nouveau] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
2015-01-07 18:52 [RFC] mesa/st: Avoid passing a NULL buffer to the drivers Tobias Klausmann
@ 2015-01-11 5:05 ` Ilia Mirkin
2015-01-12 0:43 ` Tobias Klausmann
[not found] ` <1420656766-7342-1-git-send-email-tobias.johannes.klausmann-AqjdNwhu20eELgA04lAiVw@public.gmane.org>
1 sibling, 1 reply; 7+ messages in thread
From: Ilia Mirkin @ 2015-01-11 5:05 UTC (permalink / raw)
To: Tobias Klausmann
Cc: mesa-dev@lists.freedesktop.org, nouveau@lists.freedesktop.org
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@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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Nouveau] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
2015-01-11 5:05 ` [Nouveau] " Ilia Mirkin
@ 2015-01-12 0:43 ` Tobias Klausmann
2015-01-12 0:57 ` Ilia Mirkin
0 siblings, 1 reply; 7+ messages in thread
From: Tobias Klausmann @ 2015-01-12 0:43 UTC (permalink / raw)
To: Ilia Mirkin; +Cc: mesa-dev@lists.freedesktop.org, nouveau@lists.freedesktop.org
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@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@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
Quoted 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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Nouveau] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
2015-01-12 0:43 ` Tobias Klausmann
@ 2015-01-12 0:57 ` Ilia Mirkin
2015-01-12 1:11 ` Tobias Klausmann
0 siblings, 1 reply; 7+ messages in thread
From: Ilia Mirkin @ 2015-01-12 0:57 UTC (permalink / raw)
To: Tobias Klausmann
Cc: mesa-dev@lists.freedesktop.org, nouveau@lists.freedesktop.org
On Sun, Jan 11, 2015 at 7:43 PM, Tobias Klausmann
<tobias.johannes.klausmann@mni.thm.de> wrote:
>
>
> 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@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@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
>
> Quoted 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!
The thing you're quoting is talking about the case where everything's
supposed to work. I haven't investigated, but I'm guessing that the
test has a layout(stream=1) but no buffer is bound at index 1. Is that
right? In that case, I would imagine that the TF output should
actually just get dropped on the floor. I would assume that this is in
the ARB_tf3 spec, but I don't have time to go digging right now.
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Nouveau] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
2015-01-12 0:57 ` Ilia Mirkin
@ 2015-01-12 1:11 ` Tobias Klausmann
[not found] ` <54B31F25.3070104-AqjdNwhu20eELgA04lAiVw@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Tobias Klausmann @ 2015-01-12 1:11 UTC (permalink / raw)
To: Ilia Mirkin; +Cc: mesa-dev@lists.freedesktop.org, nouveau@lists.freedesktop.org
On 12.01.2015 01:57, Ilia Mirkin wrote:
> On Sun, Jan 11, 2015 at 7:43 PM, Tobias Klausmann
> <tobias.johannes.klausmann@mni.thm.de> wrote:
>>
>> 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@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@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
>> Quoted 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!
> The thing you're quoting is talking about the case where everything's
> supposed to work. I haven't investigated, but I'm guessing that the
> test has a layout(stream=1) but no buffer is bound at index 1.
Actually no, the layout reads like this:
layout(stream = 0) out float stream0_0_out;
layout(stream = 1) out vec2 stream1_0_out;
layout(stream = 2) out float stream2_0_out;
layout(stream = 2) out vec4 stream2_1_out;
where no buffer is bound to stream2_1_out.
> Is that right? In that case, I would imagine that the TF output should
> actually just get dropped on the floor. I would assume that this is in
> the ARB_tf3 spec, but I don't have time to go digging right now.
As the layout read more like the one from the specs, i'd still go with
[n-1] and honestly (not proven correct) i assume that a stream without
an output is just wrong and should not even get through the linker...
Tobias
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
[not found] ` <54B31F25.3070104-AqjdNwhu20eELgA04lAiVw@public.gmane.org>
@ 2015-01-12 1:17 ` Ilia Mirkin
0 siblings, 0 replies; 7+ messages in thread
From: Ilia Mirkin @ 2015-01-12 1:17 UTC (permalink / raw)
To: Tobias Klausmann
Cc: mesa-dev-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
On Sun, Jan 11, 2015 at 8:11 PM, Tobias Klausmann
<tobias.johannes.klausmann@mni.thm.de> wrote:
>
>
> On 12.01.2015 01:57, Ilia Mirkin wrote:
>>
>> On Sun, Jan 11, 2015 at 7:43 PM, Tobias Klausmann
>> <tobias.johannes.klausmann@mni.thm.de> wrote:
>>>
>>>
>>> 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@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@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
>>>
>>> Quoted 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!
>>
>> The thing you're quoting is talking about the case where everything's
>> supposed to work. I haven't investigated, but I'm guessing that the
>> test has a layout(stream=1) but no buffer is bound at index 1.
>
>
> Actually no, the layout reads like this:
> layout(stream = 0) out float stream0_0_out;
> layout(stream = 1) out vec2 stream1_0_out;
> layout(stream = 2) out float stream2_0_out;
> layout(stream = 2) out vec4 stream2_1_out;
> where no buffer is bound to stream2_1_out.
>
>> Is that right? In that case, I would imagine that the TF output should
>> actually just get dropped on the floor. I would assume that this is in
>> the ARB_tf3 spec, but I don't have time to go digging right now.
>
>
> As the layout read more like the one from the specs, i'd still go with [n-1]
> and honestly (not proven correct) i assume that a stream without an output
> is just wrong and should not even get through the linker...
Well, linking has nothing to do with current bindings; it has to do
with connecting inter-stage inputs/outputs... Anyways, I'm moderately
sure that the thing you're currently doing has no justification. Try
to find what the proper way of dealing with this is (I assume i965
deals with it, see what they do), or ask around. If only no buffer is
bound to stream2_1_out, I assume that it's doing the "separate"
outputs? That probably complicates matters...
-ilia
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Mesa-dev] [RFC] mesa/st: Avoid passing a NULL buffer to the drivers
[not found] ` <1420656766-7342-1-git-send-email-tobias.johannes.klausmann-AqjdNwhu20eELgA04lAiVw@public.gmane.org>
@ 2015-01-12 12:14 ` Marek Olšák
0 siblings, 0 replies; 7+ messages in thread
From: Marek Olšák @ 2015-01-12 12:14 UTC (permalink / raw)
To: Tobias Klausmann
Cc: mesa-dev-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
nouveau
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@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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-01-12 12:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-07 18:52 [RFC] mesa/st: Avoid passing a NULL buffer to the drivers Tobias Klausmann
2015-01-11 5:05 ` [Nouveau] " Ilia Mirkin
2015-01-12 0:43 ` Tobias Klausmann
2015-01-12 0:57 ` Ilia Mirkin
2015-01-12 1:11 ` Tobias Klausmann
[not found] ` <54B31F25.3070104-AqjdNwhu20eELgA04lAiVw@public.gmane.org>
2015-01-12 1:17 ` Ilia Mirkin
[not found] ` <1420656766-7342-1-git-send-email-tobias.johannes.klausmann-AqjdNwhu20eELgA04lAiVw@public.gmane.org>
2015-01-12 12:14 ` [Mesa-dev] " Marek Olšák
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.