[Mesa-dev] [PATCH] i965: allocate at least 1 BLEND_STATE element

[Mike Stroyan]

I had actually made the change to gen8_upload_blend_state, but after
reading through the gen8 PRM a few times I decided to back it out.
It does seem that the initial gen8 BLEND_STATE DWord can disable alpha test.
Of course, new hardware features may not always behave as described.
In that case allowances will need to be made.
Or, we could always fill in the extra eight bytes for the first BLEND_STATE
entry.
Given that the structure is 64 byte aligned, that 'extra' data is almost
always free.

On Thu, Jul 2, 2015 at 1:45 AM, Kenneth Graunke <kenneth at whitecape.org>
wrote:

> On Wednesday, July 01, 2015 10:16:28 AM Mike Stroyan wrote:
> > When there are no color buffer render targets, gen6 and gen7 still
> > use the first BLEND_STATE element to determine alpha test.
> > gen6_upload_blend_state was allocating zero elements when
> > ctx->Color.AlphaEnabled was false.
> > That left _3DSTATE_CC_STATE_POINTERS or _3DSTATE_BLEND_STATE_POINTERS
> > pointing to random data from some previous brw_state_batch().
> > That sometimes suppressed depth rendering when those bits
> > happened to mean COMPAREFUNC_NEVER.
> > This produced flickering shadows for dota2 reborn.
> > ---
> >  src/mesa/drivers/dri/i965/gen6_cc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/gen6_cc.c
> b/src/mesa/drivers/dri/i965/gen6_cc.c
> > index 2bfa271..2b76e24 100644
> > --- a/src/mesa/drivers/dri/i965/gen6_cc.c
> > +++ b/src/mesa/drivers/dri/i965/gen6_cc.c
> > @@ -51,7 +51,7 @@ gen6_upload_blend_state(struct brw_context *brw)
> >      * with render target 0, which will reference BLEND_STATE[0] for
> >      * alpha test enable.
> >      */
> > -   if (nr_draw_buffers == 0 && ctx->Color.AlphaEnabled)
> > +   if (nr_draw_buffers == 0)
> >        nr_draw_buffers = 1;
> >
> >     size = sizeof(*blend) * nr_draw_buffers;
> >
>
> Great catch!
>
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>
> And pushed:
>    9d408a4..fe2b748  master -> master
>
> I think we ought to change gen8_blend_state.c as well, but I'm not quite
> sure what change to make.  Either we should make the same change you did
> here, or delete the whole "We need at least 1 BLEND_STATE written"
> block.
>
> On Gen8+, it looks like the alpha test and other functions that might
> discard pixels are all in the shared/common DWord, and the per-color
> target DWord pairs look relatively harmless.  I suppose the null RT
> would still refer to BLEND_STATE[0]...so it might still be worth
> emitting one.  Any thoughts?
>



-- 

 Mike Stroyan - Software Architect
 LunarG, Inc.  - The Graphics Experts
 Cell:  (970) 219-7905
 Email: Mike at LunarG.com
 Website: http://www.lunarg.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150702/7439e3a9/attachment.html>