From 838118462e63745ae70e05b42259f2aa4f81157a Mon Sep 17 00:00:00 2001 From: Ilia Mirkin Date: Sun, 29 Dec 2019 21:50:34 -0500 Subject: [PATCH] nv50,nvc0: fix destination coordinates of blit The fix was found by Karol Herbst a long time ago, but it was unclear why it helped or if it would create additional problems. This change adds a comment that explains what's going on, and in the process also normalizes the nv50 implementation to match. The coordinates which are fed to gl_Position map directly to pixel coordinates, since the viewport transform is disabled. If the framebuffer is MSAA, then that doesn't affect the pixel coordinates at all, it's just that each pixel has multiple samples. Note that this makes it really clear that this approach is inappropriate for EXT_framebuffer_multisample_blit_scaled, and also the 3d path will fail terribly for direct copies. Thankfully the 2d path normally takes care of this. Fixes KHR-GL43.packed_depth_stencil.blit.depth32f_stencil8 as well as scaling issues in a number of EXT_framebuffer_multisample-related piglit tests (although they continue to fail due to inaccuracies). Signed-off-by: Ilia Mirkin --- src/gallium/drivers/nouveau/nv50/nv50_surface.c | 12 ++++-------- src/gallium/drivers/nouveau/nvc0/nvc0_surface.c | 16 ++++++++++++++-- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c b/src/gallium/drivers/nouveau/nv50/nv50_surface.c index c64be0a348f..7a2402d72ed 100644 --- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c +++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c @@ -1356,7 +1356,6 @@ nv50_blit_3d(struct nv50_context *nv50, const struct pipe_blit_info *info) float x0, x1, y0, y1, z; float dz; float x_range, y_range; - float tri_x, tri_y; blit->mode = nv50_blit_select_mode(info); blit->color_mask = nv50_blit_derive_color_mask(info); @@ -1377,14 +1376,11 @@ nv50_blit_3d(struct nv50_context *nv50, const struct pipe_blit_info *info) x_range = (float)info->src.box.width / (float)info->dst.box.width; y_range = (float)info->src.box.height / (float)info->dst.box.height; - tri_x = 16384 << nv50_miptree(dst)->ms_x; - tri_y = 16384 << nv50_miptree(dst)->ms_y; - x0 = (float)info->src.box.x - x_range * (float)info->dst.box.x; y0 = (float)info->src.box.y - y_range * (float)info->dst.box.y; - x1 = x0 + tri_x * x_range; - y1 = y0 + tri_y * y_range; + x1 = x0 + 16384.0f * x_range; + y1 = y0 + 16384.0f * y_range; x0 *= (float)(1 << nv50_miptree(src)->ms_x); x1 *= (float)(1 << nv50_miptree(src)->ms_x); @@ -1457,7 +1453,7 @@ nv50_blit_3d(struct nv50_context *nv50, const struct pipe_blit_info *info) PUSH_DATAf(push, y0); PUSH_DATAf(push, z); BEGIN_NV04(push, NV50_3D(VTX_ATTR_2F_X(0)), 2); - PUSH_DATAf(push, tri_x); + PUSH_DATAf(push, 16384.0f); PUSH_DATAf(push, 0.0f); BEGIN_NV04(push, NV50_3D(VTX_ATTR_3F_X(1)), 3); PUSH_DATAf(push, x0); @@ -1465,7 +1461,7 @@ nv50_blit_3d(struct nv50_context *nv50, const struct pipe_blit_info *info) PUSH_DATAf(push, z); BEGIN_NV04(push, NV50_3D(VTX_ATTR_2F_X(0)), 2); PUSH_DATAf(push, 0.0f); - PUSH_DATAf(push, tri_y); + PUSH_DATAf(push, 16384.0f); BEGIN_NV04(push, NV50_3D(VERTEX_END_GL), 1); PUSH_DATA (push, 0); } diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c index 5f630149836..2b38fe6e7f5 100644 --- a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c @@ -1276,6 +1276,18 @@ nvc0_blit_3d(struct nvc0_context *nvc0, const struct pipe_blit_info *info) * render target, with scissors defining the destination region. * The vertex is supplied with non-normalized texture coordinates * arranged in a way to yield the desired offset and scale. + * + * Note that while the source texture is presented to the sampler as + * non-MSAA (even if it is), the destination texture is treated as MSAA for + * rendering. This means that + * - destination coordinates shouldn't be scaled + * - without per-sample rendering, the target will be a solid-fill for all + * of the samples + * + * The last point implies that this process is very bad for 1:1 blits, as + * well as scaled blits between MSAA surfaces. This works fine for + * upscaling and downscaling though. The 1:1 blits should ideally be + * handled by the 2d engine, which can do it perfectly. */ minx = info->dst.box.x; @@ -1364,14 +1376,14 @@ nvc0_blit_3d(struct nvc0_context *nvc0, const struct pipe_blit_info *info) *(vbuf++) = fui(y0); *(vbuf++) = fui(z); - *(vbuf++) = fui(32768 << nv50_miptree(dst)->ms_x); + *(vbuf++) = fui(32768.0f); *(vbuf++) = fui(0.0f); *(vbuf++) = fui(x1); *(vbuf++) = fui(y0); *(vbuf++) = fui(z); *(vbuf++) = fui(0.0f); - *(vbuf++) = fui(32768 << nv50_miptree(dst)->ms_y); + *(vbuf++) = fui(32768.0f); *(vbuf++) = fui(x0); *(vbuf++) = fui(y1); *(vbuf++) = fui(z); -- 2.30.2