Can someone push this patch for me? Thanks, Bas Nieuwenhuizen
On Wed, Mar 23, 2016 at 4:21 PM, Brian Paul <bri...@vmware.com> wrote: > No regressions here, Bas. > > -Brian > > > On 03/22/2016 01:27 PM, Brian Paul wrote: >> >> If you can wait until tomorrow, Bas, I'll do an overight piglit run to >> check for regressions. >> >> -Brian >> >> On 03/22/2016 12:31 PM, Marek Olšák wrote: >>> >>> Reviewed-by: Marek Olšák <marek.ol...@amd.com> >>> >>> Somebody from Intel or VMWare might want to take a look too. >>> >>> Marek >>> >>> On Tue, Mar 22, 2016 at 2:58 AM, Bas Nieuwenhuizen >>> <b...@basnieuwenhuizen.nl> wrote: >>>> >>>> This removes any dependency on driver validation of the number of >>>> framebuffer samples. >>>> >>>> Signed-off-by: Bas Nieuwenhuizen <b...@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; >>>> } >>>> -- >>>> 2.7.3 >>>> >>>> _______________________________________________ >>>> mesa-dev mailing list >>>> mesa-dev@lists.freedesktop.org >>>> >>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=OfoTM6iSz97qPGnB4W8irILcGJs5QWKVXdf0j8qwx9c&s=8mNsefrNuA466uJOahxnA2c9JZPEFuzbJ4VdZtWj8T0&e= >>>> >>> _______________________________________________ >>> mesa-dev mailing list >>> mesa-dev@lists.freedesktop.org >>> >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=OfoTM6iSz97qPGnB4W8irILcGJs5QWKVXdf0j8qwx9c&s=8mNsefrNuA466uJOahxnA2c9JZPEFuzbJ4VdZtWj8T0&e= >>> >>> >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev