[Mesa-dev] [PATCH] intel: Switch the order of the 2x MSAA sample positions
The Vulkan 1.1.82 spec flipped the order to better match D3D.
Cc: mesa-sta...@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:
+ * -
+ * | 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
* -
- * | 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: Switch the order of the 2x MSAA sample positions
Reviewed-by: Iago Toral Quiroga
On Wed, 2018-08-08 at 11:30 -0700, Jason Ekstrand wrote:
> The Vulkan 1.1.82 spec flipped the order to better match D3D.
>
> Cc: mesa-sta...@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:
> + * -
> + * | 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
> * -
> - * | 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,
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: Switch the order of the 2x MSAA sample positions
On Wed, Aug 8, 2018 at 11:31 AM Jason Ekstrand wrote:
>
> The Vulkan 1.1.82 spec flipped the order to better match D3D.
>
> Cc: mesa-sta...@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.
> + * -
> + * | 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.
> * -
> - * | 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
With above changes
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: Switch the order of the 2x MSAA sample positions
On Thu, Aug 9, 2018 at 3:03 PM Anuj Phogat wrote:
> On Wed, Aug 8, 2018 at 11:31 AM Jason Ekstrand
> wrote:
> >
> > The Vulkan 1.1.82 spec flipped the order to better match D3D.
> >
> > Cc: mesa-sta...@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
> >
> > _
