[Mesa-dev] [PATCH 3/3] mesa: return initial value for VALIDATE_STATUS if pipe not bound

[Timothy Arceri]

On Sun, 2015-11-22 at 22:25 +1100, Timothy Arceri wrote:
> On Sun, 2015-11-22 at 11:00 +0100, gregory hainaut wrote:
> > On Sun, 22 Nov 2015 14:25:31 +1100
> > Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> > 
> > > On Tue, 2015-09-01 at 13:53 +0300, Tapani Pälli wrote:
> > > > From OpenGL 4.5 Core spec (7.13):
> > > > 
> > > >     "If pipeline is a name that has been generated (without
> > > > subsequent
> > > >     deletion) by GenProgramPipelines, but refers to a program
> > > > pipeline
> > > >     object that has not been previously bound, the GL first
> > > > creates
> > > > a new state vector in the same manner as when
> > > > BindProgramPipeline
> > > >     creates a new program pipeline object."
> > > > 
> > > > I interpret this as "If GetProgramPipelineiv gets called
> > > > without
> > > > a
> > > > bound (but valid) pipeline object, the state should reflect
> > > > initial
> > > > state of a new pipeline object." This is also expected
> > > > behaviour
> > > > by
> > > > ES31-CTS.sepshaderobjs.PipelineApi conformance test.
> > > > 
> > > > Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> > > > ---
> > > >  src/mesa/main/pipelineobj.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/mesa/main/pipelineobj.c
> > > > b/src/mesa/main/pipelineobj.c
> > > > index 07acbf1..c2e1d29 100644
> > > > --- a/src/mesa/main/pipelineobj.c
> > > > +++ b/src/mesa/main/pipelineobj.c
> > > > @@ -614,7 +614,8 @@ _mesa_GetProgramPipelineiv(GLuint pipeline,
> > > > GLenum pname, GLint *params)
> > > >        *params = pipe->InfoLog ? strlen(pipe->InfoLog) + 1 : 0;
> > > >        return;
> > > >     case GL_VALIDATE_STATUS:
> > > > -      *params = pipe->Validated;
> > > > +      /* If pipeline is not bound, return initial value 0. */
> > > > +      *params = (ctx->_Shader->Name != pipe->Name)
> > > > ? 0 : pipe->Validated;
> > > 
> > > Hi Tapani,
> > > 
> > > As Gregory has pointed out this change causes a large number of
> > > the
> > > SSO deqp tests to fail.
> > > 
> > > I'm not sure what the solution is yet but one thing I have
> > > noticed
> > > is
> > > that with this change we will always return a value of 0 as
> > > nothing
> > > ever sets the value of ctx->_Shader->Name.
> > > 
> > > I did try setting it when the bind is done but the deqp tests
> > > still
> > > fail as the bind is done after the validate in those tests. Are
> > > you
> > > sure that this must return 0 if the pipe is not bound?
> > > 
> > > Tim
> > > 
> > > >        return;
> > > >     case GL_VERTEX_SHADER:
> > > >        *params = pipe->CurrentProgram[MESA_SHADER_VERTEX]
> > 
> > Hello,
> > 
> > Here my understanding of the specification (with my point of view
> > of
> > GL
> > users).
> > 
> > On various objects, gen* cmds reserve a key in a hash table but
> > objects
> > aren't created. Generally the object are created when they're bound
> > (often call bind to
> > create). However SSO API is (closer of the/a) direct state access
> > API. It is really 
> > awkward to bind the object to modify it. Besides there is now full
> > DSA. 
> > 
> > So for me, the following pipeline command must created the object
> > if
> > it
> > wasn't created (read if it wasn't bound)
> > 
> > 1/ BindProgramPipeline
> > 2/ UseProgramStages
> > 3/ CreateProgramPipelines (???: not sure, but likely)
> > 4/ ActiveShaderProgram
> > 5/ GetProgramPipelineiv
> > 6/ ValidateProgramPipeline
> > 
> > Note: can be read as all pipeline commands, so it is nearly
> > equivalent to
> > create the object in GenProgramPipelines. The only exception is
> > IsProgramPipeline.
> > 
> > 
> > So GetProgramPipelineiv (and others gl cmd) must start with:
> > if (! pipe->IsCreated ) {
> >     pipe->Init();
> >     pipe->IsCreated = true;
> > }
> > 
> > 
> > For example:
> > GenProgramPipelines(1, &pipe);
> > GetProgramPipelineiv(pipe, X, X); // Must create the object and
> > therefore will return the initial value (0)
> > 
> > GenProgramPipelines(1, &pipe2);
> > ValidateProgramPipeline(pipe2); // Will create the object
> > GetProgramPipelineiv(pipe2, X, X); // object was created by
> > ValidateProgramPipeline, we can return the property
> > 
> > As a side note, if Mesa is updated. Piglit test will need some
> > updates too.
> > 
> > Best regards,
> > Gregory
> 
> I did some digging in the khronos private bugzilla and it seems this
> issue was resolved and the spec updated to include the following
> failure case to validation:
> 
> "There is no current program object specified byUseProgram, there is
> acurrent program pipeline object, and that object is empty (no
> executable code is installed for any stage)."
> 
> So it seems the CTS is correct and the DEQP tests are wrong.
> 
> However as I pointed out I think this patch is still broken as
> _Shader
> ->Name is always 0 currently as far as I could tell.
> 
> Does anyone know if its possible to report bugs against DEQP? I
> couldn't find anywhere to do it.

Reported lets hope they fix it.

https://code.google.com/p/android/issues/detail?can=2&start=0&num=100&q
=&colspec=ID%20Type%20Status%20Owner%20Summary%20Stars&groupby=&sort=&i
d=194913

> 
> Tim
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev