[Mesa-dev] [PATCH v3] virgl: Add support for glGetMultisample

Erik Faye-Lund kusmabite at gmail.com
Thu Jun 28 15:40:58 UTC 2018


On Thu, Jun 28, 2018 at 5:31 PM Gert Wollny <gert.wollny at collabora.com> wrote:
>
> Am Donnerstag, den 28.06.2018, 17:09 +0200 schrieb Erik Faye-Lund:
> > Unless I'm misunderstanding, this seems to indicate that the hardware
> > has a fixed set of sample positions, which I don't think is true for
> > most hardware. Instead, the sample locations is a function of the
> > multisampling mode for a given surface...
> There are two aspects:
>
> For each number of samples there is indeed a fixes set of sample
> positions that only depends on the hardware. The set corresponding to
> the requested number of samples is used when the surface is created
> with GL_TRUE for the "fixedsamplelocations" parameter.

What I'm trying to say is that the concept of a global,
hardware-dependent set of sample-positions isn't a thing.

For instance, a single-sample multi-sample mode usually has the first
(and only sample) in the center of the pixel, whereas a 4xMSAA pattern
usually has four samples in a rotated grid, none of them in the center
of the pixel.

These are not the same, and querying the positions of the highest
sample-count mode isn't going to apply to any other mode. There simply
isn't a concept of "the hardware's sample locations". They depend on
the MSAA mode (effectively sample-count). This is exactly why you need
to create a dummy FBO to query this; you need to know the sample count
of the mode to answer this.

The fixedsamplelocations=false complication is entirely orthogonal to this.

