<div dir="ltr">On 23 January 2013 08:36, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 01/23/2013 11:16 AM, Paul Berry wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
On 23 January 2013 07:38, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a><br></div><div class="im">
<mailto:<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>>> wrote:<br>
<br>
On 01/21/2013 05:16 PM, Paul Berry wrote:<br>
<br>
This patch paves the way for allowing varying structs by<br>
generalizing<br></div>
varying_matches::compute___<u></u>packing_order to handle any type of<div class="im"><br>
varying.<br>
Previously, we packed in the order (vec4, vec2, float, vec3), with<br>
matrices being packed according to the size of their columns.<br>
Now, we<br>
pack everything according to its number of components mod 4, in the<br>
order (0, 2, 1, 3).<br>
<br>
There is no behavioural change for vectors. Matrices are now packed<br>
slightly more sensibly (e.g. a mat2 is now packed with vec4's, which<br>
guarantees that it will occupy a single varying slot, whereas it<br>
previously was packed with vec2's, which meant that it might get<br>
split<br>
across two adjacent slots).<br>
---<br>
src/glsl/link_varyings.cpp | 5 ++---<br>
1 file changed, 2 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp<br>
index 5a3240b..d8f501c 100644<br>
--- a/src/glsl/link_varyings.cpp<br>
+++ b/src/glsl/link_varyings.cpp<br>
@@ -804,16 +804,15 @@<br></div>
varying_matches::compute___<u></u>packing_order(ir_variable *var)<div class="im"><br>
{<br>
const glsl_type *element_type = var->type;<br>
<br>
- /* FINISHME: Support for "varying" records in GLSL 1.50. */<br>
while (element_type->base_type == GLSL_TYPE_ARRAY) {<br>
element_type = element_type->fields.array;<br>
}<br>
<br></div>
- switch (element_type->vector___<u></u>elements) {<br>
+ switch (element_type->component___<u></u>slots() % 4) {<div class="im"><br>
<br>
<br>
Will this produce the same result for all matrix types as well? I<br>
think a mat2x3 would be 3 in the old scheme, but 2 in the new<br>
scheme. Right?<br>
<br>
<br>
Yes, a number of matrix types will get new packing orders. Here's the<br>
complete list:<br>
<br>
- mat2x2 gets assigned PACKING_ORDER_VEC4 instead of<br>
PACKING_ORDER_VEC2. As I argue above, this is slightly better, because<br>
it guarantees that the matrix occupies a single varying slot.<br>
<br>
- mat2x3 gets assigned PACKING_ORDER_VEC2 instead of<br>
PACKING_ORDER_VEC3. This is kind of a wash. Previously, mat2x3 had a<br>
25% chance of having neither of its columns double parked, a 50% chance<br>
of having exactly one of its columns double parked, and a 25% chance of<br>
having both of its columns double parked. Now it always has exactly one<br>
of its columns double parked.<br>
<br>
- mat3x3 gets assigned PACKING_ORDER_SCALAR instead of<br>
PACKING_ORDER_VEC3. This doesn't affect much, since in both cases there<br>
is no guarantee of how the matrix will be aligned.<br>
<br>
- mat4x2 gets assigned PACKING_ORDER_VEC4 instead of<br>
PACKING_ORDER_VEC2. I think this is slightly better for the same reason<br>
as in mat2x2.<br>
<br>
- mat4x3 gets assigned PACKING_ORDER_VEC4 instead of<br>
PACKING_ORDER_VEC3. I think this is slightly better for the same reason<br>
as in mat2x2.<br>
</div></blockquote>
<br>
These changes potentially affect shader performance, do they also affect the number of slots used? In other words, will a different set of shaders fit vs. not fit in 16 vec4 slots?</blockquote><div><br></div><div>No, they just affect the order of packing. Packing is always done without gaps, so the same set of shaders will fit vs. not fit as before.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Do you think I should include these details in the commit message?<br>
</blockquote>
<br></div>
Since you've already written it, the cut-and-paste couldn't hurt.</blockquote><div><br></div><div>Ok, will do.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
case 1: return PACKING_ORDER_SCALAR;<br>
case 2: return PACKING_ORDER_VEC2;<br>
case 3: return PACKING_ORDER_VEC3;<br>
- case 4: return PACKING_ORDER_VEC4;<br>
+ case 0: return PACKING_ORDER_VEC4;<br>
default:<br>
assert(!"Unexpected value of vector_elements");<br>
return PACKING_ORDER_VEC4;<br>
<br>
<br>
<br>
</blockquote>
<br>
</div></div></blockquote></div><br></div></div>