[Mesa-dev] [PATCH v3] virgl: Add support for glGetMultisample
Gert Wollny
gert.wollny at collabora.com
Thu Jun 28 15:54:14 UTC 2018
Am Donnerstag, den 28.06.2018, 17:40 +0200 schrieb Erik Faye-Lund:
> On Thu, Jun 28, 2018 at 5:31 PM Gert Wollny <gert.wollny at collabora.co
> m> wrote:
> >
> > 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.
I'm not sure what you are getting at ...
>
> 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.
And this is exactly what I'm doing on the host (see the patch against
virglrenderer I linked before) to get the values into
vs->caps.caps.v2.msaa_sample_positions: I go through the supported
sample counts 2, 4, 8, up to 16 (if supported), store the positions,
and pass them to the guest. I looked at the radeonsi, r600, and the
intel driver, and they all define sample position sets for these sample
numbers (only up to 8 for r600). If one requests a number that is not
amongst these, then the sample positions for the next higher sample
count are returned, (eg. for 13 sample one gets the first 13 positions
for 16 samples).
Best,
Gert
>
> 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 collabor
> > > a.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_positio
> > > > n
> > > > dEQP-
> > > > GLES31.functional.texture.multisample.samples_12.sample_positio
> > > > n
> > > > dEQP-
> > > > GLES31.functional.texture.multisample.samples_13.sample_positio
> > > > n
> > > > dEQP-
> > > > GLES31.functional.texture.multisample.samples_16.sample_positio
> > > > n
> > > >
> > > > 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