[D3D] Added native and emulated S3TC support

Lionel Ulmer lionel.ulmer at free.fr
Tue Jul 27 15:57:53 CDT 2004


I have just a few comments on the patch (yeah, it would not have been fun if
I had commented on wine-d3d first :-) ).

But anyway, great work !
	
> +static BOOL DDRAW_bind_to_s3tc( void )
> +{
> +    const char *s3tcname = SONAME_LIBS3TC;
> +
> +    s3tc_handle = wine_dlopen(s3tcname, RTLD_NOW, NULL, 0);
> +    if (!s3tc_handle) {
> +        printf("Wine cannot find the S3TC software decompression library (%s).\n",s3tcname);

Why did you use 'printf' here instead of WARN or ERR ?

Moreover, with this, we will print an error at DirectX loading when the
users do not have this library installed ... even if the game does never
request any compressed mode textures.

Would it not be better to somehow print the error message when the
application wants to create a DXTN texture (ie somehow remember that we do
not support this feature and keep the error message for later).

> +	return FALSE;
> +    }
> +
> +#define API_FUNCTION(f)  \
> +    if((f = wine_dlsym(s3tc_handle, #f, NULL, 0)) == NULL) \
> +    { \
> +        printf("Can't find symbol %s\n", #f); \

Same printf comment here (but this one can be kept here :-) ).

> +        goto sym_not_found; \
> +    }
> +    API_FUNCTION(fetch_2d_texel_rgba_dxt1);
> +    API_FUNCTION(fetch_2d_texel_rgba_dxt3);
> +    API_FUNCTION(fetch_2d_texel_rgba_dxt5);
> +#undef API_FUNCTION
> +
> +    return TRUE;
> +    
> +sym_not_found:
> +    WARN("Wine cannot find functions that are necessary for S3TC decompression\n");
> +    wine_dlclose(s3tc_handle, NULL, 0);
> +    s3tc_handle = NULL;
> +    return FALSE;
> +}
> +

(...)
>  
> +    if (src_pf->dwFlags & DDPF_FOURCC) {
> +	GLenum retVal;
> +	int size = surf_ptr->surface_desc.u1.dwLinearSize;
> +	int width = surf_ptr->surface_desc.dwWidth;
> +	int height = surf_ptr->surface_desc.dwHeight;
> +	LPVOID buffer = surf_ptr->surface_desc.lpSurface;
> +
> +	switch (src_pf->dwFourCC) {
> +	    case MAKE_FOURCC('D','X','T','1'): retVal = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT; break;
> +	    case MAKE_FOURCC('D','X','T','3'): retVal = GL_COMPRESSED_RGBA_S3TC_DXT3_EXT; break;
> +	    case MAKE_FOURCC('D','X','T','5'): retVal = GL_COMPRESSED_RGBA_S3TC_DXT5_EXT; break;
> +	    default:
> +		FIXME("FourCC Not supported\n");
> +		return DD_OK;
> +		break;
> +	}
> +	GL_extensions.glCompressedTexImage2D(GL_TEXTURE_2D, current_level, retVal, width, height, 0, size, buffer);

How are we sure that this code will never be called with the
'glCompressedTexImage2D' unset ? I.e. when we are in emulation mode, what
happens here ?

Is this because when we are in emulation mode, we do not advertise support
for DTXN textures, this means that the application can create such textures,
but will be automagically converted to RGB ones at Loading ?

Anyway, I would still prefer a test here to prevent crashes for application
which do not behave well :-)

(...)
> +    if (current_surface->surface_desc.u4.ddpfPixelFormat.dwFlags & DDPF_FOURCC) {
> +	GLint retVal;
> +	int size = current_surface->surface_desc.u1.dwLinearSize;
> +	int width_ = current_surface->surface_desc.dwWidth;
> +	int height_ = current_surface->surface_desc.dwHeight;
> +	LPVOID buffer = current_surface->surface_desc.lpSurface;
> +
> +	switch (current_surface->surface_desc.u4.ddpfPixelFormat.dwFourCC) {
> +	    case MAKE_FOURCC('D','X','T','1'): retVal = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT; break;
> +	    case MAKE_FOURCC('D','X','T','3'): retVal = GL_COMPRESSED_RGBA_S3TC_DXT3_EXT; break;
> +	    case MAKE_FOURCC('D','X','T','5'): retVal = GL_COMPRESSED_RGBA_S3TC_DXT5_EXT; break;
> +	    default:
> +		FIXME("Not supported\n");
> +		return DD_OK;
> +		break;
> +	}
> +	/* GL_extensions.glCompressedTexSubImage2D(GL_TEXTURE_2D, current_level, xoffset, yoffset, width, height, retVal, (unsigned char*)temp_buffer); */
> +	GL_extensions.glCompressedTexImage2D(GL_TEXTURE_2D, current_level, retVal, width_, height_, 0, size, buffer);

Same here.

> +	}
> +
> +	if (strstr(glExtensions, "GL_EXT_texture_compression_s3tc")) {
> +	    TRACE(" - S3TC compression supported\n");
> +	    GL_extensions.s3tc_compressed_texture = TRUE;
> +	    GL_extensions.glCompressedTexImage2D = pglXGetProcAddressARB("glCompressedTexImage2D");
> +	    GL_extensions.glCompressedTexSubImage2D = pglXGetProcAddressARB("glCompressedTexSubImage2D");
>  	}

If I were pedantic, you should test for 'GL_ARB_texture_compression' to be
able to get the 'glCompressedTexImage2DARB' and
'glCompressedTexSubImage2DARB' functions (which may be better than using the
non-ARB versions which are, AFAIK, the ones in GL 1.4).

And then to get 'GL_EXT_texture_compression_s3tc' to set
'GL_extensions.s3tc_compressed_texture' to TRUE.

But as 'GL_EXT_texture_compression_s3tc' depends on
'GL_ARB_texture_compression', I will let you do this shortcut :-)

> -    /* We do not support for now compressed texture formats... */
> -    if (ddsd.u4.ddpfPixelFormat.dwFlags & DDPF_FOURCC)
> +    /* We support for now only DXT1, DXT3 & DXT5 compressed texture formats... */
> +    if ((ddsd.u4.ddpfPixelFormat.dwFlags & DDPF_FOURCC) &&
> +        (ddsd.u4.ddpfPixelFormat.dwFourCC != MAKE_FOURCC('D','X','T','1')) &&
> +        (ddsd.u4.ddpfPixelFormat.dwFourCC != MAKE_FOURCC('D','X','T','3')) &&
> +        (ddsd.u4.ddpfPixelFormat.dwFourCC != MAKE_FOURCC('D','X','T','5')) )
>      {
>          return DDERR_INVALIDPIXELFORMAT;
>      }

What to do here when neither hardware nor emulated support is present ?
Maybe we should print an error (cf previous comment) and return
INVALIDPIXELFORMAT, no ?

        Lionel (who goes testing NOFL :-) )

-- 
		 Lionel Ulmer - http://www.bbrox.org/



More information about the wine-devel mailing list