<div dir="ltr"> On Wed, Sep 9, 2015 at 11:30 AM, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> 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. ><div>Resending it with link to the image in place of big attachment.</div><div> 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:<a href="https://www.dropbox.com/s/z4ebhx0lwfmui8m/scaled-blit.png?dl=0" target="_blank"> https://www.dropbox.com/s/z4ebhx0lwfmui8m/scaled-blit.png?dl=0</a> > On 09/09/2015 11:16 AM, Ian Romanick wrote: >> On 09/09/2015 11:13 AM, Ian Romanick wrote: >>> From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>> >>> >>> 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 <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>> >>> Cc: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>> >>> Cc: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com" target="_blank">topi.pohjolainen@intel.com</a>> >>> Cc: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com" target="_blank">jordan.l.justen@intel.com</a>> >>> --- >>> 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 >> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> >> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> >> > > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> ></div></div>