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

[Andres Gomez]

On Thu, 2016-06-30 at 10:21 +1000, Timothy Arceri wrote:
> On Wed, 2016-06-29 at 17:23 +0300, Andres Gomez wrote:
> > 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.
> 
> Ok. Please include some of that in the commit message. In the end I
> needed to cache this anyway. I got rid of the duplication in
> gl_shader_program instead so you can have a:
> 
> Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>
> 
> It's still concerning that the value in gl_program is wrong but I
> don't
> think we actually use it anywhere else so I guess we dont need to
> worry.

OK. Thanks for the review, Tim!

-- 

Br,

Andres