[Mesa-dev] [PATCH] meta: Use result of texture coordinate clamping operation

[Anuj Phogat]

On Wed, Sep 9, 2015 at 11:13 AM, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Previously the result of the complicated clamp() expression just dropped
> on the floor: clamp does not modify any of its parameters.  Looking at
> the surrounding code, I believe this is supposed to modify the value of
> tex_coord.
>
> This change (along with a change to avoid the use of
> brw_blorp_framebuffer) does not affect any existing piglit tests.  I'm
> not sure what this clamp is trying to accomplish, so I'm not sure how to
> write a test to exercise this path.
>
texCoords for the src texture are offset and scaled in the fragment shader.
The resulting tex_coord need to be clamped to avoid picking samples
beyond the texture edges.

Unfortunately piglit blit scaled test don't hit this bug. I remember trying
to exercise this case but it's little tricky.

> I also noticed another bug in this code.  There is no way the array
> texture case could possibly work.  This will generate code for the
> TEXEL_FETCH macro like:
>
>     #define TEXEL_FETCH(coord) texelFetch(texSampler, ivec3(coord), sample_map[int(2 * fract(coord.x))]);
>
> Since the coord parameter of this macro is a vec2 at all invocations, no
> expansion of this macro will even compile.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: Anuj Phogat <anuj.phogat at gmail.com>
> Cc: Topi Pohjolainen <topi.pohjolainen at intel.com>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> ---
>  src/mesa/drivers/common/meta_blit.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c
> index 71d18de..a41fe42 100644
> --- a/src/mesa/drivers/common/meta_blit.c
> +++ b/src/mesa/drivers/common/meta_blit.c
> @@ -187,8 +187,8 @@ setup_glsl_msaa_blit_scaled_shader(struct gl_context *ctx,
>                                 "   vec2 tex_coord = texCoords - s_0_offset;\n"
>                                 "\n"
>                                 "   tex_coord *= scale;\n"
> -                               "   clamp(tex_coord.x, 0.0f, scale.x * src_width - 1.0f);\n"
> -                               "   clamp(tex_coord.y, 0.0f, scale.y * src_height - 1.0f);\n"
> +                               "   tex_coord.x = clamp(tex_coord.x, 0.0f, scale.x * src_width - 1.0f);\n"
> +                               "   tex_coord.y = clamp(tex_coord.y, 0.0f, scale.y * src_height - 1.0f);\n"
>                                 "   interp = fract(tex_coord);\n"
>                                 "   tex_coord = ivec2(tex_coord) * scale_inv;\n"
>                                 "\n"
> --
> 2.1.0
>

Thanks for fixing it Ian.
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>