[Mesa-dev] [PATCH] mesa: replace gl_context->Multisample._Enabled with _mesa_is_multisample_enabled.

eocallaghan at alterapraxis.com eocallaghan at alterapraxis.com
Tue Mar 22 02:30:35 UTC 2016


Too quick, very nice cleanup, thanks.

Reviewed-by: Edward O'Callaghan <eocallaghan at alterapraxis.com>

On 2016-03-22 12:58, Bas Nieuwenhuizen wrote:
> This removes any dependency on driver validation of the number of
> framebuffer samples.
> 
> Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> ---
>  src/mesa/drivers/dri/i965/brw_util.h               |  5 +++--
>  src/mesa/drivers/dri/i965/gen6_cc.c                |  6 +++---
>  src/mesa/drivers/dri/i965/gen6_multisample_state.c |  2 +-
>  src/mesa/drivers/dri/i965/gen8_blend_state.c       |  6 +++---
>  src/mesa/drivers/dri/i965/gen8_depth_state.c       |  3 ++-
>  src/mesa/drivers/dri/i965/gen8_sf_state.c          |  4 ++--
>  src/mesa/main/framebuffer.c                        | 19 
> +++++++++++++++++++
>  src/mesa/main/framebuffer.h                        |  3 +++
>  src/mesa/main/mtypes.h                             |  1 -
>  src/mesa/main/state.c                              | 17 
> -----------------
>  src/mesa/program/prog_statevars.c                  |  2 +-
>  src/mesa/state_tracker/st_atom_rasterizer.c        |  4 ++--
>  src/mesa/state_tracker/st_atom_shader.c            |  2 +-
>  src/mesa/swrast/s_points.c                         |  4 ++--
>  14 files changed, 42 insertions(+), 36 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_util.h
> b/src/mesa/drivers/dri/i965/brw_util.h
> index 1f27e98..3e9a6ee 100644
> --- a/src/mesa/drivers/dri/i965/brw_util.h
> +++ b/src/mesa/drivers/dri/i965/brw_util.h
> @@ -34,6 +34,7 @@
>  #define BRW_UTIL_H
> 
>  #include "brw_context.h"
> +#include "main/framebuffer.h"
> 
>  extern GLuint brw_translate_blend_factor( GLenum factor );
>  extern GLuint brw_translate_blend_equation( GLenum mode );
> @@ -49,13 +50,13 @@ brw_get_line_width(struct brw_context *brw)
>      * implementation-dependent maximum non-antialiased line width."
>      */
>     float line_width =
> -      CLAMP(!brw->ctx.Multisample._Enabled && 
> !brw->ctx.Line.SmoothFlag
> +      CLAMP(!_mesa_is_multisample_enabled(&brw->ctx) &&
> !brw->ctx.Line.SmoothFlag
>              ? roundf(brw->ctx.Line.Width) : brw->ctx.Line.Width,
>              0.0f, brw->ctx.Const.MaxLineWidth);
>     uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
> 
>     /* Line width of 0 is not allowed when MSAA enabled */
> -   if (brw->ctx.Multisample._Enabled) {
> +   if (_mesa_is_multisample_enabled(&brw->ctx)) {
>        if (line_width_u3_7 == 0)
>           line_width_u3_7 = 1;
>     } else if (brw->ctx.Line.SmoothFlag && line_width < 1.5f) {
> diff --git a/src/mesa/drivers/dri/i965/gen6_cc.c
> b/src/mesa/drivers/dri/i965/gen6_cc.c
> index cee139b..f5a7d4d 100644
> --- a/src/mesa/drivers/dri/i965/gen6_cc.c
> +++ b/src/mesa/drivers/dri/i965/gen6_cc.c
> @@ -198,14 +198,14 @@ gen6_upload_blend_state(struct brw_context *brw)
>        if(!is_buffer_zero_integer_format) {
>           /* _NEW_MULTISAMPLE */
>           blend[b].blend1.alpha_to_coverage =
> -            ctx->Multisample._Enabled &&
> ctx->Multisample.SampleAlphaToCoverage;
> +            _mesa_is_multisample_enabled(ctx) &&
> ctx->Multisample.SampleAlphaToCoverage;
> 
>  	/* From SandyBridge PRM, volume 2 Part 1, section 8.2.3, BLEND_STATE:
>  	 * DWord 1, Bit 30 (AlphaToOne Enable):
>  	 * "If Dual Source Blending is enabled, this bit must be disabled"
>  	 */
>           WARN_ONCE(ctx->Color.Blend[b]._UsesDualSrc &&
> -                   ctx->Multisample._Enabled &&
> +                   _mesa_is_multisample_enabled(ctx) &&
>                     ctx->Multisample.SampleAlphaToOne,
>                     "HW workaround: disabling alpha to one with dual 
> src "
>                     "blending\n");
> @@ -213,7 +213,7 @@ gen6_upload_blend_state(struct brw_context *brw)
>              blend[b].blend1.alpha_to_one = false;
>  	 else
>  	    blend[b].blend1.alpha_to_one =
> -	       ctx->Multisample._Enabled && 
> ctx->Multisample.SampleAlphaToOne;
> +	       _mesa_is_multisample_enabled(ctx) && 
> ctx->Multisample.SampleAlphaToOne;
> 
>           blend[b].blend1.alpha_to_coverage_dither = (brw->gen >= 7);
>        }
> diff --git a/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> index 8eb620d..fcd313a 100644
> --- a/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> @@ -171,7 +171,7 @@ gen6_determine_sample_mask(struct brw_context *brw)
>     /* BRW_NEW_NUM_SAMPLES */
>     unsigned num_samples = brw->num_samples;
> 
> -   if (ctx->Multisample._Enabled) {
> +   if (_mesa_is_multisample_enabled(ctx)) {
>        if (ctx->Multisample.SampleCoverage) {
>           coverage = ctx->Multisample.SampleCoverageValue;
>           coverage_invert = ctx->Multisample.SampleCoverageInvert;
> diff --git a/src/mesa/drivers/dri/i965/gen8_blend_state.c
> b/src/mesa/drivers/dri/i965/gen8_blend_state.c
> index 786c79a..63186bd 100644
> --- a/src/mesa/drivers/dri/i965/gen8_blend_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_blend_state.c
> @@ -65,7 +65,7 @@ gen8_upload_blend_state(struct brw_context *brw)
> 
>     if (rb_zero_type != GL_INT && rb_zero_type != GL_UNSIGNED_INT) {
>        /* _NEW_MULTISAMPLE */
> -      if (ctx->Multisample._Enabled) {
> +      if (_mesa_is_multisample_enabled(ctx)) {
>           if (ctx->Multisample.SampleAlphaToCoverage) {
>              blend[0] |= GEN8_BLEND_ALPHA_TO_COVERAGE_ENABLE;
>              blend[0] |= GEN8_BLEND_ALPHA_TO_COVERAGE_DITHER_ENABLE;
> @@ -183,7 +183,7 @@ gen8_upload_blend_state(struct brw_context *brw)
>        * "If Dual Source Blending is enabled, this bit must be 
> disabled."
>        */
>        WARN_ONCE(ctx->Color.Blend[i]._UsesDualSrc &&
> -                ctx->Multisample._Enabled &&
> +                _mesa_is_multisample_enabled(ctx) &&
>                  ctx->Multisample.SampleAlphaToOne,
>                  "HW workaround: disabling alpha to one with dual src "
>                  "blending\n");
> @@ -226,7 +226,7 @@ gen8_upload_ps_blend(struct brw_context *brw)
>        dw1 |= GEN8_PS_BLEND_ALPHA_TEST_ENABLE;
> 
>     /* _NEW_MULTISAMPLE */
> -   if (ctx->Multisample._Enabled && 
> ctx->Multisample.SampleAlphaToCoverage)
> +   if (_mesa_is_multisample_enabled(ctx) &&
> ctx->Multisample.SampleAlphaToCoverage)
>        dw1 |= GEN8_PS_BLEND_ALPHA_TO_COVERAGE_ENABLE;
> 
>     /* Used for implementing the following bit of 
> GL_EXT_texture_integer:
> diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> index 93100a0..8aaa1a8 100644
> --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> @@ -29,6 +29,7 @@
>  #include "brw_state.h"
>  #include "brw_defines.h"
>  #include "brw_wm.h"
> +#include "main/framebuffer.h"
> 
>  /**
>   * Helper function to emit depth related command packets.
> @@ -303,7 +304,7 @@ pma_fix_enable(const struct brw_context *brw)
>     const bool kill_pixel =
>        brw->wm.prog_data->uses_kill ||
>        brw->wm.prog_data->uses_omask ||
> -      (ctx->Multisample._Enabled && 
> ctx->Multisample.SampleAlphaToCoverage) ||
> +      (_mesa_is_multisample_enabled(ctx) &&
> ctx->Multisample.SampleAlphaToCoverage) ||
>        ctx->Color.AlphaEnabled;
> 
>     /* The big formula in CACHE_MODE_1::NP PMA FIX ENABLE. */
> diff --git a/src/mesa/drivers/dri/i965/gen8_sf_state.c
> b/src/mesa/drivers/dri/i965/gen8_sf_state.c
> index 8b6f31f..2ac21f7 100644
> --- a/src/mesa/drivers/dri/i965/gen8_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_sf_state.c
> @@ -178,7 +178,7 @@ upload_sf(struct brw_context *brw)
>        dw3 |= GEN6_SF_USE_STATE_POINT_WIDTH;
> 
>     /* _NEW_POINT | _NEW_MULTISAMPLE */
> -   if ((ctx->Point.SmoothFlag || ctx->Multisample._Enabled) &&
> +   if ((ctx->Point.SmoothFlag || _mesa_is_multisample_enabled(ctx)) &&
>         !ctx->Point.PointSprite) {
>        dw3 |= GEN8_SF_SMOOTH_POINT_ENABLE;
>     }
> @@ -249,7 +249,7 @@ upload_raster(struct brw_context *brw)
>     if (ctx->Point.SmoothFlag)
>        dw1 |= GEN8_RASTER_SMOOTH_POINT_ENABLE;
> 
> -   if (ctx->Multisample._Enabled)
> +   if (_mesa_is_multisample_enabled(ctx))
>        dw1 |= GEN8_RASTER_API_MULTISAMPLE_ENABLE;
> 
>     if (ctx->Polygon.OffsetFill)
> diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
> index d18166d..f69dc6c 100644
> --- a/src/mesa/main/framebuffer.c
> +++ b/src/mesa/main/framebuffer.c
> @@ -983,3 +983,22 @@ _mesa_is_front_buffer_drawing(const struct
> gl_framebuffer *fb)
>     return (fb->_NumColorDrawBuffers >= 1 &&
>             fb->_ColorDrawBufferIndexes[0] == BUFFER_FRONT_LEFT);
>  }
> +
> +static inline GLuint
> +_mesa_geometric_nonvalidated_samples(const struct gl_framebuffer 
> *buffer)
> +{
> +   return buffer->_HasAttachments ?
> +      buffer->Visual.samples :
> +      buffer->DefaultGeometry.NumSamples;
> +}
> +
> +bool _mesa_is_multisample_enabled(const struct gl_context *ctx)
> +{
> +   /* The sample count may not be validated by the driver, but when it 
> is set,
> +    * we know that is in a valid range and no driver should ever 
> validate a
> +    * multisampled framebuffer to non-multisampled and vice-versa.
> +    */
> +   return ctx->Multisample.Enabled &&
> +          ctx->DrawBuffer &&
> +          _mesa_geometric_nonvalidated_samples(ctx->DrawBuffer) > 1;
> +}
> diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h
> index fa434d4..384f749 100644
> --- a/src/mesa/main/framebuffer.h
> +++ b/src/mesa/main/framebuffer.h
> @@ -146,4 +146,7 @@ _mesa_is_front_buffer_reading(const struct
> gl_framebuffer *fb);
>  extern bool
>  _mesa_is_front_buffer_drawing(const struct gl_framebuffer *fb);
> 
> +extern bool
> +_mesa_is_multisample_enabled(const struct gl_context *ctx);
> +
>  #endif /* FRAMEBUFFER_H */
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 5d8bfe4..399f450 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -667,7 +667,6 @@ struct gl_list_attrib
>  struct gl_multisample_attrib
>  {
>     GLboolean Enabled;
> -   GLboolean _Enabled;   /**< true if Enabled and multisample buffer 
> */
>     GLboolean SampleAlphaToCoverage;
>     GLboolean SampleAlphaToOne;
>     GLboolean SampleCoverage;
> diff --git a/src/mesa/main/state.c b/src/mesa/main/state.c
> index 57f1341..917ae4d 100644
> --- a/src/mesa/main/state.c
> +++ b/src/mesa/main/state.c
> @@ -344,20 +344,6 @@ update_frontbit(struct gl_context *ctx)
> 
> 
>  /**
> - * Update derived multisample state.
> - */
> -static void
> -update_multisample(struct gl_context *ctx)
> -{
> -   ctx->Multisample._Enabled = GL_FALSE;
> -   if (ctx->Multisample.Enabled &&
> -       ctx->DrawBuffer &&
> -       _mesa_geometric_samples(ctx->DrawBuffer) > 0)
> -      ctx->Multisample._Enabled = GL_TRUE;
> -}
> -
> -
> -/**
>   * Update the ctx->VertexProgram._TwoSideEnabled flag.
>   */
>  static void
> @@ -450,9 +436,6 @@ _mesa_update_state_locked( struct gl_context *ctx )
>     if (new_state & _NEW_PIXEL)
>        _mesa_update_pixel( ctx, new_state );
> 
> -   if (new_state & (_NEW_MULTISAMPLE | _NEW_BUFFERS))
> -      update_multisample( ctx );
> -
>     /* ctx->_NeedEyeCoords is now up to date.
>      *
>      * If the truth value of this variable has changed, update for the
> diff --git a/src/mesa/program/prog_statevars.c
> b/src/mesa/program/prog_statevars.c
> index db53377..03ece67 100644
> --- a/src/mesa/program/prog_statevars.c
> +++ b/src/mesa/program/prog_statevars.c
> @@ -502,7 +502,7 @@ _mesa_fetch_state(struct gl_context *ctx, const
> gl_state_index state[],
>                 minImplSize = ctx->Const.MinPointSizeAA;
>                 maxImplSize = ctx->Const.MaxPointSize;
>              }
> -            else if (ctx->Point.SmoothFlag || 
> ctx->Multisample._Enabled) {
> +            else if (ctx->Point.SmoothFlag ||
> _mesa_is_multisample_enabled(ctx)) {
>                 minImplSize = ctx->Const.MinPointSizeAA;
>                 maxImplSize = ctx->Const.MaxPointSizeAA;
>              }
> diff --git a/src/mesa/state_tracker/st_atom_rasterizer.c
> b/src/mesa/state_tracker/st_atom_rasterizer.c
> index c1c143b..ed9deb0 100644
> --- a/src/mesa/state_tracker/st_atom_rasterizer.c
> +++ b/src/mesa/state_tracker/st_atom_rasterizer.c
> @@ -236,12 +236,12 @@ static void update_raster_state( struct 
> st_context *st )
>     raster->line_stipple_factor = ctx->Line.StippleFactor - 1;
> 
>     /* _NEW_MULTISAMPLE */
> -   raster->multisample = ctx->Multisample._Enabled;
> +   raster->multisample = _mesa_is_multisample_enabled(ctx);
> 
>     /* _NEW_MULTISAMPLE | _NEW_BUFFERS */
>     raster->force_persample_interp =
>           !st->force_persample_in_shader &&
> -         ctx->Multisample._Enabled &&
> +         _mesa_is_multisample_enabled(ctx) &&
>           ctx->Multisample.SampleShading &&
>           ctx->Multisample.MinSampleShadingValue *
>           _mesa_geometric_samples(ctx->DrawBuffer) > 1;
> diff --git a/src/mesa/state_tracker/st_atom_shader.c
> b/src/mesa/state_tracker/st_atom_shader.c
> index ff90bd6..709f0cb 100644
> --- a/src/mesa/state_tracker/st_atom_shader.c
> +++ b/src/mesa/state_tracker/st_atom_shader.c
> @@ -74,7 +74,7 @@ update_fp( struct st_context *st )
>     /* _NEW_MULTISAMPLE | _NEW_BUFFERS */
>     key.persample_shading =
>        st->force_persample_in_shader &&
> -      st->ctx->Multisample._Enabled &&
> +      _mesa_is_multisample_enabled(st->ctx) &&
>        st->ctx->Multisample.SampleShading &&
>        st->ctx->Multisample.MinSampleShadingValue *
>        _mesa_geometric_samples(st->ctx->DrawBuffer) > 1;
> diff --git a/src/mesa/swrast/s_points.c b/src/mesa/swrast/s_points.c
> index d9aae73..3163b04 100644
> --- a/src/mesa/swrast/s_points.c
> +++ b/src/mesa/swrast/s_points.c
> @@ -22,7 +22,7 @@
>   * OTHER DEALINGS IN THE SOFTWARE.
>   */
> 
> -
> +#include "main/framebuffer.h"
>  #include "main/glheader.h"
>  #include "main/macros.h"
>  #include "s_context.h"
> @@ -257,7 +257,7 @@ smooth_point(struct gl_context *ctx, const SWvertex 
> *vert)
>     size = get_size(ctx, vert, GL_TRUE);
> 
>     /* alpha attenuation / fade factor */
> -   if (ctx->Multisample._Enabled) {
> +   if (_mesa_is_multisample_enabled(ctx)) {
>        if (vert->pointSize >= ctx->Point.Threshold) {
>           alphaAtten = 1.0F;
>        }



More information about the mesa-dev mailing list