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

[Anuj Phogat]

On Wed, Sep 9, 2015 at 11:30 AM, 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.
>
Resending it with link to the image in place of big attachment.

Paul Berry and I agreed to follow the "NVIDIA Implementation Details"
given in the extension spec. i965 driver currently use bilinear filtering
for both SCALED_RESOLVE_FASTEST and SCALED_RESOLVE_NICEST.
Do you think the implementation doesn't match what is suggested in
the spec? I'll be happy to work on improving the implementation if you
have any specific comments.

Extension spec mentions the potential loss of quality when using a
single pass scaled resolve blit. Also the quality of image generated
depends on sample count and scaling factor. I vaguely remember
Paul mentioned the scaling factor of ~1.5, beyond which single pass
scaled resolve blit quickly becomes useless. I think the attached
image is using a scaling factor of 2.4.

Below link shows an old image generated by me doing the quality
comparison with Nvidia's proprietary driver implementation. I posted
the image on mesa-dev before enabling the extension on i965. Please
see the images under "New implementation" for the comparison.
Image: https://www.dropbox.com/s/z4ebhx0lwfmui8m/scaled-blit.png?dl=0

> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150909/c2e30fec/attachment.html>