On Thu, Sep 28, 2017 at 6:33 AM, Iago Toral Quiroga <ito...@igalia.com> 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. -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev