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

[Kristian Høgsberg]

On Tue, Jun 19, 2012 at 2:56 PM, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
> 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.

Oh, ok, I see I made a fool of myself here, the old "tabs don't align
in patches" trap.

Kristian