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

Kenneth Graunke kenneth at whitecape.org
Tue Jun 19 10:29:48 PDT 2012


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.

>         .brw   = (BRW_NEW_CONTEXT |
>   		BRW_NEW_FRAGMENT_PROGRAM),
>         .cache = CACHE_NEW_VS_PROG
> diff --git a/src/mesa/drivers/dri/i965/gen6_wm_state.c b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> index cba2a57..662435e 100644
> --- a/src/mesa/drivers/dri/i965/gen6_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> @@ -98,11 +98,11 @@ upload_wm_state(struct brw_context *brw)
>      const struct brw_fragment_program *fp =
>         brw_fragment_program_const(brw->fragment_program);
>      uint32_t dw2, dw4, dw5, dw6;
> -   bool multisampled = false;
> +   bool multisampled_fbo = false;
>
>      /* _NEW_BUFFERS */
>      if (ctx->DrawBuffer->_ColorDrawBuffers[0])
> -      multisampled = ctx->DrawBuffer->_ColorDrawBuffers[0]->NumSamples > 0;
> +      multisampled_fbo = ctx->DrawBuffer->_ColorDrawBuffers[0]->NumSamples > 0;
>
>       /* CACHE_NEW_WM_PROG */
>      if (brw->wm.prog_data->nr_params == 0) {
> @@ -197,8 +197,12 @@ upload_wm_state(struct brw_context *brw)
>
>      dw6 |= _mesa_bitcount_64(brw->fragment_program->Base.InputsRead) <<
>         GEN6_WM_NUM_SF_OUTPUTS_SHIFT;
> -   if (multisampled) {
> -      dw6 |= GEN6_WM_MSRAST_ON_PATTERN;
> +   if (multisampled_fbo) {
> +      /* _NEW_MULTISAMPLE */
> +      if (ctx->Multisample.Enabled)
> +         dw6 |= GEN6_WM_MSRAST_ON_PATTERN;
> +      else
> +         dw6 |= GEN6_WM_MSRAST_OFF_PIXEL;

Whitespace errors.

>         dw6 |= GEN6_WM_MSDISPMODE_PERPIXEL;
>      } else {
>         dw6 |= GEN6_WM_MSRAST_OFF_PIXEL;
> @@ -230,7 +234,8 @@ const struct brw_tracked_state gen6_wm_state = {
>   		_NEW_COLOR |
>   		_NEW_BUFFERS |
>   		_NEW_PROGRAM_CONSTANTS |
> -		_NEW_POLYGON),
> +		_NEW_POLYGON |
> +                _NEW_MULTISAMPLE),
>         .brw   = (BRW_NEW_FRAGMENT_PROGRAM |
>   		BRW_NEW_BATCH),
>         .cache = (CACHE_NEW_SAMPLER |
> diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> index 8a6c09b..b1fe654 100644
> --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> @@ -161,9 +161,9 @@ upload_sf_state(struct brw_context *brw)
>      float point_size;
>      /* _NEW_BUFFERS */
>      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;
>
>      dw1 = GEN6_SF_STATISTICS_ENABLE |
>            GEN6_SF_VIEWPORT_TRANSFORM_ENABLE;
> @@ -261,7 +261,8 @@ upload_sf_state(struct brw_context *brw)
>      if (ctx->Line.StippleFlag && intel->is_haswell) {
>         dw2 |= HSW_SF_LINE_STIPPLE_ENABLE;
>      }
> -   if (multisampled)
> +   /* _NEW_MULTISAMPLE */
> +   if (multisampled_fbo && ctx->Multisample.Enabled)
>         dw2 |= GEN6_SF_MSRAST_ON_PATTERN;
>
>      /* FINISHME: Last Pixel Enable?  Vertex Sub Pixel Precision Select?
> @@ -309,7 +310,8 @@ const struct brw_tracked_state gen7_sf_state = {
>   		_NEW_LINE |
>   		_NEW_SCISSOR |
>   		_NEW_BUFFERS |
> -		_NEW_POINT),
> +		_NEW_POINT |
> +                _NEW_MULTISAMPLE),

Whitespace.

>         .brw   = BRW_NEW_CONTEXT,
>         .cache = CACHE_NEW_VS_PROG
>      },
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_state.c b/src/mesa/drivers/dri/i965/gen7_wm_state.c
> index f46e3f2..45c8e46 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_state.c
> @@ -39,12 +39,12 @@ upload_wm_state(struct brw_context *brw)
>      const struct brw_fragment_program *fp =
>         brw_fragment_program_const(brw->fragment_program);
>      bool writes_depth = false;
> -   bool multisampled = false;
> +   bool multisampled_fbo = false;
>      uint32_t dw1, dw2;
>
>      /* _NEW_BUFFERS */
>      if (ctx->DrawBuffer->_ColorDrawBuffers[0])
> -      multisampled = ctx->DrawBuffer->_ColorDrawBuffers[0]->NumSamples > 0;
> +      multisampled_fbo = ctx->DrawBuffer->_ColorDrawBuffers[0]->NumSamples > 0;
>
>      dw1 = dw2 = 0;
>      dw1 |= GEN7_WM_STATISTICS_ENABLE;
> @@ -79,8 +79,12 @@ upload_wm_state(struct brw_context *brw)
>          dw1 & GEN7_WM_KILL_ENABLE) {
>         dw1 |= GEN7_WM_DISPATCH_ENABLE;
>      }
> -   if (multisampled) {
> -      dw1 |= GEN7_WM_MSRAST_ON_PATTERN;
> +   if (multisampled_fbo) {
> +      /* _NEW_MULTISAMPLE */
> +      if (ctx->Multisample.Enabled)
> +         dw1 |= GEN7_WM_MSRAST_ON_PATTERN;
> +      else
> +         dw1 |= GEN7_WM_MSRAST_OFF_PIXEL;

Whitespace.

>         dw2 |= GEN7_WM_MSDISPMODE_PERPIXEL;
>      } else {
>         dw1 |= GEN7_WM_MSRAST_OFF_PIXEL;
> @@ -97,7 +101,8 @@ upload_wm_state(struct brw_context *brw)
>   const struct brw_tracked_state gen7_wm_state = {
>      .dirty = {
>         .mesa  = (_NEW_LINE | _NEW_POLYGON |
> -	        _NEW_COLOR | _NEW_BUFFERS),
> +	        _NEW_COLOR | _NEW_BUFFERS |
> +                _NEW_MULTISAMPLE),

And whitespace.

>         .brw   = (BRW_NEW_FRAGMENT_PROGRAM |
>   		BRW_NEW_BATCH),
>         .cache = CACHE_NEW_WM_PROG,
>

Otherwise, the patch looks good, so:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>


More information about the mesa-dev mailing list