On Wed, Jan 20, 2016 at 4:28 PM, Timothy Arceri <timothy.arc...@collabora.com> wrote: > On Wed, 2016-01-20 at 15:38 -0800, Anuj Phogat wrote: >> On Wed, Jan 20, 2016 at 1:39 PM, Timothy Arceri >> <timothy.arc...@collabora.com> wrote: >> > On Wed, 2016-01-20 at 10:06 -0800, Anuj Phogat wrote: >> > > On Mon, Dec 28, 2015 at 9:00 PM, Timothy Arceri >> > > <timothy.arc...@collabora.com> wrote: >> > > > This is needed now that we pack these type of varyings when >> > > > they >> > > > have a >> > > > component layout qualifier. >> > > > --- >> > > > src/glsl/linker.cpp | 15 ++++++++------- >> > > > 1 file changed, 8 insertions(+), 7 deletions(-) >> > > > >> > > > diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp >> > > > index 44dd7f0..52a326a 100644 >> > > > --- a/src/glsl/linker.cpp >> > > > +++ b/src/glsl/linker.cpp >> > > > @@ -3763,13 +3763,14 @@ build_program_resource_list(struct >> > > > gl_shader_program *shProg) >> > > > if (input_stage == MESA_SHADER_STAGES && output_stage == 0) >> > > > return; >> > > > >> > > > - /* Program interface needs to expose varyings in case of >> > > > SSO. >> > > > */ >> > > > - if (shProg->SeparateShader) { >> > > > - if (!add_packed_varyings(shProg, input_stage, >> > > > GL_PROGRAM_INPUT)) >> > > > - return; >> > > > - if (!add_packed_varyings(shProg, output_stage, >> > > > GL_PROGRAM_OUTPUT)) >> > > > - return; >> > > > - } >> > > > + /* Program interface needs to expose varyings in case of >> > > > SSO, >> > > > or in case of >> > > > + * vertex inputs/fragement outputs that are packed unsing >> > > > the >> > > > component >> > > > + * layout qualifier. >> > > > + */ >> > > > + if (!add_packed_varyings(shProg, input_stage, >> > > > GL_PROGRAM_INPUT)) >> > > > + return; >> > > > + if (!add_packed_varyings(shProg, output_stage, >> > > > GL_PROGRAM_OUTPUT)) >> > > > + return; >> > > > >> > > > if (!add_fragdata_arrays(shProg)) >> > > > return; >> > > > -- >> > > > 2.4.3 >> > > > >> > > > _______________________________________________ >> > > > mesa-dev mailing list >> > > > mesa-dev@lists.freedesktop.org >> > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > > >> > > I will give you my r-b on this if you can help me understand the >> > > concept of >> > > exposing varyings and how it's utilized during linking. >> > >> > Sure. The inputs and outputs to a program need to be added to the >> > resource list so that they can be queried by >> > ARB_program_interface_query via PROGRAM_INPUT/PROGRAM_OUTPUT. >> Right. Aren't the input/output variables added by >> add_interface_variables() ? >> add_packed_varyings() is used just for SSO with below explanation >> from >> commit : 4e7fd66: >> "Varyings can be considered inputs or outputs of a program only when >> SSO is in use. With multi-stage programs, inputs contain only inputs >> for first stage and outputs contains outputs of the final shader >> stage." >> >> Can you explain why do we need to add packed varyings to the resource >> list even with non-SSO? > > That commit message looks incorrect to me. I don't see anything in the > ARB_program_interface_query spec saying that only SSO programs can be > queried. Commit message just says varyings are also considered program inputs/ outputs in case of SSO. So, add them to to the resource list in addition to the vertex inputs and fragment outputs.
I meant to ask why do we need to always add "varyings" to the resource list to support querying vertex inputs and fragment outputs via PROGRAM_INPUT/ PROGRAM_OUTPUT? "varyings" are not supposed to be the part of these queries. Correct me if I've misunderstood it. > In fact without this change one I believe one of the piglit > tests for that spec breaks since we pack all explicit locations > regardless of having a component qualifier in order to deal with cases > were the default is used (I guess this could be improved with some sort > of table used to only pack when there are two varyings at the same > location but thats another issue). > This change likely fixes some other use cases that don't have tests > currently. > > I believe the reason for that commit message is that without the SSO > check the code in that commit broke some non-SSO piglit tests but this > was because it was adding both the inputs and outputs of the first and > last stage which I fixed in 0508d9504aa. > > >> >> > >> > I can add that to the commit message if you think its useful. >> > >> > >> > > _______________________________________________ >> > > mesa-dev mailing list >> > > mesa-dev@lists.freedesktop.org >> > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev