[Mesa-dev] [PATCH] intel: Switch the order of the 2x MSAA sample positions

Jason Ekstrand jason at jlekstrand.net
Sat Aug 11 16:01:15 UTC 2018


On Thu, Aug 9, 2018 at 3:03 PM Anuj Phogat <anuj.phogat at gmail.com> wrote:

> On Wed, Aug 8, 2018 at 11:31 AM Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> > The Vulkan 1.1.82 spec flipped the order to better match D3D.
> >
> > Cc: mesa-stable at lists.freedesktop.org
> > ---
> >  src/intel/blorp/blorp_blit.c                       | 11 ++++++++++-
> >  src/intel/common/gen_sample_positions.h            |  8 ++++----
> >  src/mesa/drivers/dri/i965/brw_multisample_state.h  |  8 ++++----
> >  src/mesa/drivers/dri/i965/gen6_multisample_state.c |  4 ++--
> >  4 files changed, 20 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/intel/blorp/blorp_blit.c b/src/intel/blorp/blorp_blit.c
> > index 561897894c3..013f7a14fa2 100644
> > --- a/src/intel/blorp/blorp_blit.c
> > +++ b/src/intel/blorp/blorp_blit.c
> > @@ -776,6 +776,13 @@ blorp_nir_manual_blend_bilinear(nir_builder *b,
> nir_ssa_def *pos,
> >         * grid of samples with in a pixel. Sample number layout shows the
> >         * rectangular grid of samples roughly corresponding to the real
> sample
> >         * locations with in a pixel.
> > +       *
> > +       * In the case of 2x MSAA, the layout of sample indices is
> reversed from
> > +       * the layout of sample numbers:
> It is not clear from this comment if below layout is for sample index or
> sample
> number. Adding "sample number layout:" on top of it will help.
>

I've updated it to look more like the 8x case where we have two tables.


> > +       *           ---------
> > +       *           | 1 | 0 |
> > +       *           ---------
> > +       *
> >         * In case of 4x MSAA, layout of sample indices matches the
> layout of
> >         * sample numbers:
> >         *           ---------
> > @@ -819,7 +826,9 @@ blorp_nir_manual_blend_bilinear(nir_builder *b,
> nir_ssa_def *pos,
> >                                              key->x_scale *
> key->y_scale));
> >        sample = nir_f2i32(b, sample);
> >
> > -      if (tex_samples == 8) {
> > +      if (tex_samples == 2) {
> > +         sample = nir_isub(b, nir_imm_int(b, 1), sample);
> > +      } else if (tex_samples == 8) {
> >           sample = nir_iand(b, nir_ishr(b, nir_imm_int(b, 0x64210573),
> >                                         nir_ishl(b, sample,
> nir_imm_int(b, 2))),
> >                             nir_imm_int(b, 0xf));
> > diff --git a/src/intel/common/gen_sample_positions.h
> b/src/intel/common/gen_sample_positions.h
> > index f0ce95dd1fb..da48dcb5ed0 100644
> > --- a/src/intel/common/gen_sample_positions.h
> > +++ b/src/intel/common/gen_sample_positions.h
> > @@ -42,10 +42,10 @@ prefix##0YOffset   = 0.5;
> >   * c   1
> >   */
> >  #define GEN_SAMPLE_POS_2X(prefix) \
> > -prefix##0XOffset   = 0.25; \
> > -prefix##0YOffset   = 0.25; \
> > -prefix##1XOffset   = 0.75; \
> > -prefix##1YOffset   = 0.75;
> > +prefix##0XOffset   = 0.75; \
> > +prefix##0YOffset   = 0.75; \
> > +prefix##1XOffset   = 0.25; \
> > +prefix##1YOffset   = 0.25;
> >
> >  /**
> >   * Sample positions:
> > diff --git a/src/mesa/drivers/dri/i965/brw_multisample_state.h
> b/src/mesa/drivers/dri/i965/brw_multisample_state.h
> > index 6cf324e561c..2142a17a484 100644
> > --- a/src/mesa/drivers/dri/i965/brw_multisample_state.h
> > +++ b/src/mesa/drivers/dri/i965/brw_multisample_state.h
> > @@ -38,13 +38,13 @@
> >  /**
> >   * 1x MSAA has a single sample at the center: (0.5, 0.5) -> (0x8, 0x8).
> >   *
> > - * 2x MSAA sample positions are (0.25, 0.25) and (0.75, 0.75):
> > + * 2x MSAA sample positions are (0.75, 0.75) and (0.25, 0.25):
> >   *   4 c
> > - * 4 0
> > - * c   1
> > + * 4 1
> > + * c   0
> >   */
> >  static const uint32_t
> > -brw_multisample_positions_1x_2x = 0x0088cc44;
> > +brw_multisample_positions_1x_2x = 0x008844cc;
> >
> >  /**
> >   * Sample positions:
> > diff --git a/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> > index bfa84fb9b77..78ff3942075 100644
> > --- a/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> > @@ -70,7 +70,7 @@ gen6_get_sample_position(struct gl_context *ctx,
> >   *
> >   * 2X MSAA sample index / number layout
> You should update this comment matching it up with comment in blorp and
> add  "sample number layout:" on top of below layout. I would leave it up to
> you if you also want to draw sample index layout here just for uniformity.
>

Made the same change here.


> >   *           ---------
> > - *           | 0 | 1 |
> > + *           | 1 | 0 |
> >   *           ---------
> >   *
> >   * 4X MSAA sample index / number layout
> > @@ -107,7 +107,7 @@ gen6_get_sample_position(struct gl_context *ctx,
> >  void
> >  gen6_set_sample_maps(struct gl_context *ctx)
> >  {
> > -   uint8_t map_2x[2] = {0, 1};
> > +   uint8_t map_2x[2] = {1, 0};
> >     uint8_t map_4x[4] = {0, 1, 2, 3};
> >     uint8_t map_8x[8] = {3, 7, 5, 0, 1, 2, 4, 6};
> >     uint8_t map_16x[16] = { 15, 10, 9, 7, 4, 1, 3, 13,
> > --
> > 2.17.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> With above changes
> Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>
>

Thanks!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180811/30b2af74/attachment-0001.html>


More information about the mesa-dev mailing list