Re: [PATCH 3/5] wined3d: Handle sRGB_decode when reading the sampler state.

[Henri Verbeet]

On 8 October 2013 00:27, Stefan Dösinger  wrote:
> diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c
> index 52eac16..eb8ca7e 100644
> --- a/dlls/wined3d/surface.c
> +++ b/dlls/wined3d/surface.c
> @@ -5608,8 +5608,6 @@ HRESULT surface_load_location(struct wined3d_surface *surface, DWORD location, c
>          }
>      }
> -
>
> if (location == SFLAG_INSRGBTEX && gl_info->supported[EXT_TEXTURE_SRGB_DECODE])
> -
>    location = SFLAG_INTEXTURE;
>
> if (surface->flags & location)
>    {
>
> @@ -5671,12 +5669,6 @@ HRESULT surface_load_location(struct wined3d_surface *surface, DWORD location, c
>              surface_evict_sysmem(surface);
>      }
> - if (surface->flags & (SFLAG_INTEXTURE | SFLAG_INSRGBTEX)
> -
>        && gl_info->supported[EXT_TEXTURE_SRGB_DECODE])
> - {
> -
>    surface->flags |= (SFLAG_INTEXTURE | SFLAG_INSRGBTEX);
> - }
> - return WINED3D_OK;
>
> }

This change seems good on its own, as far as I can tell all callers
already handle this correctly. I'm less sure about the other changes,
the texture code seems like the right place to handle
EXT_texture_sRGB_decode.

[PATCH 3/5] wined3d: Handle sRGB_decode when reading the sampler state.

[Stefan Dösinger]

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

Am 2013-10-08 12:06, schrieb Henri Verbeet:
> On 8 October 2013 00:27, Stefan Dösinger 
> wrote:
> > diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c 
> > index 52eac16..eb8ca7e 100644 --- a/dlls/wined3d/surface.c +++
> > b/dlls/wined3d/surface.c @@ -5608,8 +5608,6 @@ HRESULT
> > surface_load_location(struct wined3d_surface *surface, DWORD
> > location, c } }
> > - if (location == SFLAG_INSRGBTEX &&
> >
> > gl_info->supported[EXT_TEXTURE_SRGB_DECODE]) -        location =
> > SFLAG_INTEXTURE;
> >
> > if (surface->flags & location) { @@ -5671,12 +5669,6 @@ HRESULT
> > surface_load_location(struct wined3d_surface *surface, DWORD
> > location, c surface_evict_sysmem(surface); }
> > - if (surface->flags & (SFLAG_INTEXTURE | SFLAG_INSRGBTEX) -
> >
> > && gl_info->supported[EXT_TEXTURE_SRGB_DECODE]) -    { -
> > surface->flags |= (SFLAG_INTEXTURE | SFLAG_INSRGBTEX); -    } - 
> > return WINED3D_OK; }
>
> This change seems good on its own, as far as I can tell all
> callers already handle this correctly.

Are you sure? E.g. if a texture is used with srgb=true and sRGB_decode
is supported, wined3d_texture_bind sets WINED3D_TEXTURE_IS_SRGB. If
the application later calls PreLoad manually, texture2d_preload is
called with SRGB_ANY. Because of the set IS_SRGB flag it chooses to
load the srgb copy. It correctly uses the rgb texture structure and
binds the rgb GL texture, but still passes srgb=true to
surface_load(), which results in SFLAG_INSRGBTEX being passed to
surface_load_location, even though we want to load the RGB copy.

I'll see if I can simplify the sRGB_decode handling while still
keeping it in the texture.

Patches 4 and 5 should apply without this one by the way.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.20 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSU/AfAAoJEN0/YqbEcdMwIP8P/iJUb9d3dfTQgdDAYNSx0fFc
wp7tJZSZKDNlSLz8oTNx7k2Lmcv7ZSInce+R1oaGxS7QjkhxD7U25CdnvgaXV+Oy
IInOFuauZ7m6zO+FfBbHd9ujM1pC1Mi7BtsE59LNypJT9ym6iqe2MLocLUjDCfbL
koro3I3rzjkMb3XVUQapMevobYBr0jfl3G3q7zirrVuh1fYnL3a1Ge4ckIGsRneL
98ZjmcQyfT8lo9zxtwXPTOR23j1oLnJDNWhn63he1sX6Vg7XQvPZwszXwbN30Jof
CjbrTzBmaKq/yY4jXnffu84tDygvWFr0a9sklX7qtaGd4cNBaiSscR6gBAgvZQdI
1533llMChhOqIUBrS5i8d3t24DLdAzd6PiD4LBCjJXzqfTiqWp3JjChR9FUvq+P1
G5gEldxF+MHPXKhZgpq1MoAy13NDYAFNT/EZSgIH/yRGcm9qVTnI98A6gFZnvel4
DQ3p05mMN4dSvGsQJt/l42k8I5IT1nYqetE1ybZd/45LgHKyjYc80z8lt6f0fhrS
j9KtY3Pu6Ks8hIjsIrnVPX7SSbiaNzytEwTpECXsmnB6T2Co+d5YOYzKmWLIqOqL
ZSTBkStiBzfxi+2KWlnfUHzWmOthaPo98ZRsiwtaux9W+8xvIDMnWD5x51EPlcjt
ca93bMIW9i8aPNOOMeKb
=dp6t
-----END PGP SIGNATURE-----

[PATCH 3/5] wined3d: Handle sRGB_decode when reading the sampler state.

[Henri Verbeet]

On 8 October 2013 13:44, Stefan Dösinger  wrote:
> Are you sure? E.g. if a texture is used with srgb=true and sRGB_decode
> is supported, wined3d_texture_bind sets WINED3D_TEXTURE_IS_SRGB. If
> the application later calls PreLoad manually, texture2d_preload is
> called with SRGB_ANY. Because of the set IS_SRGB flag it chooses to
> load the srgb copy. It correctly uses the rgb texture structure and
> binds the rgb GL texture, but still passes srgb=true to
> surface_load(), which results in SFLAG_INSRGBTEX being passed to
> surface_load_location, even though we want to load the RGB copy.

Right. It should be pretty easy to fix that by taking
EXT_TEXTURE_SRGB_DECODE into account in texture_srgb_mode() though.