On Wed, 2015-12-16 at 08:24 +0200, Tapani Pälli wrote:
Patch makes following changes for interface matching:
- do not try to match builtin variables
- handle swizzle in input name, as example 'a.z' should
match with 'a'
- add matching by location
- check that amount of inputs and outputs matches
These changes make interface matching tests to work in:
ES31-CTS.sepshaderobjs.StateInteraction
Test does not still pass completely due to errors in rendering
Test does not still pass -> The test still does not pass
output. IMO this is unrelated to interface matching.
Note that type matching is not done due to varying packing which
changes type of variable, this can be added later on. Preferably
when we have quicker way to iterate resources and have a complete
list of all existed varyings (before packing) available.
v2: add spec reference, return true on desktop since we do not
have failing cases for it, inputs and outputs amount do not
need to match on desktop.
v3: add some more spec reference, remove desktop specifics since
not used for now on desktop, add match by location qualifier,
rename input_stage and output_stage as producer and consumer
as suggested by Timothy.
Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
---
src/mesa/main/shader_query.cpp | 110
+++++++++++++++++++++++++++++++
----------
1 file changed, 83 insertions(+), 27 deletions(-)
diff --git a/src/mesa/main/shader_query.cpp
b/src/mesa/main/shader_query.cpp
index ced10a9..ea2b2f4 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -1373,46 +1373,102 @@ _mesa_get_program_resourceiv(struct
gl_shader_program *shProg,
}
static bool
-validate_io(const struct gl_shader *input_stage,
- const struct gl_shader *output_stage, bool isES)
+validate_io(const struct gl_shader *producer,
+ const struct gl_shader *consumer, bool isES)
{
- assert(input_stage && output_stage);
+ assert(producer && consumer);
+ unsigned inputs = 0, outputs = 0;
+
+ /* From OpenGL ES 3.1 spec (Interface matching):
+ *
+ * "An output variable is considered to match an input
variable in the
+ * subsequent shader if:
+ *
+ * - the two variables match in name, type, and
qualification;
or
+ * - the two variables are declared with the same location
qualifier and
+ * match in type and qualification.
+ *
+ * ...
+ *
+ * At an interface between program objects, the set of
inputs
and outputs
+ * are considered to match exactly if and only if:
+ *
+ * - Every declared input variable has a matching output, as
described
+ * above.
+ *
+ * - There are no user-defined output variables declared
without a
+ * matching input variable declaration.
+ *
+ * - All matched input and output variables have identical
precision
+ * qualification.
+ *
+ * When the set of inputs and outputs on an interface
between
programs
+ * matches exactly, all inputs are well-defined except when
the
+ * corresponding outputs were not written in the previous
shader. However,
+ * any mismatch between inputs and outputs will result in a
validation
+ * failure."
+ *
+ * OpenGL Core 4.5 spec includes same paragraph as above but
without check
+ * for precision and the last 'validation failure' clause.
Therefore
+ * behaviour is more relaxed, input and output amount is not
required by the
+ * spec to be validated.
+ *
+ * FIXME: Update once Khronos spec bug #15331 is resolved.
+ * FIXME: Add validation by type, currently information loss
during varying
+ * packing makes this challenging.
+ */
+
+ /* Currently no matching done for desktop. */
+ if (!isES)
+ return true;
/* For each output in a, find input in b and do any required
checks. */
- foreach_in_list(ir_instruction, out, input_stage->ir) {
+ foreach_in_list(ir_instruction, out, producer->ir) {
ir_variable *out_var = out->as_variable();
- if (!out_var || out_var->data.mode != ir_var_shader_out)
+ if (!out_var || out_var->data.mode != ir_var_shader_out ||
+ is_gl_identifier(out_var->name))
Another way to do this would be:
out_var->data.location < VARYING_SLOT_VAR0
Rather than:
is_gl_identifier(out_var->name)
If you make this change don't worry about sending out a new revision.
continue;
- foreach_in_list(ir_instruction, in, output_stage->ir) {
+ outputs++;
+
+ inputs = 0;
+ foreach_in_list(ir_instruction, in, consumer->ir) {
ir_variable *in_var = in->as_variable();
- if (!in_var || in_var->data.mode != ir_var_shader_in)
+ if (!in_var || in_var->data.mode != ir_var_shader_in ||
+ is_gl_identifier(in_var->name))
Same as above here.
continue;
- if (strcmp(in_var->name, out_var->name) == 0) {
- /* Since we now only validate precision, we can skip
this step for
- * desktop GLSL shaders, there precision qualifier is
ignored.
- *
- * From OpenGL 4.50 Shading Language spec, section
4.7:
- * "For the purposes of determining if an output
from one
- * shader stage matches an input of the next
stage,
the
- * precision qualifier need not match."
+ inputs++;
+
+ /* Match by location qualifier and precision. */
+ if ((in_var->data.explicit_location &&
+ out_var->data.explicit_location) &&
+ in_var->data.location == out_var->data.location &&
+ in_var->data.precision == out_var->data.precision)
+ continue;
The ES SL spec doesn't seem to say anything about what happens if one
stage has and explicit location and the other doesn't but falling
through and matching a varying with an explicit location by name
seems
like a bad idea.
This seems like an oversight in the ES spec, maybe bug report worthy?
For exaple you could end up with something like this being declared
valid.
[vertex shader]
layout(location = 7) out vec3 a;
out vec3 b;
out vec3 c;
[fragment shader]
out vec3 a;
layout(location = 5) out vec3 b;
out vec3 c;
The desktop spec has this line:
"For the purposes of determining if a non-vertex input matches an
output from a previous shader stage, the location layout qualifier
(if
any) must match."
We fail linking if the explicit location does not have a matching
explicit location.
For the sake of fixing the name matching tests, if you change the
above
to something like:
/* FIXME: Add explicit location matching validation here. Be careful
not to match varyings with explicit locations to varyings without
explicit locations. */
if (in_var->data.explicit_location)
continue;
