[Mesa-dev] [PATCH] i965: Set address rounding bits for GL_NEAREST filtering as well.

[Jason Ekstrand]

On Thu, Mar 17, 2016 at 4:42 PM, Jordan Justen <jordan.l.justen at intel.com>
wrote:

> On 2016-03-17 01:04:27, Kenneth Graunke wrote:
> > Yuanhan Liu decided these were useful for linear filtering in
> > commit 76669381 (circa 2011).  Prior to that, we never set them;
> > it seems he tried to preserve that behavior for nearest filtering.
> >
> > It turns out they're useful for nearest filtering, too: setting
> > these fixes the following dEQP-GLES3 tests:
> >
> > functional.fbo.blit.rect.nearest_consistency_mag
> > functional.fbo.blit.rect.nearest_consistency_mag_reverse_src_x
> > functional.fbo.blit.rect.nearest_consistency_mag_reverse_src_y
> > functional.fbo.blit.rect.nearest_consistency_mag_reverse_dst_x
> > functional.fbo.blit.rect.nearest_consistency_mag_reverse_dst_y
> > functional.fbo.blit.rect.nearest_consistency_mag_reverse_src_dst_x
> > functional.fbo.blit.rect.nearest_consistency_mag_reverse_src_dst_y
> > functional.fbo.blit.rect.nearest_consistency_min
> > functional.fbo.blit.rect.nearest_consistency_min_reverse_src_x
> > functional.fbo.blit.rect.nearest_consistency_min_reverse_src_y
> > functional.fbo.blit.rect.nearest_consistency_min_reverse_dst_x
> > functional.fbo.blit.rect.nearest_consistency_min_reverse_dst_y
> > functional.fbo.blit.rect.nearest_consistency_min_reverse_src_dst_x
> > functional.fbo.blit.rect.nearest_consistency_min_reverse_src_dst_y
> > functional.fbo.blit.rect.nearest_consistency_out_of_bounds_mag
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_mag_reverse_src_x
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_mag_reverse_src_y
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_mag_reverse_dst_x
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_mag_reverse_dst_y
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_mag_reverse_src_dst_x
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_mag_reverse_src_dst_y
> > functional.fbo.blit.rect.nearest_consistency_out_of_bounds_min
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_min_reverse_src_x
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_min_reverse_src_y
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_min_reverse_dst_x
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_min_reverse_dst_y
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_min_reverse_src_dst_x
> >
> functional.fbo.blit.rect.nearest_consistency_out_of_bounds_min_reverse_src_dst_y
> >
> > Apparently, BLORP has always set these bits unconditionally.
> >
> > However, setting them unconditionally appears to regress tests using
> > texture projection, 3D samplers, integer formats, and vertex shaders,
> > all in combination, such as:
> >
> > functional.shaders.texture_functions.textureprojlod.isampler3d_vertex
> >
> > Setting them on Gen4-5 appears to regress Piglit's
> > tests/spec/arb_sampler_objects/framebufferblit.
> >
> > Honestly, it looks like the real problem here is a lack of precision.
> > I'm just hacking around problems here (as embarassing as it is).
> >
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/mesa/drivers/dri/i965/brw_sampler_state.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_sampler_state.c
> b/src/mesa/drivers/dri/i965/brw_sampler_state.c
> > index c20a028..d53cb5d 100644
> > --- a/src/mesa/drivers/dri/i965/brw_sampler_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_sampler_state.c
> > @@ -436,14 +436,17 @@ brw_update_sampler_state(struct brw_context *brw,
> >        }
> >     }
> >
> > -   /* Set address rounding bits if not using nearest filtering. */
> > +   /* Set address rounding bits.  The conditions are empirically
> > +    * derived in order to pass test cases.
> > +    */
> > +   bool round_nearest = brw->gen >= 6 && target != GL_TEXTURE_3D;
> >     unsigned address_rounding = 0;
> > -   if (min_filter != BRW_MAPFILTER_NEAREST) {
> > +   if (min_filter != BRW_MAPFILTER_NEAREST || round_nearest) {
>

I feel the need to state, for the record, that this is super-sketchy.  Then
again, given that GL doesn't really specify what we're supposed to do on
the edges of a NEAREST sampling, it doesn't much matter.  That's explcitly
*not* a NAK, just a comment on the sketcyness of the whole thing.

I don't think it's too sketchy to merge, but it may be a good candidate for
the "pass dEQP" branch?


>
> How about folding the min_filter != BRW_MAPFILTER_NEAREST test into
> round_nearest?
>
> Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
>
> >        address_rounding |= BRW_ADDRESS_ROUNDING_ENABLE_U_MIN |
> >                            BRW_ADDRESS_ROUNDING_ENABLE_V_MIN |
> >                            BRW_ADDRESS_ROUNDING_ENABLE_R_MIN;
> >     }
> > -   if (mag_filter != BRW_MAPFILTER_NEAREST) {
> > +   if (mag_filter != BRW_MAPFILTER_NEAREST || round_nearest) {
> >        address_rounding |= BRW_ADDRESS_ROUNDING_ENABLE_U_MAG |
> >                            BRW_ADDRESS_ROUNDING_ENABLE_V_MAG |
> >                            BRW_ADDRESS_ROUNDING_ENABLE_R_MAG;
> > --
> > 2.7.3
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160317/949e300a/attachment-0001.html>