| Summary: | SSO: wrong interface validation between GS and VS (regresion due to latest gles 3.1) | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | gregory.hainaut |
| Component: | Mesa core | Assignee: | Ian Romanick <idr> |
| Status: | RESOLVED FIXED | QA Contact: | mesa-dev |
| Severity: | normal | ||
| Priority: | medium | CC: | idr |
| Version: | git | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: | Strip arrayness from interface block names in some IO validation | ||
Note I just took the commit from a git blame. It seems the code replace an older function. But I don't know if the older function was broken too. (In reply to gregory.hainaut from comment #1) > Note I just took the commit from a git blame. It seems the code replace an > older function. But I don't know if the older function was broken too. The previous code was pretty busted. However it was never run on desktop GL, while with the new logic, it's run on desktop GL for debug contexts. However it hasn't had a lot of testing on desktop GL, only the dEQP (ES) tests. I'm surprised that nothing in GLES 3.2 CTS or dEQP caught this. The problem is that in this variable-in-an-interface case the arrayness isn't properly checked. That should be easy enough to fix for this release. In the longer term, I think a bunch of the program_interface_query code should just be rewritten. Created attachment 124453 [details] [review] Strip arrayness from interface block names in some IO validation This patch is a bit of a hack, but I think it should fix the problem. If it works, I'll improve it & send to the list for review. If this doesn't fix the problem, I think we should just remove IO validation from desktop GL altogether. diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c index 5a46cfe..10a9544 100644 --- a/src/mesa/main/pipelineobj.c +++ b/src/mesa/main/pipelineobj.c @@ -928,8 +928,7 @@ _mesa_validate_program_pipeline(struct gl_context* ctx, * Based on this, only perform the most-strict checking on ES or when the * application has created a debug context. */ - if ((_mesa_is_gles(ctx) || (ctx->Const.ContextFlags & GL_CONTEXT_FLAG_DEBUG_BIT)) && - !_mesa_validate_pipeline_io(pipe)) + if (_mesa_is_gles(ctx) && !_mesa_validate_pipeline_io(pipe)) return GL_FALSE; pipe->Validated = GL_TRUE; I think there is a typo.
(gdb) p consumer_name
$1 = 0x8ae8b14 "SHADER[2].c"
(gdb) p consumer_name
$2 = 0x2bdf75b8 "SHADERc"
(gdb) p *outputs[0]
$4 = {
type = 0xf4c0e5e0 <glsl_type::_vec4_type>,
interface_type = 0x8ae871c,
outermost_struct_type = 0x0,
name = 0x8a84e9c "SHADER.c",
location = -1,
component = 0,
index = 0,
patch = 0,
mode = 5,
interpolation = 0,
explicit_location = 0,
precision = 0
}
Using "strcpy(s, t + 1)" is better. But another check is failling below.
if (producer_var->type != consumer_var->type ||
producer_var->interpolation != consumer_var->interpolation ||
producer_var->precision != consumer_var->precision) {
valid = false;
goto out;
}
Here some debug info. So you can't compare type pointer due to arrayness. And I don't know what interpolation is different it ought to be the same. Likely Mesa set the FLAT status to some IO.
(gdb) p *producer_var
$18 = {
type = 0xf4c0d5e0 <glsl_type::_vec4_type>,
interface_type = 0x8b08d38,
outermost_struct_type = 0x0,
name = 0x8b02028 "SHADER.c",
location = -1,
component = 0,
index = 0,
patch = 0,
mode = 5,
interpolation = 0,
explicit_location = 0,
precision = 0
}
(gdb) p *consumer_var
$19 = {
type = 0x8ac14e8,
interface_type = 0x8b0fb80,
outermost_struct_type = 0x0,
name = 0x8ad36a0 "SHADER[2].c",
location = -1,
component = 0,
index = 0,
patch = 0,
mode = 4,
interpolation = 2,
explicit_location = 0,
precision = 0
}
(gdb) p *consumer_var->type
$25 = {
gl_type = 35666,
base_type = GLSL_TYPE_ARRAY,
sampler_dimensionality = 0,
sampler_shadow = 0,
sampler_array = 0,
sampled_type = 0,
interface_packing = 0,
vector_elements = 0 '\000',
matrix_columns = 0 '\000',
length = 2,
name = 0x8ac1520 "vec4[2]",
fields = {
array = 0xf4c0d5e0 <glsl_type::_vec4_type>,
parameters = 0xf4c0d5e0 <glsl_type::_vec4_type>,
structure = 0xf4c0d5e0 <glsl_type::_vec4_type>
},
(gdb) p *producer_var->type
$27 = {
gl_type = 35666,
base_type = GLSL_TYPE_FLOAT,
sampler_dimensionality = 0,
sampler_shadow = 0,
sampler_array = 0,
sampled_type = 0,
interface_packing = 0,
vector_elements = 4 '\004',
matrix_columns = 1 '\001',
length = 0,
name = 0x87932c8 "vec4",
fields = {
array = 0x0,
parameters = 0x0,
structure = 0x0
},
Forgot to say that interface_type won't match neither again due to arrayness. I did a quick of latest Ian's patches. They are working fine for me. Bug could be closed once there are merged. Hi Gregory, I have a problem with building mesa with: https://cgit.freedesktop.org/mesa/mesa/commit/?id=6a524c76f502fe15bb3612065a23ece693aed237 Is that commit related to this bug? If not just tell me then I open new raport. mesa building time error: main/uniform_query.cpp: In function 'bool _mesa_sampler_uniforms_pipeline_are_valid(gl_pipeline_object*)': main/uniform_query.cpp:1086:47: error: cannot convert 'gl_linked_shader* const' to 'gl_shader*' in assignment shader = shProg[idx]->_LinkedShaders[idx]; ^ main/uniform_query.cpp:1087:31: error: 'struct gl_shader' has no member named 'Program' if (!shader || !shader->Program) ^~~~~~~ main/uniform_query.cpp:1090:22: error: 'struct gl_shader' has no member named 'Program' mask = shader->Program->SamplersUsed; ^~~~~~~ main/uniform_query.cpp:1093:32: error: 'struct gl_shader' has no member named 'SamplerUnits' GLuint unit = shader->SamplerUnits[s]; ^~~~~~~~~~~~ main/uniform_query.cpp:1094:31: error: 'struct gl_shader' has no member named 'SamplerTargets' GLuint tgt = shader->SamplerTargets[s]; ^~~~~~~~~~~~~~ main/uniform_query.cpp:1115:34: error: 'struct gl_shader' has no member named 'num_samplers' active_samplers += shader->num_samplers; ^~~~~~~~~~~~ make[5]: *** [Makefile:2971: main/uniform_query.lo] Error 1 (In reply to Arek Ruśniak from comment #9) > Hi Gregory, I have a problem with building mesa with: > https://cgit.freedesktop.org/mesa/mesa/commit/ > ?id=6a524c76f502fe15bb3612065a23ece693aed237 > > Is that commit related to this bug? If not just tell me then I open new > raport. > Sorry this was my bad. I'll push a fix in a moment. Np, thx for quick fix. commit 73a6a4ce4975016d4f86d644b31d30bb6d3a38f8 mesa: Strip arrayness from interface block names in some IO validation Outputs from the vertex shader need to be able to match per-vertex-arrayed inputs of later stages. Acomplish this by stripping one level of arrayness from the names and types of outputs going to a per-vertex-arrayed stage. v2: Add missing checks for TESS_EVAL->GEOMETRY. Noticed by Timothy Arceri. v3: Use a slightly simpler stage check suggested by Ilia. Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96358 Reviewed-by: Kenneth Graunke <kenneth@whitecape.org> Cc: "12.0" <mesa-stable@lists.freedesktop.org> Cc: Gregory Hainaut <gregory.hainaut@gmail.com> Cc: Ilia Mirkin <imirkin@alum.mit.edu> |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
Hello, My program generates pipeline invalidation error but it used to be fine. It is related to bd3f15cffdbbec6d1ea5b7db2fcddaf8b7ae4524. Here the Vertex Shader output out SHADER { vec4 t_float; vec4 t_int; vec4 c; flat vec4 fc; } VSout; Here the Geometry Shader input. Please notice the array. in SHADER { vec4 t_float; vec4 t_int; vec4 c; flat vec4 fc; } GSin[]; In src/mesa/main/shader_query.cpp in validate_io function: The error detected is this one: /* Section 7.4.1 (Shader Interface Matching) of the OpenGL ES 3.1 spec * says: * * - 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. */ if (producer_var == NULL) { valid = false; goto out; } Here the consumer_var info (gdb) p *consumer_var $17 = { type = 0x8acc940, interface_type = 0x8b39b50, outermost_struct_type = 0x0, name = 0x8ae8e68 "SHADER[2].c", location = -1, component = 0, index = 0, patch = 0, mode = 4, interpolation = 2, explicit_location = 0, precision = 0 } And the first output gdb) p *outputs[0 $24 = { type = 0xf4b9f4f0 <glsl_type::_vec4_type>, interface_type = 0x8b382e8, outermost_struct_type = 0x0, name = 0x8b27b80 "SHADER.c", location = -1, component = 0, index = 0, patch = 0, mode = 5, interpolation = 0, explicit_location = 0, precision = 0 } As you can the name of the GS interface got an extra "[2]" likely the size of the array. So the previous strcmp isn't possible <<<< if (!var->explicit_location && strcmp(consumer_var->name, var->name) == 0) { >>>> Good luck :)