[Mesa-dev] [PATCH v2] mesa/meta: Use interpolateAtOffset for 16x MSAA copy blit
Neil Roberts
neil at linux.intel.com
Tue Sep 29 05:57:44 PDT 2015
Previously there was a problem in i965 where if 16x MSAA is used then
some of the sample positions are exactly on the 0 x or y axis. When
the MSAA copy blit shader interpolates the texture coordinates at
these sample positions it was possible that it would jump to a
neighboring texel due to rounding errors. It is likely that these
positions would be used on 16x MSAA because that is where they are
defined to be in D3D.
To fix that this patch makes it use interpolateAtOffset in the blit
shader whenever 16x MSAA is used and the GL_ARB_gpu_shader5 extension
is available. This forces it to interpolate the texture coordinates at
the pixel center to avoid these problematic positions.
This fixes ext_framebuffer_multisample-unaligned-blit and
ext_framebuffer_multisample-clip-and-scissor-blit with 16x MSAA on
SKL+.
---
src/mesa/drivers/common/meta_blit.c | 64 ++++++++++++++++++++++++++++++-------
1 file changed, 52 insertions(+), 12 deletions(-)
diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c
index 1f9515a..e812ecb 100644
--- a/src/mesa/drivers/common/meta_blit.c
+++ b/src/mesa/drivers/common/meta_blit.c
@@ -352,17 +352,27 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,
shader_index == BLIT_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_DEPTH_COPY ||
shader_index == BLIT_MSAA_SHADER_2D_MULTISAMPLE_DEPTH_COPY) {
char *sample_index;
- const char *arb_sample_shading_extension_string;
+ const char *extra_extensions;
+ const char *tex_coords = "texCoords";
if (dst_is_msaa) {
- arb_sample_shading_extension_string = "#extension GL_ARB_sample_shading : enable";
sample_index = "gl_SampleID";
name = "depth MSAA copy";
+
+ if (ctx->Extensions.ARB_gpu_shader5 && samples >= 16) {
+ extra_extensions =
+ "#extension GL_ARB_sample_shading : enable\n"
+ "#extension GL_ARB_gpu_shader5 : enable\n";
+ /* See comment below for the color copy */
+ tex_coords = "interpolateAtOffset(texCoords, vec2(0.0))";
+ } else {
+ extra_extensions = "#extension GL_ARB_sample_shading : enable\n";
+ }
} else {
- /* Don't need that extension, since we're drawing to a single-sampled
- * destination.
+ /* Don't need any extra extensions, since we're drawing to a
+ * single-sampled destination.
*/
- arb_sample_shading_extension_string = "";
+ extra_extensions = "";
/* From the GL 4.3 spec:
*
* "If there is a multisample buffer (the value of SAMPLE_BUFFERS
@@ -399,27 +409,57 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,
"\n"
"void main()\n"
"{\n"
- " gl_FragDepth = texelFetch(texSampler, i%s(texCoords), %s).r;\n"
+ " gl_FragDepth = texelFetch(texSampler, i%s(%s), %s).r;\n"
"}\n",
- arb_sample_shading_extension_string,
+ extra_extensions,
sampler_array_suffix,
texcoord_type,
texcoord_type,
+ tex_coords,
sample_index);
} else {
/* You can create 2D_MULTISAMPLE textures with 0 sample count (meaning 1
* sample). Yes, this is ridiculous.
*/
char *sample_resolve;
- const char *arb_sample_shading_extension_string;
+ const char *extra_extensions;
const char *merge_function;
name = ralloc_asprintf(mem_ctx, "%svec4 MSAA %s",
vec4_prefix,
dst_is_msaa ? "copy" : "resolve");
if (dst_is_msaa) {
- arb_sample_shading_extension_string = "#extension GL_ARB_sample_shading : enable";
- sample_resolve = ralloc_asprintf(mem_ctx, " out_color = texelFetch(texSampler, i%s(texCoords), gl_SampleID);", texcoord_type);
+ const char *tex_coords;
+
+ if (ctx->Extensions.ARB_gpu_shader5 && samples >= 16) {
+ /* If interpolateAtOffset is available then it will be used to
+ * force the interpolation to the center. This is required at
+ * least on Intel hardware because it is possible to have a sample
+ * position on the 0 x or y axis which means it will lie exactly
+ * on the pixel boundary. If we let the hardware interpolate the
+ * coordinates at one of these positions then it is possible for
+ * it to jump to a neighboring texel when converting to ints due
+ * to rounding errors. This is only done for >= 16x MSAA because
+ * it probably has some overhead. It is more likely that some
+ * hardware will use one of these problematic positions at 16x
+ * MSAA because in that case in D3D they are defined to be at
+ * these positions.
+ */
+ extra_extensions =
+ "#extension GL_ARB_sample_shading : enable\n"
+ "#extension GL_ARB_gpu_shader5 : enable\n";
+ tex_coords = "interpolateAtOffset(texCoords, vec2(0.0))";
+ } else {
+ extra_extensions = "#extension GL_ARB_sample_shading : enable\n";
+ tex_coords = "texCoords";
+ }
+
+ sample_resolve =
+ ralloc_asprintf(mem_ctx,
+ " out_color = texelFetch(texSampler, "
+ "i%s(%s), gl_SampleID);",
+ texcoord_type, tex_coords);
+
merge_function = "";
} else {
int i;
@@ -434,7 +474,7 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,
"vec4 merge(vec4 a, vec4 b) { return (a + b); }\n";
}
- arb_sample_shading_extension_string = "";
+ extra_extensions = "";
/* We're assuming power of two samples for this resolution procedure.
*
@@ -502,7 +542,7 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,
"{\n"
"%s\n" /* sample_resolve */
"}\n",
- arb_sample_shading_extension_string,
+ extra_extensions,
vec4_prefix,
vec4_prefix,
sampler_array_suffix,
--
1.9.3
More information about the mesa-dev
mailing list