On Wed, May 19, 2010 at 7:02 PM, Henri Verbeet <[email protected]> wrote:
> On 19 May 2010 14:46, Roderick Colenbrander <[email protected]> wrote:
>>> The main issue I'm facing is p8 uploads which are completely broken
>>> right now (a regression). At the beginning of the week I wanted to add
> ...
>> What do you think?
>>
> Is there a bug report / demo application that clearly shows the
> problem? Maybe I can give it a look.
>
The bug in question is and a test case is StarCraft:
http://bugs.winehq.org/show_bug.cgi?id=22575
I have made a draft patch (this would have to be cut in a lot of
pieces and changes a few more things than just the issue). It might be
an acceptable path.
The idea is to move the 'p8 texture format' to the formats table
including the case when no shaders/ext_paletted_texture is around
(this case needs to be added in some form anyway since fixup up an
empty format desc is not that nice). The format is overridden in case
of color keying / drawpixels. So at this stage the 'blit_supported' is
implicitly performed at formats table initialization. This might be
acceptable for now. At a later stage I can imagine that we want to ask
the 'blit_shader' for the format to use as that what this is basically
about.
If I add the 'converted p8' format to the formats table, I would still
have to return 'CONVERT_PALETTED' in d3dfmt_get_conv and somehow
inspect the format. The cleanest way (I could check conv_bytes) I
think is to add a flag WINED3DFMT_FLAG_CONVERTED. I want to set this
flag also for converted textures and thus checks like 'convert !=
NO_CONVERSION || desc.convert' can be adjusted as well and
d3dfmt_convert_surface is now a bit nicer as well.
Though not all parts might be needed right now, I think at least the
Flags changes are a step in the right direction. Is the 'p8 solution'
acceptable for now?
Roderick
diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index feb3949..44afe5a 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -5265,8 +5264,8 @@ static HRESULT WINAPI IWineD3DDeviceImpl_UpdateSurface(IWineD3DDevice *iface,
* surface to the destination's sysmem copy. If surface conversion is
* needed, use BltFast instead to copy in sysmem and use regular surface
* loading. */
- d3dfmt_get_conv(dst_impl, FALSE, TRUE, &desc, &convert);
- if (convert != NO_CONVERSION || desc.convert)
+ d3dfmt_get_conv(dst_impl, FALSE, TRUE, &desc);
+ if (desc.Flags & WINED3DFMT_FLAG_CONVERTED)
return IWineD3DSurface_BltFast(dst_surface, dst_x, dst_y, src_surface, src_rect, 0);
context = context_acquire(This, NULL);
diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c
index 2d7ff23..1d84089 100644
--- a/dlls/wined3d/surface.c
+++ b/dlls/wined3d/surface.c
@@ -1562,13 +1562,12 @@ static void surface_prepare_texture_internal(IWineD3DSurfaceImpl *surface,
const struct wined3d_gl_info *gl_info, BOOL srgb)
{
DWORD alloc_flag = srgb ? SFLAG_SRGBALLOCATED : SFLAG_ALLOCATED;
- CONVERT_TYPES convert;
struct wined3d_format_desc desc;
if (surface->Flags & alloc_flag) return;
- d3dfmt_get_conv(surface, TRUE, TRUE, &desc, &convert);
- if(convert != NO_CONVERSION || desc.convert) surface->Flags |= SFLAG_CONVERTED;
+ d3dfmt_get_conv(surface, TRUE, TRUE, &desc);
+ if(desc.Flags & WINED3DFMT_FLAG_CONVERTED) surface->Flags |= SFLAG_CONVERTED;
else surface->Flags &= ~SFLAG_CONVERTED;
surface_bind_and_dirtify(surface, srgb);
@@ -2142,17 +2141,18 @@ static HRESULT WINAPI IWineD3DSurfaceImpl_ReleaseDC(IWineD3DSurface *iface, HDC
IWineD3DSurface Internal (No mapping to directx api) parts follow
****************************************************** */
-HRESULT d3dfmt_get_conv(IWineD3DSurfaceImpl *This, BOOL need_alpha_ck, BOOL use_texturing, struct wined3d_format_desc *desc, CONVERT_TYPES *convert)
+HRESULT d3dfmt_get_conv(IWineD3DSurfaceImpl *This, BOOL need_alpha_ck, BOOL use_texturing, struct wined3d_format_desc *desc)
{
BOOL colorkey_active = need_alpha_ck && (This->CKeyFlags & WINEDDSD_CKSRCBLT);
IWineD3DDeviceImpl *device = This->resource.device;
- BOOL blit_supported = FALSE;
- RECT rect = {0, 0, This->pow2Width, This->pow2Height};
/* Copy the default values from the surface. Below we might perform fixups */
/* TODO: get rid of color keying desc fixups by using e.g. a table. */
*desc = *This->resource.format_desc;
- *convert = NO_CONVERSION;
+
+//I will set this flag for all formats with a convert function pointer
+//as a demonstration it is here but I will move it to the table
+if(desc->convert) desc->Flags |= WINED3DFMT_FLAG_CONVERTED;
/* Ok, now look if we have to do any conversion */
switch(This->resource.format_desc->format)
@@ -2162,28 +2162,23 @@ HRESULT d3dfmt_get_conv(IWineD3DSurfaceImpl *This, BOOL need_alpha_ck, BOOL use_
Paletted Texture
**************** */
- blit_supported = device->blitter->blit_supported(&device->adapter->gl_info, BLIT_OP_BLIT,
- &rect, This->resource.usage, This->resource.pool,
- This->resource.format_desc, &rect, This->resource.usage,
- This->resource.pool, This->resource.format_desc);
-
- /* Use conversion when the blit_shader backend supports it. It only supports this in case of
- * texturing. Further also use conversion in case of color keying.
- * Paletted textures can be emulated using shaders but only do that for 2D purposes e.g. situations
- * in which the main render target uses p8. Some games like GTA Vice City use P8 for texturing which
- * conflicts with this.
+ /* The format entry used for texturing is set in the formats table. On modern GPUs we use shaders for palette
+ * conversion but we limit that to 2D purposes e.g. situations in which the main render target uses p8.
+ * Some games like GTA Vice City use P8 for texturing which conflicts with this.
+ * Also don't use it for color keying and when drawpixels is used (!use_texturing).
*/
- if (!((blit_supported && device->render_targets && This == device->render_targets[0]))
+ if (!(device->render_targets && This == device->render_targets[0])
|| colorkey_active || !use_texturing)
{
desc->glFormat = GL_RGBA;
desc->glInternal = GL_RGBA;
desc->glType = GL_UNSIGNED_BYTE;
desc->conv_byte_count = 4;
+
if(colorkey_active) {
- *convert = CONVERT_PALETTED_CK;
+ desc->Flags |= WINED3DFMT_FLAG_CONVERTED | WINED3DFMT_FLAG_COLOR_KEYING;
} else {
- *convert = CONVERT_PALETTED;
+ desc->Flags |= WINED3DFMT_FLAG_CONVERTED;
}
}
break;
@@ -2201,40 +2196,40 @@ HRESULT d3dfmt_get_conv(IWineD3DSurfaceImpl *This, BOOL need_alpha_ck, BOOL use_
case WINED3DFMT_B5G6R5_UNORM:
if (colorkey_active) {
- *convert = CONVERT_CK_565;
desc->glFormat = GL_RGBA;
desc->glInternal = GL_RGB5_A1;
desc->glType = GL_UNSIGNED_SHORT_5_5_5_1;
+ desc->Flags |= WINED3DFMT_FLAG_CONVERTED | WINED3DFMT_FLAG_COLOR_KEYING;
desc->conv_byte_count = 2;
}
break;
..
..
..
..
case WINED3DFMT_B8G8R8X8_UNORM:
if (colorkey_active) {
- *convert = CONVERT_RGB32_888;
desc->glFormat = GL_RGBA;
desc->glInternal = GL_RGBA8;
desc->glType = GL_UNSIGNED_INT_8_8_8_8;
+ desc->Flags |= WINED3DFMT_FLAG_CONVERTED | WINED3DFMT_FLAG_COLOR_KEYING;
desc->conv_byte_count = 4;
}
break;
@@ -2327,30 +2322,34 @@ void d3dfmt_p8_init_palette(IWineD3DSurfaceImpl *This, BYTE table[256][4], BOOL
}
static HRESULT d3dfmt_convert_surface(const BYTE *src, BYTE *dst, UINT pitch, UINT width,
- UINT height, UINT outpitch, CONVERT_TYPES convert, IWineD3DSurfaceImpl *This)
+ UINT height, UINT outpitch, IWineD3DSurfaceImpl *This)
{
+ const struct wined3d_format_desc *format_desc = This->resource.format_desc;
const BYTE *source;
BYTE *dest;
- TRACE("(%p)->(%p),(%d,%d,%d,%d,%p)\n", src, dst, pitch, height, outpitch, convert,This);
+ TRACE("(%p)->(%p),(%d,%d,%d,%p)\n", src, dst, pitch, height, outpitch, This);
- switch (convert) {
- case NO_CONVERSION:
- {
- memcpy(dst, src, pitch * height);
- break;
- }
- case CONVERT_PALETTED:
- case CONVERT_PALETTED_CK:
+ if (!(format_desc->Flags & WINED3DFMT_FLAG_CONVERTED))
+ {
+ /* This shouldn't happen, so likely should be removed */
+ memcpy(dst, src, pitch * height);
+ return WINED3D_OK;
+ }
+
+ switch (format_desc->format)
+ {
+ case WINED3DFMT_P8_UINT:
{
IWineD3DPaletteImpl* pal = This->palette;
BYTE table[256][4];
unsigned int x, y;
+ BOOL use_color_keying = (format_desc->Flags & WINED3DFMT_FLAG_COLOR_KEYING) ? TRUE : FALSE;
if( pal == NULL) {
/* TODO: If we are a sublevel, try to get the palette from level 0 */
}
- d3dfmt_p8_init_palette(This, table, (convert == CONVERT_PALETTED_CK));
+ d3dfmt_p8_init_palette(This, table, use_color_keying);
for (y = 0; y < height; y++)
{
@@ -2368,7 +2367,7 @@ static HRESULT d3dfmt_convert_surface(const BYTE *src, BYTE *dst, UINT pitch, UI
}
break;
- case CONVERT_CK_565:
+ case WINED3DFMT_B5G6R5_UNORM:
{
/* Converting the 565 format in 5551 packed to emulate color-keying.
..
..
..
..
@@ -2475,7 +2474,7 @@ static HRESULT d3dfmt_convert_surface(const BYTE *src, BYTE *dst, UINT pitch, UI
break;
default:
- ERR("Unsupported conversion type %#x.\n", convert);
+ ERR("Unsupported conversion for fmt=%s.\n", debug_d3dformat(format_desc->format));
}
return WINED3D_OK;
}
@@ -4480,7 +4478,7 @@ static HRESULT WINAPI IWineD3DSurfaceImpl_LoadLocation(IWineD3DSurface *iface, D
IWineD3DSurfaceImpl_LoadLocation(iface, SFLAG_INSYSMEM, rect);
}
- d3dfmt_get_conv(This, FALSE /* We need color keying */, FALSE /* We won't use textures */, &desc, &convert);
+ d3dfmt_get_conv(This, FALSE /* We need color keying */, FALSE /* We won't use textures */, &desc);
/* The width is in 'length' not in bytes */
width = This->currentDesc.Width;
@@ -4488,7 +4486,7 @@ static HRESULT WINAPI IWineD3DSurfaceImpl_LoadLocation(IWineD3DSurface *iface, D
/* Don't use PBOs for converted surfaces. During PBO conversion we look at SFLAG_CONVERTED
* but it isn't set (yet) in all cases it is getting called. */
- if ((convert != NO_CONVERSION) && (This->Flags & SFLAG_PBO))
+ if ((desc.Flags & WINED3DFMT_FLAG_CONVERTED) && (This->Flags & SFLAG_PBO))
{
struct wined3d_context *context = NULL;
@@ -4499,7 +4497,7 @@ static HRESULT WINAPI IWineD3DSurfaceImpl_LoadLocation(IWineD3DSurface *iface, D
if (context) context_release(context);
}
- if((convert != NO_CONVERSION) && This->resource.allocatedMemory) {
+ if((desc.Flags & WINED3DFMT_FLAG_CONVERTED) && This->resource.allocatedMemory) {
int height = This->currentDesc.Height;
byte_count = desc.conv_byte_count;
@@ -4512,7 +4510,7 @@ static HRESULT WINAPI IWineD3DSurfaceImpl_LoadLocation(IWineD3DSurface *iface, D
ERR("Out of memory %d, %d!\n", outpitch, height);
return WINED3DERR_OUTOFVIDEOMEMORY;
}
- d3dfmt_convert_surface(This->resource.allocatedMemory, mem, pitch, width, height, outpitch, convert, This);
+ d3dfmt_convert_surface(This->resource.allocatedMemory, mem, pitch, width, height, outpitch, This);
This->Flags |= SFLAG_CONVERTED;
} else {
@@ -4537,8 +4535,7 @@ static HRESULT WINAPI IWineD3DSurfaceImpl_LoadLocation(IWineD3DSurface *iface, D
BOOL srgb = flag == SFLAG_INSRGBTEX;
struct wined3d_context *context = NULL;
- d3dfmt_get_conv(This, TRUE /* We need color keying */, TRUE /* We will use textures */,
- &desc, &convert);
+ d3dfmt_get_conv(This, TRUE /* We need color keying */, TRUE /* We will use textures */, &desc);
if(srgb) {
if((This->Flags & (SFLAG_INTEXTURE | SFLAG_INSYSMEM)) == SFLAG_INTEXTURE) {
@@ -4575,9 +4572,9 @@ static HRESULT WINAPI IWineD3DSurfaceImpl_LoadLocation(IWineD3DSurface *iface, D
width = This->currentDesc.Width;
pitch = IWineD3DSurface_GetPitch(iface);
- /* Don't use PBOs for converted surfaces. During PBO conversion we look at SFLAG_CONVERTED
- * but it isn't set (yet) in all cases it is getting called. */
- if(((convert != NO_CONVERSION) || desc.convert) && (This->Flags & SFLAG_PBO)) {
+ /* Don't use PBOs for converted surfaces. */
+//this change should be a separate patch and is quite safe these days
+ if((This->Flags & SFLAG_CONVERTED) && (This->Flags & SFLAG_PBO)) {
TRACE("Removing the pbo attached to surface %p\n", This);
surface_remove_pbo(This, gl_info);
}
@@ -4597,7 +4594,7 @@ static HRESULT WINAPI IWineD3DSurfaceImpl_LoadLocation(IWineD3DSurface *iface, D
return WINED3DERR_OUTOFVIDEOMEMORY;
}
desc.convert(This->resource.allocatedMemory, mem, pitch, width, height);
- } else if((convert != NO_CONVERSION) && This->resource.allocatedMemory) {
+ } else if((desc.Flags & WINED3DFMT_FLAG_CONVERTED) && This->resource.allocatedMemory) {
/* This code is only entered for color keying fixups */
int height = This->currentDesc.Height;
@@ -4611,7 +4608,7 @@ static HRESULT WINAPI IWineD3DSurfaceImpl_LoadLocation(IWineD3DSurface *iface, D
if (context) context_release(context);
return WINED3DERR_OUTOFVIDEOMEMORY;
}
- d3dfmt_convert_surface(This->resource.allocatedMemory, mem, pitch, width, height, outpitch, convert, This);
+ d3dfmt_convert_surface(This->resource.allocatedMemory, mem, pitch, width, height, outpitch, This);
} else {
mem = This->resource.allocatedMemory;
}
diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c
index 404768d..f5e276d 100644
--- a/dlls/wined3d/utils.c
+++ b/dlls/wined3d/utils.c
@@ -644,6 +644,10 @@ static const struct wined3d_format_texture_info format_texture_info[] =
ARB_TEXTURE_FLOAT, NULL},
/* Palettized formats */
{WINED3DFMT_P8_UINT, GL_RGBA, GL_RGBA, 0,
+ GL_RGBA, GL_UNSIGNED_BYTE, 4,
+ WINED3DFMT_FLAG_CONVERTED,
+ WINED3D_GL_EXT_NONE, NULL},
+ {WINED3DFMT_P8_UINT, GL_RGBA, GL_RGBA, 0,
GL_ALPHA, GL_UNSIGNED_BYTE, 0,
0,
ARB_FRAGMENT_PROGRAM, NULL},
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
index 0b22a56..75e7c6a 100644
--- a/dlls/wined3d/wined3d_private.h
+++ b/dlls/wined3d/wined3d_private.h
@@ -2964,6 +2954,8 @@ extern WINED3DFORMAT pixelformat_for_depth(DWORD depth) DECLSPEC_HIDDEN;
#define WINED3DFMT_FLAG_SRGB_WRITE 0x00001000
#define WINED3DFMT_FLAG_VTF 0x00002000
#define WINED3DFMT_FLAG_SHADOW 0x00004000
+#define WINED3DFMT_FLAG_CONVERTED 0x00008000
+#define WINED3DFMT_FLAG_COLOR_KEYING 0x00010000
struct wined3d_format_desc
{
