[Mesa-dev] [PATCH 23/28] glsl: add pack varying to resource list for vertex input / fragment output
Timothy Arceri
timothy.arceri at collabora.com
Thu Jan 21 13:47:28 PST 2016
On Thu, 2016-01-21 at 09:23 -0800, Anuj Phogat wrote:
> On Wed, Jan 20, 2016 at 4:28 PM, Timothy Arceri
> <timothy.arceri at 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.arceri at 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.arceri at 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 at 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.
I don't see any such restriction in the spec:
"* PROGRAM_INPUT corresponds to the set of active input variables
used by the first shader stage of <program>. If <program> includes
multiple shader stages, input variables from any shader stage
other than the first will not be enumerated.
* PROGRAM_OUTPUT corresponds to the set of active output variables
(section 2.14.11) used by the last shader stage of <program>.
If <program> includes multiple shader stages, output variables
from any shader stage other than the last will not be enumerated."
...
For the UNIFORM, PROGRAM_INPUT, PROGRAM_OUTPUT, and
TRANSFORM_FEEDBACK_VARYING interfaces, the active resource list will
include all active variables for the interface, including any active
built-in variables."
>
>
> > 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 at lists.freedesktop.org
> > > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list