[Mesa-dev] [PATCH] Revert "i965: get PrimitiveMode from the program rather than the shader struct"

[Andres Gomez]

On Thu, 2016-06-30 at 00:09 +1000, Timothy Arceri wrote:
> 
> On 29 June 2016 11:29:17 pm AEST, Andres Gomez <agomez at igalia.com>
> wrote:
> > This reverts commit 644e015f0b9236e955d679cac4bcc7a1523fc475.
> > 
> > PrimitiveMode from the program doesn't hold the proper value when
> > reaching this code. We rather take it from the linked shader.
> 
> Usually when reverting you should say what the commit broke or what
> you are fixing.

Yes, I should probably have been more verbose here. The original patch
caused regressions in the following CTS tests:
 * GL44-CTS.stencil_texturing.functional
 * GL44-CTS.shading_language_420pack.binding_images
 * GL44-CTS.shading_language_420pack.binding_samplers
 * GL44-CTS.shading_language_420pack.binding_uniform_single_block
 * GL44-CTS.shading_language_420pack.implicit_conversions
 * GL44-CTS.shading_language_420pack.initializer_list
 * GL44-CTS.shading_language_420pack.length_of_vector_and_matrix
 * GL44-CTS.shading_language_420pack.line_continuation

All were SIGABRTing with something like:
glcts: brw_shader.cpp:613: unsigned int
tesslevel_outer_components(GLenum): Assertion `!"Bogus tessellation
domain"' failed.

The reason is that the PrimitiveMode used holds a value that is neither
of GL_TRIANGLES, GL_QUADS nor GL_ISOLINES.

> I'll take another look tomorrow but my feeling is program should have
> the correct value here. In my testing it seemed to be ok. Can you
> explain why the value is different? 

The patch is a quick revert to the original behavior, I didn't invest
enough time to know why brw->tess_eval_program->PrimitiveMode didn't
have the expected value by the time that code is reached.

Thanks for taking a look at this.

Br.

> 
> Thanks.
> > 
> > Signed-off-by: Andres Gomez <agomez at igalia.com>
> > ---
> > src/mesa/drivers/dri/i965/brw_tcs.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_tcs.c
> > b/src/mesa/drivers/dri/i965/brw_tcs.c
> > index d488715..2029ea5 100644
> > --- a/src/mesa/drivers/dri/i965/brw_tcs.c
> > +++ b/src/mesa/drivers/dri/i965/brw_tcs.c
> > @@ -395,8 +395,9 @@ brw_tcs_precompile(struct gl_context *ctx,
> >          _LinkedShaders[MESA_SHADER_TESS_CTRL]-
> > >TessCtrl.VerticesOut;
> >    }
> > 
> > -   key.tes_primitive_mode = brw->tess_eval_program ?
> > -      brw->tess_eval_program->PrimitiveMode : GL_TRIANGLES;
> > +   key.tes_primitive_mode =
> > shader_prog->_LinkedShaders[MESA_SHADER_TESS_EVAL]
> > +      ?
> > shader_prog->_LinkedShaders[MESA_SHADER_TESS_EVAL]-
> > >TessEval.PrimitiveMode
> > +      : GL_TRIANGLES;
> > 
> >    key.outputs_written = prog->OutputsWritten;
> >    key.patch_outputs_written = prog->PatchOutputsWritten;
> > -- 
> > 2.8.1
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
-- 

Br,

Andres