<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
On 08/11/18 22:42, Jason Ekstrand wrote:<br>
<blockquote type="cite"
cite="mid:CAOFGe94utx24EB1Xv_ddfkQN1URhG-K8H+VRQDE9zjkRiC=NKQ@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr">On Thu, Nov 8, 2018 at 7:22 AM Alejandro
Piñeiro <<a href="mailto:apinheiro@igalia.com"
moz-do-not-send="true">apinheiro@igalia.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">In order
to allow nir_gather_xfb_info to be used on OpenGL,<br>
specifically ARB_gl_spirv.<br>
<br>
So, from OpenGL 4.6 spec, section 11.1.2.1, "Output
Variables":<br>
<br>
"outputs specifying both an *XfbBuffer* and an *Offset*
are<br>
captured, while outputs not specifying both of these
are not<br>
captured. Values are captured each time the shader
writes to such<br>
a decorated object."<br>
<br>
This implies that are captured if both are present, and not
if one of<br>
those are lacking. Technically, it doesn't explicitly point
that<br>
having just one or the other is a mistake. In some cases,
glslang is<br>
adding some extra XfbBuffer without XfbOffset around, and
mentioning<br>
that technically that is not a bug (see issue#1526)<br>
<br>
And for the case of Vulkan, as the same glslang issue
mentions, it is<br>
not clear if that should be a mistake or not. But even if it
is a<br>
mistake, it is not really needed to be checked on the
driver, and we<br>
can let the validation layers to check that.<br>
---<br>
src/compiler/nir/nir_gather_xfb_info.c | 23
++++++++++++++++++++---<br>
1 file changed, 20 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/src/compiler/nir/nir_gather_xfb_info.c
b/src/compiler/nir/nir_gather_xfb_info.c<br>
index e282bba0081..f5d831c6567 100644<br>
--- a/src/compiler/nir/nir_gather_xfb_info.c<br>
+++ b/src/compiler/nir/nir_gather_xfb_info.c<br>
@@ -109,9 +109,16 @@ nir_gather_xfb_info(const nir_shader
*shader, void *mem_ctx)<br>
nir_foreach_variable(var, &shader->outputs) {<br>
if (var->data.explicit_xfb_buffer ||<br>
var->data.explicit_xfb_stride) {<br>
- assert(var->data.explicit_xfb_buffer &&<br>
- var->data.explicit_xfb_stride &&<br>
- var->data.explicit_offset);<br>
+<br>
+ /* OpenGL points that both are needed to capture
the output, but<br>
+ * doesn't direcly imply that it is a mistake
having one but not the<br>
+ * other.<br>
+ */<br>
+ if (!var->data.explicit_xfb_buffer ||<br>
+ !var->data.explicit_offset) {<br>
</blockquote>
<div><br>
</div>
<div>Why not just change the check above to
"var->data.explicit_xfb_buffer &&
var->data.explicit_offset" and not bother with two
checks?<br>
</div>
</div>
</div>
</blockquote>
<br>
True. Change done locally.<br>
<br>
<blockquote type="cite"
cite="mid:CAOFGe94utx24EB1Xv_ddfkQN1URhG-K8H+VRQDE9zjkRiC=NKQ@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
+ continue;<br>
+ }<br>
+<br>
num_outputs +=
glsl_count_attribute_slots(var->type, false);<br>
}<br>
}<br>
@@ -124,6 +131,16 @@ nir_gather_xfb_info(const nir_shader
*shader, void *mem_ctx)<br>
nir_foreach_variable(var, &shader->outputs) {<br>
if (var->data.explicit_xfb_buffer ||<br>
var->data.explicit_xfb_stride) {<br>
+<br>
+ /* OpenGL points that both are needed to capture
the output, but<br>
+ * doesn't direcly imply that it is a mistake
having one but not the<br>
+ * other.<br>
+ */<br>
+ if (!var->data.explicit_xfb_buffer ||<br>
+ !var->data.explicit_offset) {<br>
</blockquote>
<div><br>
</div>
<div>Same here.<br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
+ continue;<br>
+ }<br>
+<br>
unsigned location = var->data.location;<br>
unsigned offset = var->data.offset;<br>
add_var_xfb_outputs(xfb, var, &location,
&offset, var->type);<br>
-- <br>
2.14.1<br>
<br>
</blockquote>
</div>
</div>
</blockquote>
<br>
</body>
</html>