Roland,

Series looks good AFAICT. Thanks for nailing this nasty and long standing 
issues.

----- Original Message -----
> On Tue, May 21, 2013 at 6:12 PM,  <srol...@vmware.com> wrote:
> > From: Roland Scheidegger <srol...@vmware.com>
> >
> > This optimization disabled mask checks if the shader is simple enough.
> > While this should work correctly, the problem is that it can hide real
> > issues
> > because shaders in practice are usually complex enough (8 instructions or 1
> > texture is already enough) so this doesn't get used, whereas dumbed-down
> > tests which should hit all the same code paths suddenly do something quite
> > different. This was the reason that bug 41787 could not be easily tracked
> > as
> > stencil test not working correctly (piglit would in fact have failed some
> > tests without that optimization).
> > So disable it for now, it's unclear if it's much of a win in any case.
> > ---
> >  src/gallium/drivers/llvmpipe/lp_state_fs.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/gallium/drivers/llvmpipe/lp_state_fs.c
> > b/src/gallium/drivers/llvmpipe/lp_state_fs.c
> > index 9661273..b06f915 100644
> > --- a/src/gallium/drivers/llvmpipe/lp_state_fs.c
> > +++ b/src/gallium/drivers/llvmpipe/lp_state_fs.c
> > @@ -247,7 +247,7 @@ generate_fs_loop(struct gallivm_state *gallivm,
> >     struct lp_build_mask_context mask;
> >     boolean simple_shader =
> >     (shader->info.base.file_count[TGSI_FILE_SAMPLER] == 0 &&
> >                              shader->info.base.num_inputs < 3 &&
> > -                            shader->info.base.num_instructions < 8);
> > +                            shader->info.base.num_instructions < 8) && 0;
> 
> Maybe add a comment here as per your commit message as to why it's disabled.

Agreed.

You often write very thoughtful commit messages, but unfortunately for future 
reference commit messages can only be seen through git blame, and until any 
refactory completely hides them away.  It would be preferable to have the 
detailed explanation as comments, and merely say "See code comments for 
details" in the commit messages.

Jose

> 
> Alex
> 
> >     const boolean dual_source_blend = key->blend.rt[0].blend_enable &&
> >                                       util_blend_state_is_dual(&key->blend,
> >                                       0);
> >     unsigned attrib;
> > --
> > 1.7.9.5
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to