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 <tapani.pa...@intel.com>
---
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.
+ }
+ }
+
/* 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)?
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.
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.
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
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
