[prev in list] [next in list] [prev in thread] [next in thread]
List: mesa3d-dev
Subject: [Mesa-dev] [PATCH] nv50,nvc0: fix destination coordinates of blit
From: Ilia Mirkin <imirkin () alum ! mit ! edu>
Date: 2019-12-30 2:56:53
Message-ID: 20191230025653.27847-1-imirkin () alum ! mit ! edu
[Download RAW message or body]
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>
---
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.24.1
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic