Brian found a bug with my "inline built-ins immediately" code for shaders which use ftransform() and declare gl_Position invariant:
https://lists.freedesktop.org/archives/mesa-dev/2016-October/132452.html Before my patch, things worked due to a specific order of operations: 1. link_intrastage_varyings imported the ftransform function into the VS 2. cross_validate_uniforms() ran and signed off that everything matched 3. do_common_optimization did both inlining and invariance propagation, making the VS/FS versions of gl_ModelViewProjectionMatrix have different invariant qualifiers...but after the check in step 2, so we never raised an error. After my patch, ftransform() is inlined right away, and at compile time, do_common_optimization propagates the invariant qualifier to the gl_ModelViewProjectionMatrix. When the linker eventually happens, it detects the mismatch. I can't see any good reason to raise a linker error based on qualifiers we internally applied to built-in variables - it's not the application's fault. It's either not a problem, or it's our fault.o We should probably rework invariance, but this should keep us limping along for now. It's definitely a hack. Signed-off-by: Kenneth Graunke <[email protected]> --- src/compiler/glsl/linker.cpp | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) Hi Brian, I'm on vacation today through Friday, so I likely won't be able to push this until next week. If people are okay with my hack, feel free to push it before I get back :) diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index 8599590..66f9e76 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -1038,12 +1038,28 @@ cross_validate_globals(struct gl_shader_program *prog, } } - if (existing->data.invariant != var->data.invariant) { - linker_error(prog, "declarations for %s `%s' have " - "mismatching invariant qualifiers\n", - mode_string(var), var->name); - return; + /* Skip invariant/precise checks for built-in uniforms. + * If they're used in an invariant calculation, the invariance + * propagation pass might mark these. But that's not an error + * on the programmer's part - it's our problem. It shouldn't + * actually matter anyway, so ignore it. + */ + if (var->get_num_state_slots() == 0) { + if (existing->data.invariant != var->data.invariant) { + linker_error(prog, "declarations for %s `%s' have " + "mismatching invariant qualifiers\n", + mode_string(var), var->name); + return; + } + + if (prog->IsES && existing->data.precision != var->data.precision) { + linker_error(prog, "declarations for %s `%s` have " + "mismatching precision qualifiers\n", + mode_string(var), var->name); + return; + } } + if (existing->data.centroid != var->data.centroid) { linker_error(prog, "declarations for %s `%s' have " "mismatching centroid qualifiers\n", @@ -1062,13 +1078,6 @@ cross_validate_globals(struct gl_shader_program *prog, mode_string(var), var->name); return; } - - if (prog->IsES && existing->data.precision != var->data.precision) { - linker_error(prog, "declarations for %s `%s` have " - "mismatching precision qualifiers\n", - mode_string(var), var->name); - return; - } } else variables->add_variable(var); } -- 2.10.0 _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
