Re: [PATCH v2 2/2] wined3d: Add quirk for Nvidia viewport subpixel bits bug.
On 27.02.2017 13:42, Józef Kucia wrote:
Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> ---
Version 2: Test if the pixel center offset works as expected.
Tested with nouveau, radeonsi and Nvidia blob. wined3d_caps_gl_ctx_test_viewport_subpixel_bits() fails only with Nvidia blob.
As you might be aware, the problem on Nvidia binary can also be worked around by using slightly different offsets (basically reverting 1bed10abee7d2439a5c7358596c860c454f06b53). Do you think its better to disable it, or should we just use different pixel center offsets?
--- dlls/wined3d/directx.c | 27 ++++++++++++++++++++++++ dlls/wined3d/utils.c | 47 ++++++++++++++++++++++++++++++++++++++++++ dlls/wined3d/wined3d_private.h | 2 ++ 3 files changed, 76 insertions(+)
diff --git a/dlls/wined3d/directx.c b/dlls/wined3d/directx.c index 7f1a850..3495710 100644 --- a/dlls/wined3d/directx.c +++ b/dlls/wined3d/directx.c @@ -913,6 +913,17 @@ static BOOL match_broken_arb_fog(const struct wined3d_gl_info *gl_info, struct w return data[0] != 0x00ff0000 || data[3] != 0x0000ff00; }
+static BOOL match_broken_viewport_subpixel_bits(const struct wined3d_gl_info *gl_info, + struct wined3d_caps_gl_ctx *ctx, const char *gl_renderer, enum wined3d_gl_vendor gl_vendor, + enum wined3d_pci_vendor card_vendor, enum wined3d_pci_device device) +{ + if (!gl_info->supported[ARB_VIEWPORT_ARRAY]) + return FALSE; + if (wined3d_settings.offscreen_rendering_mode != ORM_FBO) + return FALSE; + return !wined3d_caps_gl_ctx_test_viewport_subpixel_bits(ctx); +} + static void quirk_apple_glsl_constants(struct wined3d_gl_info *gl_info) { /* MacOS needs uniforms for relative addressing offsets. This can accumulate to quite a few uniforms. @@ -1041,6 +1052,17 @@ static void quirk_broken_arb_fog(struct wined3d_gl_info *gl_info) gl_info->quirks |= WINED3D_QUIRK_BROKEN_ARB_FOG; }
+static void quirk_broken_viewport_subpixel_bits(struct wined3d_gl_info *gl_info) +{ + TRACE("Disabling ARB_viewport_array.\n"); + gl_info->supported[ARB_VIEWPORT_ARRAY] = FALSE; + if (gl_info->supported[ARB_CLIP_CONTROL]) + { + TRACE("Disabling ARB_clip_control.\n"); + gl_info->supported[ARB_CLIP_CONTROL] = FALSE; + } +} + struct driver_quirk { BOOL (*match)(const struct wined3d_gl_info *gl_info, struct wined3d_caps_gl_ctx *ctx, @@ -1132,6 +1154,11 @@ static const struct driver_quirk quirk_table[] = quirk_broken_arb_fog, "ARBfp fogstart == fogend workaround" }, + { + match_broken_viewport_subpixel_bits, + quirk_broken_viewport_subpixel_bits, + "Nvidia viewport subpixel bits bug" + }, };
/* Certain applications (Steam) complain if we report an outdated driver version. In general, diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c index b076783..c392388 100644 --- a/dlls/wined3d/utils.c +++ b/dlls/wined3d/utils.c @@ -3509,6 +3509,53 @@ fail: return FALSE; }
+BOOL wined3d_caps_gl_ctx_test_viewport_subpixel_bits(struct wined3d_caps_gl_ctx *ctx) +{ + static const struct wined3d_color red = {1.0f, 0.0f, 0.0f, 1.0f}; + const struct wined3d_gl_info *gl_info = ctx->gl_info; + static const float offset = -63.0f / 128.0f; + GLuint texture, fbo; + DWORD readback[4]; + unsigned int i; + + 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, ARRAY_SIZE(readback), 1, 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_glClearColor(1.0f, 1.0f, 1.0f, 1.0f); + gl_info->gl_ops.gl.p_glClear(GL_COLOR_BUFFER_BIT); + GL_EXTCALL(glViewportIndexedf(0, offset, offset, 4.0f, 1.0f)); + draw_test_quad(ctx, NULL, &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"); + + TRACE("Readback colors are 0x%08x, 0x%08x, 0x%08x, 0x%08x.\n", + readback[0], readback[1], readback[2], readback[3]); + + 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"); + + for (i = 0; i < ARRAY_SIZE(readback); ++i) + { + if (readback[i] != 0xffff0000) + return FALSE; + } + return TRUE; +} + float wined3d_adapter_find_polyoffset_scale(struct wined3d_caps_gl_ctx *ctx, GLenum format) { const struct wined3d_gl_info *gl_info = ctx->gl_info; diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 7950011..3cd7ae5 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2354,6 +2354,8 @@ BOOL wined3d_adapter_init_format_info(struct wined3d_adapter *adapter, struct wined3d_caps_gl_ctx *ctx) DECLSPEC_HIDDEN; 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; + void install_gl_compat_wrapper(struct wined3d_gl_info *gl_info, enum wined3d_gl_extension ext) DECLSPEC_HIDDEN;
enum projection_types
On Mon, Feb 27, 2017 at 1:49 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote:
As you might be aware, the problem on Nvidia binary can also be worked around by using slightly different offsets (basically reverting 1bed10abee7d2439a5c7358596c860c454f06b53). Do you think its better to disable it, or should we just use different pixel center offsets?
The offsets used by 1bed10abee7d2439a5c7358596c860c454f06b53 are more correct, e.g. because of filling conventions. I don't think using different offsets is a correct thing to do. We also have a code path which handles the pixel offsets correctly when ARB_clip_control is not available. On the other hand, we may want to reconsider the ARB_viewport_array detection logic when we'll add support for multiple viewports in wined3d.
On 27.02.2017 14:47, Józef Kucia wrote:
On Mon, Feb 27, 2017 at 1:49 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote:
As you might be aware, the problem on Nvidia binary can also be worked around by using slightly different offsets (basically reverting 1bed10abee7d2439a5c7358596c860c454f06b53). Do you think its better to disable it, or should we just use different pixel center offsets?
The offsets used by 1bed10abee7d2439a5c7358596c860c454f06b53 are more correct, e.g. because of filling conventions. I don't think using different offsets is a correct thing to do.
I'm not suggesting to change it in general, but the question is if it is reliable enough to be used as a quirk on Nvidia drivers. It is sufficient to pass all the Wine tests at least. If you think manual fixup is better, thats also fine ofc.
We also have a code path which handles the pixel offsets correctly when ARB_clip_control is not available. On the other hand, we may want to reconsider the ARB_viewport_array detection logic when we'll add support for multiple viewports in wined3d.
On Mon, Feb 27, 2017 at 3:12 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote:
On 27.02.2017 14:47, Józef Kucia wrote:
The offsets used by 1bed10abee7d2439a5c7358596c860c454f06b53 are more correct, e.g. because of filling conventions. I don't think using different offsets is a correct thing to do.
I'm not suggesting to change it in general, but the question is if it is reliable enough to be used as a quirk on Nvidia drivers. It is sufficient to pass all the Wine tests at least. If you think manual fixup is better, thats also fine ofc.
I know, but I think that we do not want to use different offsets just because the Wine tests pass when these offsets are used. The fractional viewport handling seems to be quite clearly broken in Nvidia drivers. Anyway, the final decision is up to Henri.
participants (2)
-
Józef Kucia -
Sebastian Lackner