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

[Gert Wollny]

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.

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