On 11/23/2015 10:18 AM, Timothy Arceri wrote:
On Mon, 2015-11-23 at 08:28 +0200, Tapani Pälli wrote:

On 11/22/2015 01:25 PM, 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_arc...@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.pa...@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.

IMO the check done by this patch is valid, Name field has the name of
currently bound program pipeline, it changes when current pipeline
object gets changed. If name does not match the pipeline given to
GetProgramPipelineiv it means it is not bound. If you call
BindProgramPipeline, Name changes.

Right thanks, I wasn't saying it wasn't correct to do the check just
that I couldn't see where name was set. I see how it works now, thanks.

Not a big deal but the check should probably be moved into the validate
function as it is one of the validation rules.

There is check in validation if pipeline is bound because validation can be also run against a pipeline that is not bound. In this case there is some difference in error reporting compared to running validation when pipeline is bound.

"If validation succeeded, no INVALID_OPERATION validation error is generated if pipeline is bound and no program is made current via UseProgram, given the current state. If validation failed, such errors are generated under the current state."

So error reporting is only happening if the pipeline is bound, otherwise it should just return validation result for that pipeline.


Does anyone know if its possible to report bugs against DEQP? I
couldn't find anywhere to do it.

Don't know about this one, it's part of Android so I guess the
process
is just sending changes through their patch submission process (?)

// Tapani
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to