[PATCH 2/4] wined3d: Test if a nudge is needed to get the correct filling convention (v2).
Stefan Dösinger
stefan at codeweavers.com
Wed Sep 29 14:42:58 CDT 2021
This fixes stray lines in GameFace GUIs, e.g. in World of Tanks.
Signed-off-by: Stefan Dösinger <stefan at codeweavers.com>
---
Version 2:
*) Use a binary search for the fixup value.
*) Change how an unexpected test result is detected.
*) Add a comment about this to the Vulkan backend.
*) Small wording changes.
Re Vulkan: If you can refer me to the place in the spec that mandates a
top-left filling convention I am more than happy to add it in the comment.
I really don't want to add this detection mess to the Vulkan backend.
I am also more than happy to just drop this nudge thingy unconditionally. I
am not aware of a card + game combination that is fixed by this. Spore
doesn't need it on my gf9600. I didn't bother to download Everquest, I
expect it to have changed a lot in the past 10 years. I don't have access
to a dx9 level card right now, so I can't entirely rule out that Spore needs
this nudge on those cards.
The ddraw tests started succeeding in the todo block for a few of the tested
pixels. I looked at the output results and I don't think it happened because
the Z values are more precise now. I think a few bits flipped (in the random
readback we get) and now a few of those tests were inside the expected diff.
---
dlls/d3d11/tests/d3d11.c | 1 -
dlls/ddraw/tests/ddraw4.c | 6 +-
dlls/ddraw/tests/ddraw7.c | 6 +-
dlls/wined3d/adapter_gl.c | 50 ++++++++++++++
dlls/wined3d/adapter_vk.c | 10 +++
dlls/wined3d/state.c | 3 +-
dlls/wined3d/utils.c | 118 ++++++++++++++++++++++++++++++---
dlls/wined3d/wined3d_private.h | 13 +++-
8 files changed, 189 insertions(+), 18 deletions(-)
diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c
index d8c10ab054c..9bff90bc1e7 100644
--- a/dlls/d3d11/tests/d3d11.c
+++ b/dlls/d3d11/tests/d3d11.c
@@ -28155,7 +28155,6 @@ static void test_fractional_viewports(void)
ok(compare_float(v->x, expected.x, 0) && compare_float(v->y, expected.y, 0),
"Got fragcoord {%.8e, %.8e}, expected {%.8e, %.8e} at (%u, %u), offset %.8e.\n",
v->x, v->y, expected.x, expected.y, x, y, viewport_offsets[i]);
- todo_wine
ok(compare_float(v->z, expected.z, 2) && compare_float(v->w, expected.w, 2),
"Got texcoord {%.8e, %.8e}, expected {%.8e, %.8e} at (%u, %u), offset %.8e.\n",
v->z, v->w, expected.z, expected.w, x, y, viewport_offsets[i]);
diff --git a/dlls/ddraw/tests/ddraw4.c b/dlls/ddraw/tests/ddraw4.c
index 6b514b15e25..4f052256882 100644
--- a/dlls/ddraw/tests/ddraw4.c
+++ b/dlls/ddraw/tests/ddraw4.c
@@ -16118,8 +16118,10 @@ static void test_depth_readback(void)
/* The ddraw4 version of this test behaves similarly to the ddraw7 version on Nvidia GPUs,
* except that Geforce 7 also returns garbage data in D24S8, whereas the ddraw7 version
* returns 0 for that format. Give up on pre-filtering formats, accept Nvidia as generally
- * broken here, but still expect at least one format (D16 or D24X8 in practise) to pass. */
- todo_wine_if(tests[i].todo)
+ * broken here, but still expect at least one format (D16 or D24X8 in practise) to pass.
+ *
+ * Some of the tested places pass on some GPUs on Wine by accident. */
+ todo_wine_if(tests[i].todo && !compare_uint(expected_depth, depth, max_diff))
ok(compare_uint(expected_depth, depth, max_diff) || ddraw_is_nvidia(ddraw),
"Test %u: Got depth 0x%08x (diff %d), expected 0x%08x+/-%u, at %u, %u.\n",
i, depth, expected_depth - depth, expected_depth, max_diff, x, y);
diff --git a/dlls/ddraw/tests/ddraw7.c b/dlls/ddraw/tests/ddraw7.c
index 4c42d6f4b64..4402f2d93b5 100644
--- a/dlls/ddraw/tests/ddraw7.c
+++ b/dlls/ddraw/tests/ddraw7.c
@@ -15597,8 +15597,10 @@ static void test_depth_readback(void)
* Geforce GTX 650 has working D16 and D24, but D24S8 returns 0.
*
* Arx Fatalis is broken on the Geforce 9 in the same way it was broken in Wine (bug 43654).
- * The !tests[i].s_depth is supposed to rule out D16 on GF9 and D24X8 on GF7. */
- todo_wine_if(tests[i].todo)
+ * The !tests[i].s_depth is supposed to rule out D16 on GF9 and D24X8 on GF7.
+ *
+ * Some of the tested places pass on some GPUs on Wine by accident. */
+ todo_wine_if(tests[i].todo && !compare_uint(expected_depth, depth, max_diff))
ok(compare_uint(expected_depth, depth, max_diff)
|| (ddraw_is_nvidia(ddraw) && (all_zero || all_one || !tests[i].s_depth)),
"Test %u: Got depth 0x%08x (diff %d), expected 0x%08x+/-%u, at %u, %u.\n",
diff --git a/dlls/wined3d/adapter_gl.c b/dlls/wined3d/adapter_gl.c
index bba728e2fb5..386d26828dc 100644
--- a/dlls/wined3d/adapter_gl.c
+++ b/dlls/wined3d/adapter_gl.c
@@ -5134,6 +5134,7 @@ static void wined3d_adapter_gl_init_d3d_info(struct wined3d_adapter_gl *adapter_
d3d_info->scaled_resolve = !!gl_info->supported[EXT_FRAMEBUFFER_MULTISAMPLE_BLIT_SCALED];
d3d_info->pbo = !!gl_info->supported[ARB_PIXEL_BUFFER_OBJECT];
d3d_info->feature_level = feature_level_from_caps(gl_info, &shader_caps, &fragment_caps);
+ d3d_info->filling_convention_nudge = gl_info->filling_convention_nudge;
if (gl_info->supported[ARB_TEXTURE_MULTISAMPLE])
d3d_info->multisample_draw_location = WINED3D_LOCATION_TEXTURE_RGB;
@@ -5141,6 +5142,53 @@ static void wined3d_adapter_gl_init_d3d_info(struct wined3d_adapter_gl *adapter_
d3d_info->multisample_draw_location = WINED3D_LOCATION_RB_MULTISAMPLE;
}
+static float wined3d_adapter_find_fill_nudge(struct wined3d_caps_gl_ctx *ctx)
+{
+ static const float test_array[] =
+ {
+ 0.0f,
+ -1.0f / 1024.0f,
+ -1.0f / 512.0f,
+ -1.0f / 256.0f,
+ -1.0f / 128.0f,
+ -1.0f / 64.0f
+ };
+ unsigned int good = ARRAY_SIZE(test_array), bad = 0, test;
+ float value;
+
+ if (wined3d_settings.offscreen_rendering_mode != ORM_FBO)
+ goto end;
+
+ while (good != bad)
+ {
+ test = (good + bad) / 2;
+ value = test_array[test];
+ TRACE("Good %u bad %u, test %u.\n", good, bad, test);
+ if (wined3d_caps_gl_ctx_test_filling_convention(ctx, value))
+ good = test;
+ else
+ bad = test + 1;
+ }
+
+ if (good < ARRAY_SIZE(test_array))
+ {
+ value = test_array[good];
+ if (value)
+ WARN("Using a filling convention fixup nudge of -1/%f.\n", -1.0f / value);
+ else
+ TRACE("No need for a filling convetion nudge.\n");
+
+ return value;
+ }
+
+ FIXME("Did not find a way to get the filling convention we want.\n");
+
+end:
+ /* This value was used unconditionally before the dynamic test function was
+ * introduced. */
+ return -1.0f / 64.0f;
+}
+
static BOOL wined3d_adapter_gl_init(struct wined3d_adapter_gl *adapter_gl,
unsigned int ordinal, unsigned int wined3d_creation_flags)
{
@@ -5226,6 +5274,8 @@ static BOOL wined3d_adapter_gl_init(struct wined3d_adapter_gl *adapter_gl,
return FALSE;
}
+ gl_info->filling_convention_nudge = wined3d_adapter_find_fill_nudge(&caps_gl_ctx);
+
wined3d_adapter_gl_init_d3d_info(adapter_gl, wined3d_creation_flags);
if (!adapter_gl->a.d3d_info.shader_color_key)
diff --git a/dlls/wined3d/adapter_vk.c b/dlls/wined3d/adapter_vk.c
index 18c73312daf..324f4316901 100644
--- a/dlls/wined3d/adapter_vk.c
+++ b/dlls/wined3d/adapter_vk.c
@@ -2206,6 +2206,16 @@ static void wined3d_adapter_vk_init_d3d_info(struct wined3d_adapter_vk *adapter_
d3d_info->pbo = true;
d3d_info->feature_level = feature_level_from_caps(&shader_caps);
+ /* Like GL, Vulkan doesn't explicitly specify a filling convention and only mandates that a
+ * shared edge of two adjacent triangles generate a fragment for exactly one of the triangles.
+ * vktRasterizationTests.cpp from the vulkan CTS tests this by drawing two triangles with a
+ * shared edge with additive blending.
+ *
+ * However, every Vulkan implementation we have seen so far uses a top-left rule. Hardware
+ * that differs either predates Vulkan (d3d9 class HW, GeForce 9xxx) or behaves the way we
+ * want in Vulkan (MacOS Radeon driver through MoltenVK). */
+ d3d_info->filling_convention_nudge = 0.0;
+
d3d_info->multisample_draw_location = WINED3D_LOCATION_TEXTURE_RGB;
}
diff --git a/dlls/wined3d/state.c b/dlls/wined3d/state.c
index 8316269afcf..5c1c69fb650 100644
--- a/dlls/wined3d/state.c
+++ b/dlls/wined3d/state.c
@@ -4233,13 +4233,14 @@ static void viewport_miscpart_cc(struct wined3d_context *context,
const struct wined3d_gl_info *gl_info = wined3d_context_gl(context)->gl_info;
/* See get_projection_matrix() in utils.c for a discussion about those values. */
float pixel_center_offset = context->d3d_info->wined3d_creation_flags
- & WINED3D_PIXEL_CENTER_INTEGER ? 63.0f / 128.0f : -1.0f / 128.0f;
+ & WINED3D_PIXEL_CENTER_INTEGER ? 0.5f : 0.0f;
struct wined3d_viewport vp[WINED3D_MAX_VIEWPORTS];
GLdouble depth_ranges[2 * WINED3D_MAX_VIEWPORTS];
GLfloat viewports[4 * WINED3D_MAX_VIEWPORTS];
unsigned int i, reset_count = 0;
float min_z, max_z;
+ pixel_center_offset += context->d3d_info->filling_convention_nudge / 2.0f;
get_viewports(context, state, state->viewport_count, vp);
GL_EXTCALL(glClipControl(context->render_offscreen ? GL_UPPER_LEFT : GL_LOWER_LEFT, GL_ZERO_TO_ONE));
diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c
index 0a1e0707359..96351d7ead6 100644
--- a/dlls/wined3d/utils.c
+++ b/dlls/wined3d/utils.c
@@ -3928,6 +3928,100 @@ BOOL wined3d_caps_gl_ctx_test_viewport_subpixel_bits(struct wined3d_caps_gl_ctx
return TRUE;
}
+bool wined3d_caps_gl_ctx_test_filling_convention(struct wined3d_caps_gl_ctx *ctx, float nudge)
+{
+ static const struct wined3d_color red = {1.0f, 0.0f, 0.0f, 1.0f};
+ const struct wined3d_gl_info *gl_info = ctx->gl_info;
+ unsigned int x, y, clear = 0, draw = 0;
+ GLuint texture, fbo;
+ DWORD readback[8][8];
+
+ /* This is a very simple test to find out how GL handles polygon edges:
+ * Draw a quad exactly through 4 pixel centers in an 8x8 viewport and see
+ * which pixel it ends up in. So far we've seen top left and bottom
+ * left conventions. This test may produce unexpected results if the
+ * driver forces multisampling on us.
+ *
+ * If we find a bottom-left filling behavior we also nudge the x-axis
+ * by the same amount. This is necessary to keep diagonals that go
+ * through the pixel center intact.
+ *
+ * Note that we are ignoring some settings that might influence the
+ * driver: How we switch GL to an upper-left coordinate system,
+ * shaders vs fixed function GL. Testing these isn't possible with
+ * the current draw_test_quad() infrastructure. Also the test is
+ * skipped if we are not using FBOs. Drawing into the onscreen
+ * frame buffer may also yield different driver behavior.
+ *
+ * The minimum nudge also depends on the viewport size, although
+ * the relation between those two is GPU dependent and not exactly
+ * sensible. E.g. a 8192x8192 viewport on a GeForce 9 needs at
+ * least a nudge of 1/240.9, whereas a 8x8 one needs 1/255.982;
+ * 32x32 needs 1/255.935. 4x4 and lower are happy with something
+ * below 1/256. The 8x8 size below has been arbitrarily chosen to
+ * get a useful result out of that card and avoid allocating a
+ * gigantic texture during library init.
+ *
+ * Newer cards usually do the right thing anyway. In cases where
+ * they do not (e.g. Radeon GPUs in a macbookpro14,3 running MacOS)
+ * a nudge of 1/2^20 is enough. */
+ const struct wined3d_vec3 edge_geometry[] =
+ {
+ {(-1.0f + nudge) / 8.0f, (-1.0f + nudge) / 8.0f, 0.0f},
+ {( 1.0f + nudge) / 8.0f, (-1.0f + nudge) / 8.0f, 0.0f},
+ {(-1.0f + nudge) / 8.0f, ( 1.0f + nudge) / 8.0f, 0.0f},
+ {( 1.0f + nudge) / 8.0f, ( 1.0f + nudge) / 8.0f, 0.0f},
+ };
+
+ gl_info->gl_ops.gl.p_glGenTextures(1, &texture);
+ gl_info->gl_ops.gl.p_glBindTexture(GL_TEXTURE_2D, texture);
+ gl_info->gl_ops.gl.p_glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 0);
+ gl_info->gl_ops.gl.p_glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 8, 8, 0,
+ GL_BGRA, GL_UNSIGNED_INT_8_8_8_8_REV, NULL);
+ gl_info->fbo_ops.glGenFramebuffers(1, &fbo);
+ gl_info->fbo_ops.glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+ gl_info->fbo_ops.glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+ GL_TEXTURE_2D, texture, 0);
+ checkGLcall("create resources");
+
+ gl_info->gl_ops.gl.p_glViewport(0, 0, 8, 8);
+ gl_info->gl_ops.gl.p_glClearColor(0.0f, 0.0f, 1.0f, 1.0f);
+ gl_info->gl_ops.gl.p_glClear(GL_COLOR_BUFFER_BIT);
+
+ draw_test_quad(ctx, edge_geometry, &red);
+ checkGLcall("draw");
+
+ gl_info->gl_ops.gl.p_glBindTexture(GL_TEXTURE_2D, texture);
+ gl_info->gl_ops.gl.p_glGetTexImage(GL_TEXTURE_2D, 0,
+ GL_BGRA, GL_UNSIGNED_INT_8_8_8_8_REV, readback);
+ checkGLcall("readback");
+
+ gl_info->gl_ops.gl.p_glDeleteTextures(1, &texture);
+ gl_info->fbo_ops.glDeleteFramebuffers(1, &fbo);
+ gl_info->fbo_ops.glBindFramebuffer(GL_FRAMEBUFFER, 0);
+ checkGLcall("delete resources");
+
+ /* We expect that exactly one fragment is generated. */
+ for (y = 0; y < ARRAY_SIZE(readback); ++y)
+ {
+ for (x = 0; x < ARRAY_SIZE(readback[0]); ++x)
+ {
+ if (readback[y][x] == 0xff0000ff)
+ clear++;
+ else if (readback[y][x] == 0xffff0000)
+ draw++;
+ }
+ }
+
+ if (clear != 63 || draw != 1)
+ {
+ FIXME("Unexpected filling convention test result.\n");
+ return FALSE;
+ }
+
+ /* One pixel was drawn, check if it is the expect one */
+ return readback[3][3] == 0xffff0000;
+}
static float wined3d_adapter_find_polyoffset_scale(struct wined3d_caps_gl_ctx *ctx, GLenum format)
{
const struct wined3d_gl_info *gl_info = ctx->gl_info;
@@ -5540,15 +5634,19 @@ void get_projection_matrix(const struct wined3d_context *context, const struct w
* - We need to flip along the y-axis in case of offscreen rendering.
* - OpenGL Z range is {-Wc,...,Wc} while D3D Z range is {0,...,Wc}.
* - <= D3D9 coordinates refer to pixel centers while GL coordinates
- * refer to pixel corners.
- * - D3D has a top-left filling convention. We need to maintain this
- * even after the y-flip mentioned above.
- * In order to handle the last two points, we translate by
- * (63.0 / 128.0) / VPw and (63.0 / 128.0) / VPh. This is equivalent to
- * translating slightly less than half a pixel. We want the difference to
- * be large enough that it doesn't get lost due to rounding inside the
- * driver, but small enough to prevent it from interfering with any
- * anti-aliasing. */
+ * refer to pixel corners. D3D10 fixed this particular oddity.
+ * - D3D has a top-left filling convention while GL does not specify
+ * a particular behavior, other than that that the GL implementation
+ * needs to be consistent.
+ *
+ * In order to handle the pixel center, we translate by 0.5 / VPw and
+ * 0.5 / VPh. We test the filling convention during adapter init and
+ * add a small offset to correct it if necessary. See
+ * wined3d_caps_gl_ctx_test_filling_convention() for more details on how
+ * we test GL and considerations regarding the added nudge value.
+ *
+ * If we have GL_ARB_clip_control we take care of all this through
+ * viewport properties and don't have to translate geometry. */
/* Projection matrices are <= d3d9, which all have integer pixel centers. */
if (!(d3d_info->wined3d_creation_flags & WINED3D_PIXEL_CENTER_INTEGER))
@@ -5557,7 +5655,7 @@ void get_projection_matrix(const struct wined3d_context *context, const struct w
clip_control = d3d_info->clip_control;
flip = !clip_control && context->render_offscreen;
if (!clip_control)
- center_offset = 63.0f / 64.0f;
+ center_offset = 1.0f + d3d_info->filling_convention_nudge;
else
center_offset = 0.0f;
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
index 8c49e694fa9..0990bffed21 100644
--- a/dlls/wined3d/wined3d_private.h
+++ b/dlls/wined3d/wined3d_private.h
@@ -243,6 +243,8 @@ struct wined3d_d3d_info
enum wined3d_feature_level feature_level;
DWORD multisample_draw_location;
+
+ float filling_convention_nudge;
};
static const struct color_fixup_desc COLOR_FIXUP_IDENTITY =
@@ -3252,6 +3254,7 @@ struct wined3d_gl_info
DWORD quirks;
BOOL supported[WINED3D_GL_EXT_COUNT];
GLint wrap_lookup[WINED3D_TADDRESS_MIRROR_ONCE - WINED3D_TADDRESS_WRAP + 1];
+ float filling_convention_nudge;
HGLRC (WINAPI *p_wglCreateContextAttribsARB)(HDC dc, HGLRC share, const GLint *attribs);
struct opengl_funcs gl_ops;
@@ -3500,6 +3503,7 @@ BOOL wined3d_adapter_vk_init_format_info(struct wined3d_adapter_vk *adapter_vk,
UINT64 adapter_adjust_memory(struct wined3d_adapter *adapter, INT64 amount) DECLSPEC_HIDDEN;
BOOL wined3d_caps_gl_ctx_test_viewport_subpixel_bits(struct wined3d_caps_gl_ctx *ctx) DECLSPEC_HIDDEN;
+bool wined3d_caps_gl_ctx_test_filling_convention(struct wined3d_caps_gl_ctx *ctx, float nudge) DECLSPEC_HIDDEN;
void install_gl_compat_wrapper(struct wined3d_gl_info *gl_info, enum wined3d_gl_extension ext) DECLSPEC_HIDDEN;
@@ -5680,10 +5684,15 @@ static inline void shader_get_position_fixup(const struct wined3d_context *conte
float center_offset;
unsigned int i;
+ /* See get_projection_matrix() in utils.c for a discussion of the position fixup.
+ * This function here also applies to d3d10+ which does not need adjustment for
+ * integer pixel centers, but it may need the filling convention nudge. */
if (context->d3d_info->wined3d_creation_flags & WINED3D_PIXEL_CENTER_INTEGER)
- center_offset = 63.0f / 64.0f;
+ center_offset = 1.0f;
else
- center_offset = -1.0f / 64.0f;
+ center_offset = 0.0f;
+
+ center_offset += context->d3d_info->filling_convention_nudge;
for (i = 0; i < fixup_count; ++i)
{
--
2.32.0
More information about the wine-devel
mailing list