[Mesa-dev] [PATCH 1/4] i965/fs: Use sample interpolation for interpolateAtCentroid in persample mode

[Jason Ekstrand]

On Wed, Sep 14, 2016 at 1:29 PM, Anuj Phogat <anuj.phogat at gmail.com> wrote:

> On Wed, Sep 14, 2016 at 10:45 AM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > From the ARB_gpu_shader5 spec:
> >
> >    The built-in functions interpolateAtCentroid() and
> interpolateAtSample()
> >    will sample variables as though they were declared with the "centroid"
> >    or "sample" qualifiers, respectively.
> >
> > When running with persample dispatch forced by the API, we interpolate
> > anything that isn't flat as if it's qualified by "sample".  In order to
> > keep interpolateAtCentroid() consistent with the "centroid" qualifier, we
> > need to make interpolateAtCentroid() do sample interpolation instead.
> > Nothing in the GLSL spec guarantees that the result of
> > interpolateAtCentroid is uniform across samples in any way, so this is a
> > perfectly fine thing to do.
> >
> This explanation sounds good to me. To be consistent with what
> we do in case of per sample interpolation, shouldn't we do sample
> interpolation in case of InterpolateAtOffset() too? This series
> doesn't seem to include it.
>

No.  interpolateAtOffset ask that the input be interpolated at a particular
offset relative to the pixel center.  I believe we have to respect that.


> > Fixes 8 of the new dEQP-VK.pipeline.multisample_interpolation.* Vulkan
> CTS
> > tests that specifically validate consistency between the "sample"
> qualifier
> > and interpolateAtSample()
> >
> > Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs.cpp | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > index 75642d3..9dbb699 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > @@ -6497,6 +6497,32 @@ brw_nir_set_default_interpolation(const struct
> gen_device_info *devinfo,
> >           var->data.sample = false;
> >        }
> >     }
> > +
> > +   if (per_sample_interpolation) {
> > +      nir_foreach_block(block, nir_shader_get_entrypoint(nir)) {
> > +         nir_foreach_instr(instr, block) {
> > +            if (instr->type != nir_instr_type_intrinsic)
> > +               continue;
> > +
> > +            nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
> > +            if (intrin->intrinsic != nir_intrinsic_interp_var_at_
> centroid)
> > +               continue;
> > +
> > +            nir_variable *var = intrin->variables[0]->var;
> > +            if (var->data.interpolation == INTERP_MODE_FLAT)
> > +               continue;
> > +
> > +            /* The description of the interpolateAtCentroid intrinsic
> is that
> > +             * it interpolates the variable as if it had the "centroid"
> > +             * qualifier.  When executing with
> per_sample_interpolation, this
> > +             * is equivalent to having the "sample" qualifier.  Just
> convert
> > +             * it to a load_var instead.
> > +             */
> > +            assert(var->data.sample);
> > +            intrin->intrinsic = nir_intrinsic_load_var;
> > +         }
> > +      }
> > +   }
> >  }
> >
> >  /**
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > 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/20160914/1719dc99/attachment.html>