[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 > > > > _