[PATCH 3/6] wined3d: Add support for separate sRGB formats.

[Henri Verbeet]

On 4 February 2016 at 01:17, Józef Kucia <jkucia at codeweavers.com> wrote:
> -    factory->wined3d = wined3d_create(0);
> +    factory->wined3d = wined3d_create(WINED3D_SEPARATE_SRGB_FORMATS);
In some ways I like this, although I wonder if it wouldn't make more
sense to handle this in terms of a flag to enable support for
SRGB_READ/SRGB_WRITE. I.e., when WINED3D_SAMP_SRGB_TEXTURE and
WINED3D_RS_SRGBWRITEENABLE are supported, WINED3DFMT_R8G8B8A8_UNORM
and WINED3DFMT_R8G8B8A8_UNORM_SRGB would effectively be the same,
while when they aren't you'd get the d3d10+ behaviour.

> @@ -5993,7 +5994,7 @@ HRESULT wined3d_init(struct wined3d *wined3d, DWORD flags)
>          return WINED3D_OK;
>      }
>
> -    if (!wined3d_adapter_init(&wined3d->adapters[0], 0))
> +    if (!wined3d_adapter_init(&wined3d->adapters[0], wined3d, 0))
I'm not a huge fan of this, although I could probably live with it. I
guess it's mostly to get at wined3d->flags during format
initialization, in which case it may be sufficient to just pass the
flags to wined3d_adapter_init().

> diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c
> index 1904dd1..718aa18 100644
> --- a/dlls/wined3d/utils.c
> +++ b/dlls/wined3d/utils.c
> @@ -58,8 +58,11 @@ static const struct wined3d_format_channels formats[] =
>      {WINED3DFMT_DXT4,                       0,  0,  0,  0,   0,  0,  0,  0,    1,   0,     0},
>      {WINED3DFMT_DXT5,                       0,  0,  0,  0,   0,  0,  0,  0,    1,   0,     0},
>      {WINED3DFMT_BC1_UNORM,                  0,  0,  0,  0,   0,  0,  0,  0,    1,   0,     0},
> +    {WINED3DFMT_BC1_UNORM_SRGB,             0,  0,  0,  0,   0,  0,  0,  0,    1,   0,     0},
>      {WINED3DFMT_BC2_UNORM,                  0,  0,  0,  0,   0,  0,  0,  0,    1,   0,     0},
> +    {WINED3DFMT_BC2_UNORM_SRGB,             0,  0,  0,  0,   0,  0,  0,  0,    1,   0,     0},
>      {WINED3DFMT_BC3_UNORM,                  0,  0,  0,  0,   0,  0,  0,  0,    1,   0,     0},
> +    {WINED3DFMT_BC3_UNORM_SRGB,             0,  0,  0,  0,   0,  0,  0,  0,    1,   0,     0},
>      {WINED3DFMT_MULTI2_ARGB8,               0,  0,  0,  0,   0,  0,  0,  0,    1,   0,     0},
>      {WINED3DFMT_G8R8_G8B8,                  0,  0,  0,  0,   0,  0,  0,  0,    1,   0,     0},
>      {WINED3DFMT_R8G8_B8G8,                  0,  0,  0,  0,   0,  0,  0,  0,    1,   0,     0},
One thought I have about this is that we could just store the channel
sizes and shifts for the typeless formats, and then have the typed
formats copy the information from there, perhaps instead of the
format_base_flags[] table we have now. So you'd have something like
this:
    {WINED3DFMT_R8G8B8A8_UNORM,      WINED3DFMT_R8G8B8A8_TYPELESS,
UNORM, UNORM, UNORM, UNORM}
    {WINED3DFMT_R8G8B8A8_UNORM_SRGB, WINED3DFMT_R8G8B8A8_TYPELESS,
UNORM, UNORM, UNORM, UNORM}
    {WINED3DFMT_R8G8B8A8_SNORM,      WINED3DFMT_R8G8B8A8_TYPELESS,
SNORM, SNORM, SNORM, SNORM}
    {WINED3DFMT_R8G8B8A8_UINT,       WINED3DFMT_R8G8B8A8_TYPELESS,
UINT,  UINT,  UINT,  UINT}
    {WINED3DFMT_R8G8B8A8_SINT,       WINED3DFMT_R8G8B8A8_TYPELESS,
SINT,  SINT,  SINT,  SINT}

> @@ -2237,7 +2255,7 @@ static void query_internal_format(struct wined3d_adapter *adapter,
>             format_clear_flag(format, WINED3DFMT_FLAG_SRGB_WRITE);
>
>         if (!gl_info->supported[ARB_DEPTH_TEXTURE]
> -                && texture_info->flags & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL))
> +                && !!texture_info && texture_info->flags & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL))
The "!!" before texture_info seems unnecessary.

> +static inline BOOL should_use_separate_srgb_gl_texture(const struct wined3d_context *context)
That's a fairly long function name. Not that I have a better suggestion though.
> +{
> +    return !context->gl_info->supported[EXT_TEXTURE_SRGB_DECODE]
> +            && !(context->swapchain->device->wined3d->flags & WINED3D_SEPARATE_SRGB_FORMATS);
> +}
The number of pointer dereferences in here is scary. :) (Yeah, I know,
there are other places that are just as bad.) Maybe it makes sense to
store a copy of the flags in struct d3d_info?