On 04/20/2014 04:53 PM, Eric Anholt wrote: > Ian Romanick <i...@freedesktop.org> writes: > >> On 04/11/2014 04:35 PM, Eric Anholt wrote: >>> Ian Romanick <i...@freedesktop.org> writes: >>> >>>> From: Ian Romanick <ian.d.roman...@intel.com> >>>> >>>> Almost all of the time the location set by layout(location=...) is the >>>> location that will be used for the variable. Vertex shader inputs and >>>> fragment shader outputs, for example, are visible to the API. We just >>>> use those actual settings. >>>> >>>> Locations set for varyings, however, are a different story. In those >>>> cases, the locations set in the shader are just used to facilitate >>>> matching outputs to inputs. We need to track the value so that we can >>>> ensure that >>>> >>>> layout(location=2) out float foo; >>>> >>>> will be assigned the same resource as >>>> >>>> layout(location=2) in float bar; >>>> >>>> but we probably don't want to use 2 as the actual location in the set of >>>> varyings. >>> >>> I assumed that layout(location=2) would be using VARYING_SLOT_VARx -- I >>> don't know of any reason to try and pack these, and it sucks to see >>> another field in our already-bloated ir_variables. It looks like other >>> code in the series would go away if we just mapped things directly to >>> VARYING_SLOTs. >> >> I started off with that approach, and I encountered a couple problems. >> Since you can have a mix of varyings with and without explicit >> locations, it made the code for resetting the locations during link more >> complex. I recall thinking that the changes should have been simple and >> obvious, but they weren't. It was a few months ago at this point, so I >> don't recall the details. >> >> The other problem was that the i965 backend makes assumptions about >> there not being holes in the set of varyings used. When shaders ended >> up using, say, VARYING_SLOT_VAR0 and VARYING_SLOT_VAR1 (but not >> VARYING_SLOT_VAR2), I started hitting assertions in the backend. It >> seemed easier and less obtrusive to have the frontend continue giving >> the backend shaders that matched its assumptions. > > There shouldn't be anything requiring that varying slots used be > contiguous. For example, someone could use gl_TexCoord[0] and > gl_TexCoord[2] today, and there should be an open slot between the two.
Yeah... my memory was a bit faulty. The problem wasn't in the
backend. The problem was in lower_packed_varyings. I also believe
that lower_packed_varyings would split and repack gl_TexCoord in your
example. :) I removed user_location (just using the explicit_location
flag and location), and it hits:
arb_separate_shader_object-rendezvous_by_location:
../../src/glsl/lower_packed_varyings.cpp:546: ir_dereference*
{anonymous}::lower_packed_varyings_visitor::get_packed_varying_deref(unsigned
int, ir_variable*, const char*, unsigned int): Assertion `slot <
locations_used' failed.
I'm pretty sure this is the same assertion that I was hitting before.
Removing the calls to lower_packed_varyings from the linker avoids the
assertion (duh), and all the SSO rendering tests still pass.
All of the transform feedback code assumes that varyings are packed.
Without lower_packed_varyings, I hit:
ext_transform_feedback-interleaved: gen7_sol_state.c:148:
gen7_upload_3dstate_so_decl_list: Assertion `vue_map->varying_to_slot[varying]
>= 0' failed.
I haven't verified this yet, but I assume that if we just pack varyings
that don't have an explicit location the backend XFB code will still
assert when a varying with a location is used for XFB.
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
