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 <imirkin@alum.mit.edu>

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
index c64be0a348f8defb38d6f48ee05b74258f01c4fa..7a2402d72eda9c47eb890d5e1715d32012d3703c 100644 (file)
--- 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 5f6301498367c9163bba76ad9e2a47e44ea60949..2b38fe6e7f575a0d472f7ef349deac863e638134 100644 (file)
--- 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);