On Saturday, August 19, 2017 8:56:06 PM PDT Jason Ekstrand wrote: > In f9fd976e8adba733b08d we changed the clear value to be stored as an > isl_color_value. This had the side-effect same clear value check is now > happening directly between the f32[0] field of the isl_color_value and > ctx->Depth.Clear. This isn't what we want for two reasons. One is that > the comparison happens in floating point even for Z16 and Z24 formats. > Worse than that, ctx->Depth.Clear is a double so, even for 32-bit float > formats, we were comparing as doubles and not floats. This means that > the test basically always fails for anything other than 0.0f and 1.0f. > This caused a slight performance regression in Lightsmark 2008 because > it was using a depth clear value of 0.999 which can't be stored in a > 32-bit float so we were doing unneeded resolves. > > Cc: Kenneth Graunke <kenn...@whitecape.org> > Bugzilla: https://bugs.freedesktop.org/101678 > --- > src/mesa/drivers/dri/i965/brw_clear.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_clear.c > b/src/mesa/drivers/dri/i965/brw_clear.c > index 5eb2423..b7bf03d 100644 > --- a/src/mesa/drivers/dri/i965/brw_clear.c > +++ b/src/mesa/drivers/dri/i965/brw_clear.c > @@ -159,12 +159,22 @@ brw_fast_clear_depth(struct gl_context *ctx) > break; > } > > + /* Quantize the clear value to what can be stored in the actual depth > + * buffer. This makes the following check more accurate because it now > + * checks if the actual depth bits will match. It also prevents us from > + * getting a too-accurate depth value during depth testing or when > sampling > + * with HiZ enabled. > + */ > + float clear_value = > + mt->format == MESA_FORMAT_Z_FLOAT32 ? ctx->Depth.Clear : > + (unsigned)(ctx->Depth.Clear * fb->_DepthMax) / (float)fb->_DepthMax; > + > const uint32_t num_layers = depth_att->Layered ? depth_irb->layer_count : > 1; > > /* If we're clearing to a new clear value, then we need to resolve any > clear > * flags out of the HiZ buffer into the real depth buffer. > */ > - if (mt->fast_clear_color.f32[0] != ctx->Depth.Clear) { > + if (mt->fast_clear_color.f32[0] != clear_value) { > for (uint32_t level = mt->first_level; level <= mt->last_level; > level++) { > if (!intel_miptree_level_has_hiz(mt, level)) > continue; > @@ -201,7 +211,7 @@ brw_fast_clear_depth(struct gl_context *ctx) > } > } > > - mt->fast_clear_color.f32[0] = ctx->Depth.Clear; > + mt->fast_clear_color.f32[0] = clear_value; > } > > bool need_clear = false; >
This second version of the patch is: Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> (The one from 11 minutes earlier is bogus.)
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
