This fixes stray lines in GameFace GUIs, e.g. in World of Tanks.
Signed-off-by: Stefan Dösinger stefan@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) {