Re: [PATCH v2 2/2] wined3d: Add quirk for Nvidia viewport subpixel bits bug.

[Sebastian Lackner]

On 27.02.2017 13:42, Józef Kucia wrote:
> Signed-off-by: Józef Kucia
>
> 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