[Mesa-dev] [PATCH 4/9] anv: Added support for non-dynamic sample locations on Gen8+

[Jason Ekstrand]

On Wed, Mar 13, 2019 at 8:43 AM Eleni Maria Stea <estea at igalia.com> wrote:

> On Wed, 13 Mar 2019 08:16:10 -0500
> Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> > On Mon, Mar 11, 2019 at 10:05 AM Eleni Maria Stea <estea at igalia.com>
> > wrote:
> >
> > > Allowing the user to set custom sample locations non-dynamically, by
> > > filling the extension structs and chaining them to the pipeline
> > > structs according to the Vulkan specification section [26.5. Custom
> > > Sample Locations]
>
> [...]
>
> > > +void
> > > +anv_calc_sample_locations(struct anv_sample *samples,
> > > +                          uint32_t num_samples,
> > > +                          const VkSampleLocationsInfoEXT *info)
> > > +{
> > > +   int i;
> > > +
> > > +   for(i = 0; i < num_samples; i++) {
> > > +      float dx, dy;
> > > +
> > > +      /* this is because the grid is 1x1, in case that
> > > +       * we support different grid sizes in the future
> > > +       * this must be changed.
> > > +       */
> > > +      samples[i].offs_x = info->pSampleLocations[i].x;
> > > +      samples[i].offs_y = info->pSampleLocations[i].y;
> > > +
> > > +      /* distance from the center */
> > > +      dx = samples[i].offs_x - 0.5;
> > > +      dy = samples[i].offs_y - 0.5;
> > > +
> > > +      samples[i].radius = dx * dx + dy * dy;
> > > +   }
> > > +
> > > +   qsort(samples, num_samples, sizeof *samples, compare_samples);
> > >
> >
> > Are we allowed to re-order the samples like this?  The spec says:
> >
> > The sample location for sample i at the pixel grid location (x,y) is
> > taken from pSampleLocations[(x + y * sampleLocationGridSize.width) *
> > sampleLocationsPerPixel + i]
> >
> > Which leads me to think that they expect the ordering of samples to be
> > respected.  Yes, I know the HW docs say we're supposed to order them
> > from nearest to furthest.  However, AFAIK, that's only so we get nice
> > centroids and I don't know that it's actually required.
> >
> > --Jason
>
> I wasn't sure about this to be honest. I could remove the qsort and
> explain why we decided to ignore the PRM in a comment for the case that
> someone decides to put this back in the future.
>

I think we're better off ignoring the hardware and adding a comment
something like ths:

The Skylake PRM Vol. 2a "3DSTATE_SAMPLE_PATTERN" says:

   "When programming the sample offsets (for NUMSAMPLES_4 or _8 and
MSRASTMODE_xxx_PATTERN), the
   order of the samples 0 to 3 (or 7 for 8X, or 15 for 16X) must have
monotonically increasing distance from the
   pixel center. This is required to get the correct centroid computation
in the device."

However, the Vulkan spec seems to require that the the samples occur in the
order provided through the API.  The standard sample patterns have the
above property that they have monotonically increasing distances from the
center but client-provided ones do not.  As long as this only affects
centroid calculations as the docs say, we should be ok because OpenGL and
Vulkan only require that the centroid be some lit sample and that it's the
same for all samples in a pixel; they have no requirement that it be the
one closest to center.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190313/cd33986b/attachment-0001.html>