On 12/08/2015 03:55 PM, Timothy Arceri wrote:
On Tue, 2015-12-08 at 13:34 +0200, Tapani Pälli wrote:
On 12/08/2015 01:31 PM, Tapani Pälli wrote:
On 12/08/2015 12:57 PM, Timothy Arceri wrote:
On Tue, 2015-12-08 at 11:16 +0200, Tapani Pälli wrote:
Validation checks that we do not have active shader stages that
are
not
used by the pipeline.
This fixes a subtest in following CTS test:
ES31-CTS.sepshaderobjs.StateInteraction
v2: move as generic validation check for both ES and desktop
(Timothy)
Signed-off-by: Tapani Pälli <[email protected]>
---
src/mesa/main/context.c | 9 +++++++++
src/mesa/main/pipelineobj.c | 35
+++++++++++++++++++++++++++++++++++
src/mesa/main/pipelineobj.h | 2 ++
3 files changed, 46 insertions(+)
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index be983d4..d73c984 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -2042,6 +2042,15 @@ _mesa_valid_to_render(struct gl_context
*ctx,
const char *where)
}
}
+ /* If SSO in use, validate that all linked program stages
are
used. */
+ if (ctx->Pipeline.Current) {
+ if (!_mesa_active_program_stages_used(ctx
->Pipeline.Current))
{
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "Shader program active for shader stages
that "
+ "are not used by the pipeline");
You can remove this now. The call to
_mesa_validate_program_pipeline()
below should cause the error to be thrown for us when it fails.
Oops forgot this one. This is necessary to be run as the full
validation
happens only if pipeline was not validated before.
I see. Well in that case shouldn't we set the validated flag to false
when UseProgramStages is set? Since the pipeline could now be invalid
for this rule and for:
"One program object is active for at least two shader stages
and a second program is active for a shader stage between two
stages for which the first program was active."
Surely it would be less of a preformance hit to re-run full validation
when UseProgramStages is called on an than to re-validate these two
rules everytime?
Yeah I guess so, overall I was hoping to prevent full validation runs
and run just 'what is absolutely required' both in UseProgramStages and
in draw time but I agree it would be good to get functionality in place
before trying to optimize too much. It looks like interface matching
will anyway be the hottest spot to optimize, not these small loops.
+ }
+ }
+
/* If a program is active and SSO not in use, check if
validation
of
* samplers succeeded for the active program. */
if (ctx->_Shader->ActiveProgram && ctx->_Shader != ctx
->Pipeline.Current) {
diff --git a/src/mesa/main/pipelineobj.c
b/src/mesa/main/pipelineobj.c
index f4ce3ed..9067551 100644
--- a/src/mesa/main/pipelineobj.c
+++ b/src/mesa/main/pipelineobj.c
@@ -782,6 +782,21 @@ program_stages_interleaved_illegally(const
struct gl_pipeline_object *pipe)
return false;
}
+bool
+_mesa_active_program_stages_used(struct gl_pipeline_object
*pipe)
+{
+ unsigned i;
+ for (i = 0; i < MESA_SHADER_STAGES; i++) {
+ if (!pipe->CurrentProgram[i])
+ continue;
+ /* If program has linked but not used by pipeline. */
+ if (pipe->CurrentProgram[i]->LinkStatus &&
+ !pipe->CurrentProgram[i]->ValidProgramUse)
Two things here first if ValidProgramUse is false we need to redo
the
validation to be sure it really is an invalid use as its possible
that
the program was relinked since the UseProgramStages call and is
now
valid.
I'm not sure about this. This feels like the ValidProgramUse would
be
then unnecessary as we would anyway need to iterate through
everything
(just because of possible relink)?
Right I'm leaning towards removing ValidProgramUse and setting the
validated pipeline flag to false when UseProgramStages is called then
revalidating everything. As you said with regards to the sampler checks
we can break things up later and introduce flags if perf testing shows
it to be useful.
Yeah, this seems the simplest way to get things working.
Second are you sure about the LinkStatus check? Won't this cause
validation to pass when it should fail if the program was linked
successfully but ValidProgramUse is false, and we then do a
relink that
fails? As the program will stay in the pipeline.
IMO we need to check that program has been linked, otherwise it
should
not be considered active at all. UseProgramStages will need to be
called first to have actual value in ValidProgramUse and that
cannot
be called with program that has not been succesfully linked.
But the LinkStatus flag gets set to false for a program that has
already been used with UseProgramStage but is then relinked
unsuccesfully which is what patch 3 is about.
I've also sent an alternate fix for patch 3 as I spent a bunch of time
testing and digging into it. It was just easier to send it out since it
was done.
OK, cool.
Make that three things. It seems there is already code that
should be
doing this validation already. The first validation test in
_mesa_validate_program_pipeline() calls
program_stages_all_active() in
a loop which should fail for this case. The code pretty much does
what
I was suggesting only it always does the validation, there is no
flag
to reduce calls. I guess it really shouldn't matter a huge deal
since
its only done once when somethings relinked or a sampler changes.
Yeah it seems, I did not realize it's trying to test this same
thing
as it did not catch this but then again I guess it's not called at
all
during draw time currently.
From my quick testing on the CTS test the program in the
pipeline only
has a single shader when it should have two so the test fails
with this
existing code. As per my first point I believe we always need to
The test in question is "Separable program with both vertex and
fragment shaders attached to only one stage". It fails because
there
is no existing check run at draw time for the case where you enable
one stage but supply a program with multiple stages.
I believe the existing code should cover it if we reset the validated
pipeline flag.
Yes, I've tested that this is enough for it.
revalidate here to be sure that it really is an invalid use of
the
program so what we really need to do is track down why there is
only a
single shader in the program when it seems there should be two.
If we
can resolve that we might not need to rework this code unless
there
really is a performance hit.
+ return false;
+ }
+ return true;
+}
+
extern GLboolean
_mesa_validate_program_pipeline(struct gl_context* ctx,
struct gl_pipeline_object
*pipe)
@@ -839,6 +854,26 @@ _mesa_validate_program_pipeline(struct
gl_context* ctx,
return GL_FALSE;
}
+ /* Section 11.1.3.11 (Validation) of the OpenGL 4.5 and
OpenGL ES
3.1
+ * spec say:
+ *
+ * "An INVALID_OPERATION error is generated by any
command
that trans-
+ * fers vertices to the GL or launches compute work if
the
current set
+ * of active program objects cannot be executed, for
reasons
including:
+ *
+ * ...
+ *
+ * "A program object is active for at least one, but
not all
of
+ * the shader stages that were present when the program
was
linked."
+ */
+ if (!_mesa_active_program_stages_used(pipe)) {
+ pipe->InfoLog =
+ ralloc_strdup(pipe,
+ "Shader program active for shader
stages that
"
+ "are not used by the pipeline");
+ return GL_FALSE;
+ }
+
/* Section 2.11.11 (Shader Execution), subheading
"Validation,"
of the
* OpenGL 4.1 spec says:
*
diff --git a/src/mesa/main/pipelineobj.h
b/src/mesa/main/pipelineobj.h
index fbcb765..e56b522 100644
--- a/src/mesa/main/pipelineobj.h
+++ b/src/mesa/main/pipelineobj.h
@@ -70,6 +70,8 @@ extern GLboolean
_mesa_validate_program_pipeline(struct gl_context * ctx,
struct gl_pipeline_object
*pipe);
+extern bool
+_mesa_active_program_stages_used(struct gl_pipeline_object
*pipe);
extern void GLAPIENTRY
_mesa_UseProgramStages(GLuint pipeline, GLbitfield stages,
GLuint
program);
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
