On Thu, Sep 24, 2015 at 8:07 AM, Neil Roberts <n...@linux.intel.com> wrote:
>
> v2: Fix the x_scale in the shader. Remove the doubts in the commit
> message.
> ---
>
> After some helpful explanation from Anuj and reading the code a bit
> more, I think I understand this a bit better and I no longer think
> there is an issue with the sample map array having out-of-bounds
> indices. The texture coordinates are effectively multiplied by
> ‘scale’, converted to integers and then divided by scale again. That
> means that fract(coord) can effectively only be a multiple of
> 1.0/scale. In the 8x MSAA case, 1.0/scale is vec2(0.5f, 0.25f) which
> means the maximum array index is int(0.5*2.0 + 0.75*8.0) == 7.
>
> In this v2 of the patch I've fixed scale.x in the shader. Now for the
> 16x MSAA case 1.0/scale is vec2(0.25f,0.25f). I've left the formula
> for the array index the same as v1 so the maximum index is now
> int(0.75*4.0 + 0.75*16) == 15 which makes sense.
>
> src/mesa/drivers/common/meta.h | 2 ++
> src/mesa/drivers/common/meta_blit.c | 29
> ++++++++++++++--------
> src/mesa/drivers/dri/i965/gen6_multisample_state.c | 14 +++++++++++
> src/mesa/main/mtypes.h | 15 ++++++++++-
> 4 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h
> index fe43915..e42ddc8 100644
> --- a/src/mesa/drivers/common/meta.h
> +++ b/src/mesa/drivers/common/meta.h
> @@ -285,9 +285,11 @@ enum blit_msaa_shader {
> BLIT_2X_MSAA_SHADER_2D_MULTISAMPLE_SCALED_RESOLVE,
> BLIT_4X_MSAA_SHADER_2D_MULTISAMPLE_SCALED_RESOLVE,
> BLIT_8X_MSAA_SHADER_2D_MULTISAMPLE_SCALED_RESOLVE,
> + BLIT_16X_MSAA_SHADER_2D_MULTISAMPLE_SCALED_RESOLVE,
> BLIT_2X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_SCALED_RESOLVE,
> BLIT_4X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_SCALED_RESOLVE,
> BLIT_8X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_SCALED_RESOLVE,
> + BLIT_16X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_SCALED_RESOLVE,
> BLIT_MSAA_SHADER_COUNT,
> };
>
> diff --git a/src/mesa/drivers/common/meta_blit.c
> b/src/mesa/drivers/common/meta_blit.c
> index a41fe42..1f9515a 100644
> --- a/src/mesa/drivers/common/meta_blit.c
> +++ b/src/mesa/drivers/common/meta_blit.c
> @@ -74,20 +74,25 @@ setup_glsl_msaa_blit_scaled_shader(struct gl_context *ctx,
> const char *vs_source;
> const char *sampler_array_suffix = "";
> const char *texcoord_type = "vec2";
> - float y_scale;
> + float x_scale, y_scale;
> enum blit_msaa_shader shader_index;
>
> assert(src_rb);
> samples = MAX2(src_rb->NumSamples, 1);
> - y_scale = samples * 0.5;
> +
> + if (samples == 16)
> + x_scale = 4.0;
> + else
> + x_scale = 2.0;
> + y_scale = samples / x_scale;
>
> /* We expect only power of 2 samples in source multisample buffer. */
> assert(samples > 0 && _mesa_is_pow_two(samples));
> while (samples >> (shader_offset + 1)) {
> shader_offset++;
> }
> - /* Update the assert if we plan to support more than 8X MSAA. */
> - assert(shader_offset > 0 && shader_offset < 4);
> + /* Update the assert if we plan to support more than 16X MSAA. */
> + assert(shader_offset > 0 && shader_offset <= 4);
>
> assert(target == GL_TEXTURE_2D_MULTISAMPLE ||
> target == GL_TEXTURE_2D_MULTISAMPLE_ARRAY);
> @@ -132,6 +137,10 @@ setup_glsl_msaa_blit_scaled_shader(struct gl_context
> *ctx,
> sample_number = "sample_map[int(2 * fract(coord.x) + 8 *
> fract(coord.y))]";
> sample_map = ctx->Const.SampleMap8x;
> break;
> + case 16:
> + sample_number = "sample_map[int(4 * fract(coord.x) + 16 *
> fract(coord.y))]";
> + sample_map = ctx->Const.SampleMap16x;
> + break;
> default:
> sample_number = NULL;
> sample_map = NULL;
> @@ -178,9 +187,9 @@ setup_glsl_msaa_blit_scaled_shader(struct gl_context *ctx,
> "{\n"
> "%s"
> " vec2 interp;\n"
> - " const vec2 scale = vec2(2.0f, %ff);\n"
> - " const vec2 scale_inv = vec2(0.5f, %ff);\n"
> - " const vec2 s_0_offset = vec2(0.25f,
> %ff);\n"
> + " const vec2 scale = vec2(%ff, %ff);\n"
> + " const vec2 scale_inv = vec2(%ff, %ff);\n"
> + " const vec2 s_0_offset = vec2(%ff, %ff);\n"
> " vec2 s_0_coord, s_1_coord, s_2_coord,
> s_3_coord;\n"
> " vec4 s_0_color, s_1_color, s_2_color,
> s_3_color;\n"
> " vec4 x_0_color, x_1_color;\n"
> @@ -214,9 +223,9 @@ setup_glsl_msaa_blit_scaled_shader(struct gl_context *ctx,
> sampler_array_suffix,
> texcoord_type,
> sample_map_expr,
> - y_scale,
> - 1.0f / y_scale,
> - 1.0f / samples,
> + x_scale, y_scale,
> + 1.0f / x_scale, 1.0f / y_scale,
> + 0.5f / x_scale, 0.5f / y_scale,
> texel_fetch_macro);
>
> _mesa_meta_compile_and_link_program(ctx, vs_source, fs_source, name,
> diff --git a/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> index 49c6eba..8eb620d 100644
> --- a/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> @@ -91,6 +91,17 @@ gen6_get_sample_position(struct gl_context *ctx,
> * | 6 | 7 | | 7 | 1 |
> * --------- ---------
> *
> + * 16X MSAA sample index layout 16x MSAA sample number layout
> + * ----------------- -----------------
> + * | 0 | 1 | 2 | 3 | |15 |10 | 9 | 7 |
> + * ----------------- -----------------
> + * | 4 | 5 | 6 | 7 | | 4 | 1 | 3 |13 |
> + * ----------------- -----------------
> + * | 8 | 9 |10 |11 | |12 | 2 | 0 | 6 |
> + * ----------------- -----------------
> + * |12 |13 |14 |15 | |11 | 8 | 5 |14 |
> + * ----------------- -----------------
> + *
> * A sample map is used to map sample indices to sample numbers.
> */
> void
> @@ -99,10 +110,13 @@ gen6_set_sample_maps(struct gl_context *ctx)
> uint8_t map_2x[2] = {0, 1};
> uint8_t map_4x[4] = {0, 1, 2, 3};
> uint8_t map_8x[8] = {5, 2, 4, 6, 0, 3, 7, 1};
> + uint8_t map_16x[16] = { 15, 10, 9, 7, 4, 1, 3, 13,
> + 12, 2, 0, 6, 11, 8, 5, 14 };
>
> memcpy(ctx->Const.SampleMap2x, map_2x, sizeof(map_2x));
> memcpy(ctx->Const.SampleMap4x, map_4x, sizeof(map_4x));
> memcpy(ctx->Const.SampleMap8x, map_8x, sizeof(map_8x));
> + memcpy(ctx->Const.SampleMap16x, map_16x, sizeof(map_16x));
> }
>
> /**
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index fac45aa..e0a06f4 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3550,11 +3550,24 @@ struct gl_constants
> * below:
> * SampleMap8x = {a, b, c, d, e, f, g, h};
> *
> - * Follow the logic for other sample counts.
> + * Follow the logic for sample counts 2-8.
> + *
> + * For 16x the sample indices layout as a 4x4 grid as follows:
> + *
> + * -----------------
> + * | 0 | 1 | 2 | 3 |
> + * -----------------
> + * | 4 | 5 | 6 | 7 |
> + * -----------------
> + * | 8 | 9 |10 |11 |
> + * -----------------
> + * |12 |13 |14 |15 |
> + * -----------------
> */
> uint8_t SampleMap2x[2];
> uint8_t SampleMap4x[4];
> uint8_t SampleMap8x[8];
> + uint8_t SampleMap16x[16];
>
> /** GL_ARB_shader_atomic_counters */
> GLuint MaxAtomicBufferBindings;
> --
> 1.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat <anuj.pho...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
