On 27/06/17 21:20, Juan A. Suarez Romero wrote:
On Tue, 2017-06-27 at 09:29 +1000, Timothy Arceri wrote:
On 16/06/17 18:12, Juan A. Suarez Romero wrote:
Commit 00620782c9 (i965: use nir_shader_gather_info() over
do_set_program_inouts()) changed how we compute the outputs written.
In the previous version it was using the IR declared outputs, while in
the new one it uses NIR to parse the instructions that write outputs.
Thus, if the shader has declared some output that is not written later
in the code, like this:
~~~
struct S {
vec4 a;
vec4 b;
vec4 c;
};
layout (xfb_offset = sizeof_type) out S s;
void main()
{
s.a = vec4(1.0, 0.0, 0.0, 1.0);
s.c = vec4(0.0, 1.0, 0.0, 1.0);
}
~~~
The former version computing 3 outputs written (s.a, s.b and s.c), while
the new version only counts 2 (s.a and s.c).
This means that with the new version, then could be varyings in the VUE
map that do not have an slot assigned (s.b), that must be skipped.
This fixes KHR-GL45.enhanced_layouts.xfb_capture_struct.
---
src/mesa/drivers/dri/i965/genX_state_upload.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
b/src/mesa/drivers/dri/i965/genX_state_upload.c
index a5ad2ca..573f0e3 100644
--- a/src/mesa/drivers/dri/i965/genX_state_upload.c
+++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
@@ -3102,9 +3102,10 @@ genX(upload_3dstate_so_decl_list)(struct brw_context
*brw,
const unsigned stream_id = output->StreamId;
assert(stream_id < MAX_VERTEX_STREAMS);
- buffer_mask[stream_id] |= 1 << buffer;
+ if (vue_map->varying_to_slot[varying] == -1)
+ continue;
- assert(vue_map->varying_to_slot[varying] >= 0);
+ buffer_mask[stream_id] |= 1 << buffer;
My feeling is we should try to avoid adding it to the VUE map in the
first place rather than trying to work around it.
It isn't in the VUE map. That's the reason to skip it.
Maybe you mean not adding it in the linked_xfb_info?
oh, right. I had it the wrong way around in my head.
I think the problem is we setup xfb in the glsl linker but then run all
the NIR optimisation before calling nir_shader_gather_info().
However I'm not sure removing the assert is the best idea, as it could
result in real issues being hidden.
Ideally we would run the NIR opts before we do the final linking in GLSL
IR. I've outlined how this can be done in past emails (which I can't
seem to find), but its a lot of work. Nicolai's spirv might make is
easier to do, but there will still be things like a nir varying packing
pass required which I believe will be outside of what Nicolai needs for
his changes.
For now I believe this issue only impacts debug builds so I'm not sure
removing the assert and silently skipping is a good idea.
I'll let others comment further.
J.A.
Is it not possible to do that instead?
/* Mesa doesn't store entries for gl_SkipComponents in the Outputs[]
* array. Instead, it simply increments DstOffset for the following
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
