On Thu, 2017-09-28 at 07:24 -0400, Ilia Mirkin wrote: > On Thu, Sep 28, 2017 at 6:33 AM, Iago Toral Quiroga <ito...@igalia.co > m> wrote: > > we can skip these slots when they are not read in the fragment > > shader > > and they are positioned right after a VUE header that we are > > already > > skipping. We also need to ensure that we are passing at least one > > other > > varying, since that is a hardware requirement. > > > > This helps alleviate a problem introduced with 99df02ca26f61 for > > separate shader objects: without separate shader objects we assign > > locations sequentially, however, since that commit we have changed > > the > > method for SSO so that the VUE slot assigned depends on the number > > of > > builtin slots plus the location assigned to the varying. This fixed > > layout is intended to help SSO programs by avoiding on-the-fly > > recompiles > > when swapping out shaders, however, it also means that if a varying > > uses > > a large location number close to the maximum allowed by the SF/FS > > units > > (31), then the offset introduced by the number of builtin slots can > > push > > the location outside the range and trigger an assertion. > > > > This problem is affecting at least the following CTS tests for > > enhanced layouts: > > > > KHR-GL45.enhanced_layouts.varying_array_components > > KHR-GL45.enhanced_layouts.varying_array_locations > > KHR-GL45.enhanced_layouts.varying_components > > KHR-GL45.enhanced_layouts.varying_locations > > > > which use SSO and the the location layout qualifier to select such > > location numbers explicitly. > > > > This change helps these tests because for SSO we always have to > > include > > VARYING_SLOT_CLIP_DIST{0,1} even if the fragment shader is very > > unlikely > > to read them, so by doing this we free builtin slots from the fixed > > VUE > > layout and we avoid the tests to crash in this scenario. > > > > Of course, this is not a proper fix, we'd still run into problems > > if someone > > tries to use an explicit max location and read gl_ViewportIndex, > > gl_LayerID or > > gl_CullDistancein in the FS, but that would be a much less common > > bug and we > > can probably wait to see if anyone actually runs into that > > situation in a real > > world scenario before making the decision that more aggresive > > changes are > > required to support this without reverting 99df02ca26f61. > > > > v2: > > - Add a debug message when we skip clip distances (Ilia) > > Sorry, I should have been more specific. I meant adding a debug > message when you detect the buggy condition -- i.e. too many inputs > being used, or whatever the error condition you're fixing here. You > mention that this isn't a proper fix and it's still possible to run > into problems. Would be nice to get a perf_debug() or something when > you see that happening. Should be possible pretty easily. I suspect > such a message would have saved you a lot of debugging time had it > been there, and will likely save the next poor sap a lot of time when > they run into the issue of using an explicit max location + viewport > index/etc.
Ah, not really. The driver asserts when this happens because it sees varying slots that go beyond the maximum limit, so I think we already have a good starting point for debugging this situations. Iago _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev