On Fri, Jun 29, 2018 at 12:52 PM Erik Faye-Lund <kusmab...@gmail.com> wrote:
>
> On Fri, Jun 29, 2018 at 12:39 PM Gert Wollny <gert.wol...@collabora.com>
> wrote:
> >
> > Use caps to obtain the multisample sample positions for up to 16
> > positions and implement the according Gallium interface.
> >
> > This implemenation (plus its counterpart in virglrenderer) assume that
> > the fixed sample position are always the same for a given number of samples
> > over the whole live time of a qemu session. It also assumes that sample
> > series are only given for 2, 4, 8, and 16 samples, and for intermediate
> > numbers N of samples the next higher supported set from above list is picked
> > and the sample positions for the first N samples are returned accordingly.
> >
> > 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
> > v4: explain better what this patch assumes and how it handles sample numbers
> > that are not directly advertised (thanks go to Erik Faye-Lund for making
> > me aware that this should be documented)
> >
> > Signed-off-by: Gert Wollny <gert.wol...@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]);
>
> Are you sure that an unconditional debug_printf in the success case
> here is a good idea?
>
> AFAIK, most debug_printfs are either in error paths, or guarded by a
> compile-time flag, a run-time flag (typically environment variables)
> or something like that.
>
> The most kosher way of doing this would be with a driver-specific
> debug-flag. Sadly, it seems there's no such flag in virgl yet for
> this, and maybe this is a too small detail to bother with this? If so,
> quite a lot of places just throw an #if 0 around it so it's easy to
> enable for developers in the future...
>
Silly me, that part was already explained above. Never mind me then,
that part is fine for now also!
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
