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

[Ilia Mirkin]

In case it's of any interest, nouveau fails many of the blit-scaled
tests. However nouveau's MS blits are notoriously bad. [I think I know
what's wrong, just don't have the energy to fix it.]

OTOH the NVIDIA driver also fails at every scale factor (for 2x and
8x, passes for 4x).

On Wed, Sep 9, 2015 at 2:30 PM, Ian Romanick <idr at freedesktop.org> wrote:
> I'm pretty sure our implementation of this extension is complete
> rubbish.  I have attached an image from the blit-scaled test.  This
> result cannot be useful to anyone.
>
> On 09/09/2015 11:16 AM, Ian Romanick wrote:
>> On 09/09/2015 11:13 AM, Ian Romanick 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.
>>
>> I just noticed that the piglit test contains the same incorrect shader
>> as meta for the "reference" implementation.
>>
>>> 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"
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>