On Thu, Jun 28, 2018 at 5:31 PM Gert Wollny <gert.wol...@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.wol...@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.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]); > > > +} > > > + > > > 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@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev