On 01/25/2016 12:29 PM, Timothy Arceri wrote:
On Mon, 2016-01-25 at 16:41 +1100, Timothy Arceri wrote:
On Mon, 2015-11-30 at 14:31 +0200, Tapani Pälli wrote:
Reviewed-by: Tapani Pälli <[email protected]>
On 11/25/2015 11:54 AM, Timothy Arceri wrote:
From: Gregory Hainaut <[email protected]>
This fixes an issue where the addition of the FLAT qualifier in
varying_matches::record() can break the expected varying order.
It also avoids a future issue with the relaxing of interpolation
qualifier matching constraints in GLSL 4.50.
V2: (by Timothy Arceri)
* reworked comment slightly
Signed-off-by: Gregory Hainaut <[email protected]>
Reviewed-by: Timothy Arceri <[email protected]>
---
src/glsl/link_varyings.cpp | 38
++++++++++++++++++++++++++++++++-
-----
1 file changed, 32 insertions(+), 6 deletions(-)
diff --git a/src/glsl/link_varyings.cpp
b/src/glsl/link_varyings.cpp
index ac2755f..71750d1 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -766,7 +766,7 @@ public:
gl_shader_stage consumer_stage);
~varying_matches();
void record(ir_variable *producer_var, ir_variable
*consumer_var);
- unsigned assign_locations(uint64_t reserved_slots);
+ unsigned assign_locations(uint64_t reserved_slots, bool
separate_shader);
void store_locations() const;
private:
@@ -988,11 +988,36 @@ varying_matches::record(ir_variable
*producer_var, ir_variable *consumer_var)
* passed to varying_matches::record().
*/
unsigned
-varying_matches::assign_locations(uint64_t reserved_slots)
+varying_matches::assign_locations(uint64_t reserved_slots, bool
separate_shader)
{
- /* Sort varying matches into an order that makes them easy to
pack. */
- qsort(this->matches, this->num_matches, sizeof(*this
->matches),
- &varying_matches::match_comparator);
+ /* We disable varying sorting for separate shader programs
for
the
+ * following reasons:
+ *
+ * 1/ All programs must sort the code in the same order to
guarantee the
+ * interface matching. However varying_matches::record()
will change the
+ * interpolation qualifier of some stages.
+ *
+ * 2/ GLSL version 4.50 removes the matching constrain on the
interpolation
+ * qualifier.
+ *
+ * From Section 4.5 (Interpolation Qualifiers) of the GLSL
4.40
spec:
+ *
+ * "The type and presence of interpolation qualifiers of
variables with
+ * the same name declared in all linked shaders for the
same
cross-stage
+ * interface must match, otherwise the link command will
fail.
+ *
+ * When comparing an output from one stage to an input of
a
subsequent
+ * stage, the input and output don't match if their
interpolation
+ * qualifiers (or lack thereof) are not the same."
+ *
+ * "It is a link-time error if, within the same stage, the
interpolation
+ * qualifiers of variables of the same name do not match."
+ */
+ if (!separate_shader) {
+ /* Sort varying matches into an order that makes them easy
to pack. */
+ qsort(this->matches, this->num_matches, sizeof(*this
->matches),
+ &varying_matches::match_comparator);
+ }
unsigned generic_location = 0;
unsigned generic_patch_location = MAX_VARYING*4;
@@ -1592,7 +1617,8 @@ assign_varying_locations(struct gl_context
*ctx,
reserved_varying_slot(producer, ir_var_shader_out) |
reserved_varying_slot(consumer, ir_var_shader_in);
- const unsigned slots_used =
matches.assign_locations(reserved_slots);
+ const unsigned slots_used =
matches.assign_locations(reserved_slots,
+ prog
->SeparateShader);
matches.store_locations();
for (unsigned i = 0; i < num_tfeedback_decls; ++i) {
I haven't figured out why yet but this patch breaks transform
feedback
when the last stage is a SSO (which there is currently no piglit
tests
for).
OK so the problem seems to be that if we don't do a sort we don't move
vec4's to the top of the list, this means they don't get assigned the
first locations which means they can end up getting split.
e.g
If float a is assigned location 0 then vec4 b will be assigned location
0 with component 3 in location 1.
This is obviously bad to begin with and it also causes the backend to
fall over for transform feedback as its not expecting a vec4 to be
split over multiple locations.
Did you try to disable varying packing? I believe it is broken for SSO
currently. IMO packing cannot be done safely before both producer and
consumer interface are known, and producer is really known only when we
validate the pipeline for first time. We discussed this with Samuel some
time ago related to cases where consumer interface may override producer
qualifiers, in those cases packing does not work correctly.
// Tapani
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
