On Tue, 2017-01-03 at 22:07 +0100, Marek Olšák wrote:
> On Tue, Jan 3, 2017 at 9:57 PM, Timothy Arceri
> <timothy.arc...@collabora.com> wrote:
> > On Tue, 2017-01-03 at 20:47 +0100, Marek Olšák wrote:
> > > On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceri
> > > <timothy.arc...@collabora.com> wrote:
> > > > We only need to set it when linking was successful and the
> > > > program
> > > > being linked is currently active.
> > > >
> > > > The programs_in_use mask is just used as a flag for now but in
> > > > a following patch we will use it to update the CurrentProgram
> > > > array.
> > > > ---
> > > > src/mesa/main/shaderapi.c | 22 +++++++++++++++++++++-
> > > > 1 file changed, 21 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/mesa/main/shaderapi.c
> > > > b/src/mesa/main/shaderapi.c
> > > > index 022afab..6c03dcb 100644
> > > > --- a/src/mesa/main/shaderapi.c
> > > > +++ b/src/mesa/main/shaderapi.c
> > > > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct gl_context
> > > > *ctx,
> > > > struct gl_shader_program *shProg)
> > > > return;
> > > > }
> > > >
> > > > - FLUSH_VERTICES(ctx, _NEW_PROGRAM);
> > > > + unsigned programs_in_use = 0;
> > > > + if (ctx->_Shader)
> > > > + for (unsigned stage = 0; stage < MESA_SHADER_STAGES;
> > > > stage++) {
> > > > + if (ctx->_Shader->CurrentProgram[stage] == shProg) {
> > > > + programs_in_use |= 1 << stage;
> > > > + }
> > > > + }
> > > >
> > > > _mesa_glsl_link_shader(ctx, shProg);
> > > >
> > > > + /* From section 7.3 (Program Objects) of the OpenGL 4.5
> > > > spec:
> > > > + *
> > > > + * "If LinkProgram or ProgramBinary successfully re-
> > > > links a
> > > > program
> > > > + * object that is active for any shader stage, then the
> > > > newly generated
> > > > + * executable code will be installed as part of the
> > > > current
> > > > rendering
> > > > + * state for all shader stages where the program is
> > > > active.
> > > > + * Additionally, the newly generated executable code is
> > > > made part of
> > > > + * the state of any program pipeline for all stages
> > > > where
> > > > the program
> > > > + * is attached."
> > > > + */
> > > > + if (shProg->data->LinkStatus && programs_in_use) {
> > > > + FLUSH_VERTICES(ctx, _NEW_PROGRAM);
> > > > + }
> > >
> > > This doesn't seem correct. If the context has unflushed vertices,
> > > calling FLUSH_VERTICES after linking will use the new linked
> > > program
> > > instead of the previous program for which the vertices were
> > > submitted.
> >
> > Your probably right but this doesn't make anything worse than it
> > already is right?
> >
> > Before this change we where always calling FLUSH_VERTICES(ctx,
> > _NEW_PROGRAM);
>
> The useless flagging is a different issue.
>
> FLUSH_VERTICES should be called before a state change, because it
> flushes draw calls that the driver hasn't even seen yet. After the
> driver processes those draw calls, _NEW_PROGRAM is set. That's how it
> works.
>
> If you instead change a state and then call FLUSH_VERTICES, the
> previous unflushed draw calls will use the new state, which is
> incorrect.
But the state doesn't change just from linking does it? The new
programs won't be made current until glUseProgram() is called.
>
> Marek
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
