Re: [PATCHv3] wined3d: Use an rbtree for storing shaders for texture format conversion/blitting

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Two more non-functional nitpicks:

Am 2014-02-13 11:16, schrieb Martin Storsjo:
> +static void arbfp_free_blit_shader(struct wine_rb_entry *entry, void *context)
> +{
> - const struct wined3d_gl_info *gl_info = context;
> - struct arbfp_blit_desc *entry_arb = WINE_RB_ENTRY_VALUE(entry, struct arbfp_blit_desc, entry);
> -
> - GL_EXTCALL(glDeleteProgramsARB(1, &entry_arb->shader));
> - checkGLcall("glDeleteProgramsARB(1, &entry_arb->shader)");
> - HeapFree(GetProcessHeap(), 0, entry_arb);
>
> +}

Please add a comment

/* Context activation is done by the caller. */

above the function. While you're at it, please also add it above
gen_p8_shader. It is not necessary to add this comment above
gen_planar_yuv_read and gen_nv12_read because those functions do not
call OpenGL functions.
> -
>    switch (fixup)
> -
>    {
>    case COMPLEX_FIXUP_P8:
> -
>        shader = textype == GL_TEXTURE_RECTANGLE_ARB ? priv->p8_rect_shader : priv->p8_2d_shader;
> -
>        if (!shader) shader = gen_p8_shader(priv, gl_info, textype);
> -
> -
>        upload_palette(surface, context);
> -
>        shader = gen_p8_shader(priv, gl_info, textype);
>        break;
>
>    default:
> -
>        shader = gen_yuv_shader(priv, gl_info, fixup, textype);
> -
>        break;
> -
>    }

Please add 4 spaces of indentation, like this

switch (fixup)
{
    case foo:
        code();
        more_code();
        break;

case bar:
        other_code();
        break;
}

Other than that the code looks good to me.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJS/K4ZAAoJEN0/YqbEcdMwHzoP/0/wPibZu3Jy3MAlACpiPlq9
0inM3Egyh/qfYw7Qi8I+2eaAYZKZYy9LZDJ89fXjS87Ip8v+ryoGMXpLeCGgGze4
VV7F/mr/V7OKHs6ObFipmO4pEgqYn6gbc5x87SCuHvEwz70CUOfZiTPzt94d71Z4
Hr69M6wSU4DNOndOr83pJNjEQAff9qIecvNZwp/xlvgg/rZSdr3uMBLap/fJmAGy
NtbJVgIAtCovkANBTDmE55U8bPQLdRQapZbBH+JQN3LMxmg3C4pJ++Wnc/1kRmmv
3ZvVSBHdwTK7dPuLx7aGl66gMYo9IdyGUYvD+Jk2GchfhRowwC2FcWDhLEDRhWpd
ahQnoa8bTuKgtlTwygWZLm748e8asaC1Chcr5pKj+IZDo/F/qq6U46LL9K5SBFC1
1sTjF578QQ3K5LiCrGv6ivsHvJq+JkwQz0QxDhqJ4Bs0lJ5LEvC54l7XjumDvtfg
8+4slUf/FKhZNjgp/XRF/qip1+hwMehZULlHIo0CBzSjBiNxxKA+IQq/CeKfjAMa
cjPkhRW42H/l47IJthepxuMaoNO5WAp75TXha2vwHQY/Axs8AsKqyzTyImuMa50U
b+kHVilfW9hjSJWSGbLb7JgCurgTMKHkt1W2+6OQpbFOBrCihr3uQWAifo8+IuTP
3U5Y0hUe8rU9IKyDS0eO
=AU5Y
-----END PGP SIGNATURE-----