On Fri, 2016-01-15 at 08:50 +0200, Tapani Pälli wrote:
>
> On 01/06/2016 03:40 AM, Timothy Arceri wrote:
> > Even if re-linking fails rendering shouldn't fail as the previous
> > succesfully linked program will still be available. It also
> > shouldn't
> > be possible to have an unlinked program as part of the current
> > rendering
> > state.
> >
> > This fixes a subtest in:
> > ES31-CTS.sepshaderobjs.StateInteraction
>
> Which is the last one, after this change this horrible test from
> depths
> of hell starts to finally pass!
hehe
>
> It would be cool if Ian and Brian could comment a bit here why these
> checks were originally done and if they can be safely removed, was is
> just to play 'extra safe'?
I looked at the history a while back so to save everyone some time here
it is.
This was added way back in 2009 by Brian:
commit 56c4226fcc54158eb7fe54eeb13539a979ec155c
Author: Brian Paul <bri...@vmware.com>
Date: Fri Aug 14 10:45:17 2009 -0600
mesa: new _mesa_valid_to_render() function
Tests if the current shader/program is valid and that the
framebuffer is
complete. To be called by glBegin, glDrawArrays, etc.
There seems to have been only two major changes since then:
commit 84eba3ef71dfa822e5ff0463032cdd2e3515b888
Author: Ian Romanick <ian.d.roman...@intel.com>
Date: Wed Oct 13 13:58:44 2010 -0700
Track separate programs for each stage
The assumption is that all stages are the same program or that
varyings are passed between stages using built-in varyings.
commit 79146065f9261a9004359338f1a7b8b5a534ebc3
Author: Ian Romanick <ian.d.roman...@intel.com>
Date: Fri Feb 7 21:13:02 2014 -0800
mesa: Refactor per-stage link check to its own function
Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
To me it looks like this was just added to play safe when adding other
valid to render checks.
>
>
> > This change should improve performance on CPU limited benchmarks as
> > noted
> > in commit d6c6b186cf308f.
> >
> > From Section 7.3 (Program Objects) of the OpenGL 4.5 spec:
> >
> > "If a program object that is active for any shader stage is re
> > -linked
> > unsuccessfully, the link status will be set to FALSE, but any
> > existing
> > executables and associated state will remain part of the
> > current rendering
> > state until a subsequent call to UseProgram, UseProgramStages,
> > or
> > BindProgramPipeline removes them from use. If such a program
> > is attached to
> > any program pipeline object, the existing executables and
> > associated state
> > will remain part of the program pipeline object until a
> > subsequent call to
> > UseProgramStages removes them from use. An unsuccessfully
> > linked program may
> > not be made part of the current rendering state by UseProgram
> > or added to
> > program pipeline objects by UseProgramStages until it is
> > successfully
> > re-linked."
> >
> > "void UseProgram(uint program);
> >
> > ...
> >
> > An INVALID_OPERATION error is generated if program has not been
> > linked, or
> > was last linked unsuccessfully. The current rendering state is
> > not modified."
> >
> > V2: apply the rule to both core and compat.
> >
> > Cc: Tapani Pälli <tapani.pa...@intel.com>
> > Cc: Ian Romanick <ian.d.roman...@intel.com>
> > Cc: Brian Paul <bri...@vmware.com>
> > ---
> > src/mesa/main/context.c | 63 +++---------------------------------
> > -------------
> > 1 file changed, 3 insertions(+), 60 deletions(-)
> >
> > diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> > index be983d4..f3fd01f 100644
> > --- a/src/mesa/main/context.c
> > +++ b/src/mesa/main/context.c
> > @@ -1930,31 +1930,6 @@ _mesa_check_blend_func_error(struct
> > gl_context *ctx)
> > return GL_TRUE;
> > }
> >
> > -static bool
> > -shader_linked_or_absent(struct gl_context *ctx,
> > - const struct gl_shader_program *shProg,
> > - bool *shader_present, const char *where)
> > -{
> > - if (shProg) {
> > - *shader_present = true;
> > -
> > - if (!shProg->LinkStatus) {
> > - _mesa_error(ctx, GL_INVALID_OPERATION, "%s(shader not
> > linked)", where);
> > - return false;
> > - }
> > -#if 0 /* not normally enabled */
> > - {
> > - char errMsg[100];
> > - if (!_mesa_validate_shader_program(ctx, shProg, errMsg))
> > {
> > - _mesa_warning(ctx, "Shader program %u is invalid: %s",
> > - shProg->Name, errMsg);
> > - }
> > - }
> > -#endif
> > - }
> > -
> > - return true;
> > -}
> >
> > /**
> > * Prior to drawing anything with glBegin, glDrawArrays, etc.
> > this function
> > @@ -1967,54 +1942,22 @@ shader_linked_or_absent(struct gl_context
> > *ctx,
> > GLboolean
> > _mesa_valid_to_render(struct gl_context *ctx, const char *where)
> > {
> > - unsigned i;
> > -
> > /* This depends on having up to date derived state (shaders)
> > */
> > if (ctx->NewState)
> > _mesa_update_state(ctx);
> >
> > - if (ctx->API == API_OPENGL_CORE || ctx->API == API_OPENGLES2) {
> > - bool from_glsl_shader[MESA_SHADER_COMPUTE] = { false };
> > -
> > - for (i = 0; i < MESA_SHADER_COMPUTE; i++) {
> > - if (!shader_linked_or_absent(ctx, ctx->_Shader
> > ->CurrentProgram[i],
> > - &from_glsl_shader[i],
> > where))
> > - return GL_FALSE;
> > - }
> > -
> > - /* In OpenGL Core Profile and OpenGL ES 2.0 / 3.0, there are
> > no assembly
> > - * shaders. Don't check state related to those.
> > - */
> > - } else {
> > - bool has_vertex_shader = false;
> > - bool has_fragment_shader = false;
> > -
> > - /* In OpenGL Compatibility Profile, there is only vertex
> > shader and
> > - * fragment shader. We take this path also for API_OPENGLES
> > because
> > - * optimizing that path would make the other (more common)
> > paths
> > - * slightly slower.
> > - */
> > - if (!shader_linked_or_absent(ctx,
> > - ctx->_Shader
> > ->CurrentProgram[MESA_SHADER_VERTEX],
> > - &has_vertex_shader, where))
> > - return GL_FALSE;
> > -
> > - if (!shader_linked_or_absent(ctx,
> > - ctx->_Shader
> > ->CurrentProgram[MESA_SHADER_FRAGMENT],
> > - &has_fragment_shader, where))
> > - return GL_FALSE;
> > -
> > + if (ctx->API == API_OPENGL_COMPAT) {
> > /* Any shader stages that are not supplied by the GLSL
> > shader and have
> > * assembly shaders enabled must now be validated.
> > */
> > - if (!has_vertex_shader
> > + if (!ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX]
> > && ctx->VertexProgram.Enabled && !ctx
> > ->VertexProgram._Enabled) {
> > _mesa_error(ctx, GL_INVALID_OPERATION,
> > "%s(vertex program not valid)", where);
> > return GL_FALSE;
> > }
> >
> > - if (!has_fragment_shader) {
> > + if (!ctx->_Shader->CurrentProgram[MESA_SHADER_FRAGMENT]) {
> > if (ctx->FragmentProgram.Enabled && !ctx
> > ->FragmentProgram._Enabled) {
> > _mesa_error(ctx, GL_INVALID_OPERATION,
> > "%s(fragment program not valid)", where);
> >
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
