[Mesa-dev] [PATCH] i965/msaa: Only do multisample rasterization if GL_MULTISAMPLE enabled.

[Kristian Høgsberg]

On Tue, Jun 19, 2012 at 11:27:17AM -0700, Paul Berry wrote:
> On 19 June 2012 10:29, Kenneth Graunke <kenneth at whitecape.org> wrote:
> 
> > On 06/19/2012 08:16 AM, Paul Berry wrote:
> >
> >>  From the GL 3.0 spec (p.116):
> >>
> >>     "Multisample rasterization is enabled or disabled by calling
> >>     Enable or Disable with the symbolic constant MULTISAMPLE."
> >>
> >> Elsewhere in the spec, where multisample rasterization is described
> >> (sections 3.4.3, 3.5.4, and 3.6.6), the following text is consistently
> >> used:
> >>
> >>     "If MULTISAMPLE is enabled, and the value of SAMPLE_BUFFERS is
> >>     one, then..."
> >>
> >> So, in other words, disabling GL_MULTISAMPLE should prevent
> >> multisample rasterization from occurring, even if the draw framebuffer
> >> is multisampled.  This patch implements that behaviour by setting the
> >> WM and SF stage's "multisample rasterization mode" to
> >> MSRAST_ON_PATTERN only when the draw framebuffer is multisampled *and*
> >> GL_MULTISAMPLE is enabled.
> >> ---
> >>  src/mesa/drivers/dri/i965/**gen6_sf_state.c |   10 ++++++----
> >>  src/mesa/drivers/dri/i965/**gen6_wm_state.c |   15 ++++++++++-----
> >>  src/mesa/drivers/dri/i965/**gen7_sf_state.c |   10 ++++++----
> >>  src/mesa/drivers/dri/i965/**gen7_wm_state.c |   15 ++++++++++-----
> >>  4 files changed, 32 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/**gen6_sf_state.c
> >> b/src/mesa/drivers/dri/i965/**gen6_sf_state.c
> >> index e0aaa90..aeed369 100644
> >> --- a/src/mesa/drivers/dri/i965/**gen6_sf_state.c
> >> +++ b/src/mesa/drivers/dri/i965/**gen6_sf_state.c
> >> @@ -122,9 +122,9 @@ upload_sf_state(struct brw_context *brw)
> >>     int i;
> >>     /* _NEW_BUFFER */
> >>     bool render_to_fbo = _mesa_is_user_fbo(brw->intel.**ctx.DrawBuffer);
> >> -   bool multisampled = false;
> >> +   bool multisampled_fbo = false;
> >>     if (ctx->DrawBuffer->_**ColorDrawBuffers[0])
> >> -      multisampled = ctx->DrawBuffer->_**ColorDrawBuffers[0]->**NumSamples
> >> > 0;
> >> +      multisampled_fbo = ctx->DrawBuffer->_**ColorDrawBuffers[0]->**NumSamples
> >> > 0;
> >>
> >>     int attr = 0, input_index = 0;
> >>     int urb_entry_read_offset = 1;
> >> @@ -242,7 +242,8 @@ upload_sf_state(struct brw_context *brw)
> >>        dw3 |= GEN6_SF_LINE_AA_MODE_TRUE;
> >>        dw3 |= GEN6_SF_LINE_END_CAP_WIDTH_1_**0;
> >>     }
> >> -   if (multisampled)
> >> +   /* _NEW_MULTISAMPLE */
> >> +   if (multisampled_fbo && ctx->Multisample.Enabled)
> >>        dw3 |= GEN6_SF_MSRAST_ON_PATTERN;
> >>
> >>     /* _NEW_PROGRAM | _NEW_POINT */
> >> @@ -349,7 +350,8 @@ const struct brw_tracked_state gen6_sf_state = {
> >>                _NEW_LINE |
> >>                _NEW_SCISSOR |
> >>                _NEW_BUFFERS |
> >> -               _NEW_POINT),
> >> +               _NEW_POINT |
> >> +                _NEW_MULTISAMPLE),
> >>
> >
> > Whitespace errors here.  I know Mesa isn't terribly consistent w.r.t. tabs
> > or spaces, but in general these files use 3 space indent with 8 space tabs.
> >  Please use tabs to match the surrounding lines.
> 
> 
> I'm reluctant to add fuel to coding convention debates, 

There is no debate here, we have an established convention and we
follow that.  Kenneths comments wasn't about space vs tabs, but that
the new _NEW_MULTISAMPLE token doesn't line up with the other tokens.
We don't have a strict rule about spaces vs tabs (except that tabs are
8 spaces wide) so you can use all spaces or a mix and that's fine.

Kristian