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

[Anuj Phogat]

On Wed, Sep 14, 2016 at 1:31 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
>
> 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.
>
Series is: Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>
>>
>> > 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
>
>