Re: [PATCH 3/6] wined3d: Add support for separate sRGB formats.
On 4 February 2016 at 01:17, Józef Kucia <jkucia(a)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?
On Thu, Feb 4, 2016 at 3:57 PM, Henri Verbeet <hverbeet(a)gmail.com> wrote:
On 4 February 2016 at 01:17, Józef Kucia <jkucia(a)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.
I'll explore this option.
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}
I'll keep this in mind when extending support for typeless formats.
participants (2)
-
Henri Verbeet -
Józef Kucia