> If this parameter is set to false, then all bets are off and the sample
> positions can even vary over the surface area. For this case
> glGetMultisample is kind of useless, for the other case one can query
> all sets once on the host and then re-use these values (see this patch
> for the host side: https://patchwork.freedesktop.org/patch/233354/)
>
> best,
> Gert
>
> >
> > On Thu, Jun 28, 2018 at 3:45 PM Gert Wollny <gert.wollny at collabora.co
> > m> wrote:
> > >
> > > Use caps to obtain the multisample sample positions for up to 16
> > > positions and implement the according Gallium interface.
> > >
> > > Fixes (when run on GL host):
> > >     dEQP-
> > > GLES31.functional.texture.multisample.samples_1.sample_position
> > >     dEQP-
> > > GLES31.functional.texture.multisample.samples_2.sample_position
> > >     dEQP-
> > > GLES31.functional.texture.multisample.samples_3.sample_position
> > >     dEQP-
> > > GLES31.functional.texture.multisample.samples_4.sample_position
> > >     dEQP-
> > > GLES31.functional.texture.multisample.samples_8.sample_position
> > >     dEQP-
> > > GLES31.functional.texture.multisample.samples_10.sample_position
> > >     dEQP-
> > > GLES31.functional.texture.multisample.samples_12.sample_position
> > >     dEQP-
> > > GLES31.functional.texture.multisample.samples_13.sample_position
> > >     dEQP-
> > > GLES31.functional.texture.multisample.samples_16.sample_position
> > >
> > > v2: remove unrelated chunk (thanks Ilia Mirkin)
> > > v3: - also return positions for intermediate sample counts
> > >     - fix unused varible warning
> > >     - update description
> > >
> > > Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> > > ---
> > > I left the debug_printf in there, because this patch (together with
> > > the
> > > related virglrenderer patch) is not sufficient to fix above tests
> > > on a GLES
> > > host.
> > >
> > >  src/gallium/drivers/virgl/virgl_context.c | 38
> > > +++++++++++++++++++++++++++++++
> > >  src/gallium/drivers/virgl/virgl_hw.h      |  1 +
> > >  2 files changed, 39 insertions(+)
> > >
> > > diff --git a/src/gallium/drivers/virgl/virgl_context.c
> > > b/src/gallium/drivers/virgl/virgl_context.c
> > > index e6f8dc8525..43c141e42d 100644
> > > --- a/src/gallium/drivers/virgl/virgl_context.c
> > > +++ b/src/gallium/drivers/virgl/virgl_context.c
> > > @@ -920,6 +920,42 @@ virgl_context_destroy( struct pipe_context
> > > *ctx )
> > >     FREE(vctx);
> > >  }
> > >
> > > +static void virgl_get_sample_position(struct pipe_context *ctx,
> > > +                                      unsigned sample_count,
> > > +                                      unsigned index,
> > > +                                      float *out_value)
> > > +{
> > > +   struct virgl_context *vctx = virgl_context(ctx);
> > > +   struct virgl_screen *vs = virgl_screen(vctx->base.screen);
> > > +
> > > +   if (sample_count > vs->caps.caps.v1.max_samples) {
> > > +      debug_printf("VIRGL: requested %d MSAA samples, but only %d
> > > supported\n",
> > > +                   sample_count, vs->caps.caps.v1.max_samples);
> > > +      return;
> > > +   }
> > > +
> > > +   /* The following is basically copied from
> > > dri/i965gen6_get_sample_position
> > > +    * The only addition is that we hold the msaa positions for all
> > > sample
> > > +    * counts in a flat array. */
> > > +   uint32_t bits = 0;
> > > +   if (sample_count == 1) {
> > > +      out_value[0] = out_value[1] = 0.5f;
> > > +      return;
> > > +   } else if (sample_count == 2) {
> > > +      bits = vs->caps.caps.v2.msaa_sample_positions[0] >> (8 *
> > > index);
> > > +   } else if (sample_count < 4) {
> > > +      bits = vs->caps.caps.v2.msaa_sample_positions[1] >> (8 *
> > > index);
> > > +   } else if (sample_count < 8) {
> > > +      bits = vs->caps.caps.v2.msaa_sample_positions[2 + (index >>
> > > 2)] >> (8 * (index & 3));
> > > +   } else if (sample_count < 8) {
> > > +      bits = vs->caps.caps.v2.msaa_sample_positions[4 + (index >>
> > > 2)] >> (8 * (index & 3));
> > > +   }
> > > +   out_value[0] = ((bits >> 4) & 0xf) / 16.0f;
> > > +   out_value[1] = (bits & 0xf) / 16.0f;
> > > +   debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
> > > +                index, sample_count, out_value[0], out_value[1]);
> > > +}
> > > +
> > >  struct pipe_context *virgl_context_create(struct pipe_screen
> > > *pscreen,
> > >                                            void *priv,
> > >                                            unsigned flags)
> > > @@ -994,6 +1030,8 @@ struct pipe_context
> > > *virgl_context_create(struct pipe_screen *pscreen,
> > >
> > >     vctx->base.set_blend_color = virgl_set_blend_color;
> > >
> > > +   vctx->base.get_sample_position = virgl_get_sample_position;
> > > +
> > >     vctx->base.resource_copy_region = virgl_resource_copy_region;
> > >     vctx->base.flush_resource = virgl_flush_resource;
> > >     vctx->base.blit =  virgl_blit;
> > > diff --git a/src/gallium/drivers/virgl/virgl_hw.h
> > > b/src/gallium/drivers/virgl/virgl_hw.h
> > > index ee58520f9b..82cbb8aed1 100644
> > > --- a/src/gallium/drivers/virgl/virgl_hw.h
> > > +++ b/src/gallium/drivers/virgl/virgl_hw.h
> > > @@ -298,6 +298,7 @@ struct virgl_caps_v2 {
> > >          uint32_t uniform_buffer_offset_alignment;
> > >          uint32_t shader_buffer_offset_alignment;
> > >          uint32_t capability_bits;
> > > +        uint32_t msaa_sample_positions[8];
> > >  };
> > >
> > >  union virgl_caps {
> > > --
> > > 2.16.4
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list