Re: [PATCH] wined3d: Implement check for shader creation if hardware supports requested version.

[Henri Verbeet]

2008/12/17 Pauli Nieminen :
> +static inline void test_create_vshader_version_check(IDirect3DDevice9 *device_ptr, const D3DCAPS9 *caps,
> -
>    const DWORD version, const DWORD *shader_code)
>
> +{
> - IDirect3DVertexShader9 *vshader_ptr = 0;
> - HRESULT hret = 0;
> -
> - hret = IDirect3DDevice9_CreateVertexShader(device_ptr, shader_code, &vshader_ptr);
> -
> -
> - if( version <= caps->VertexShaderVersion )
> - {
> -
>    ok(hret == D3D_OK && vshader_ptr != NULL, "Vertex shader (0x%x) creation failed but d3dcaps claim to support it. hret = 0x%x, vshader_ptr = %p", version, hret, vshader_ptr);
> - } else {
> -
>    ok(hret == D3DERR_INVALIDCALL && vshader_ptr == NULL,"Vertex shader (0x%x) creation succesed but d3dcaps claim not to support it. hret = 0x%x, vshader_ptr = %p", version, hret, vshader_ptr);
> - }
> -
>
> +}

If creation succeeds, you need to Release the shader again. Same goes
for the pixel shader version of this function.
> - ok(hret == D3D_OK && shader_refcount == i && current_shader_ptr == shader_ptr,
> - ok(hret == D3D_OK && shader_refcount == i && current_shader_ptr == shader_ptr,

I don't particularly like the trailing space either, but you shouldn't
make unrelated whitespace changes in your patch.
> +static inline BOOL is_shader_version_supported(IWineD3DDeviceImpl *deviceImpl, const DWORD *byte_code)
> +{
> +//     IWineD3DPixelShaderImpl *This = (IWineD3DPixelShaderImpl *)shader;
> -
>   struct shader_caps shader_caps;
> -
>   const DWORD shader_version = *byte_code; /* First instruction should be shader version */
> - const WineD3D_GL_Info *gl_info = &deviceImpl->adapter->gl_info;

Please don't use tabs or C++ style comments.
> /* Note that this does not count the loop register
> - as an address register. */
>
> @@ -216,7 +244,27 @@ HRESULT shader_get_registers_used(IWineD3DBaseShader *iface, struct shader_reg_m
>     memset(reg_maps->bumpmat, 0, sizeof(reg_maps->bumpmat));
>     memset(reg_maps->luminanceparams, 0, sizeof(reg_maps->luminanceparams));
> - /* get_registers_used is called on every compile on some 1.x shaders, which can result
> -
>   /* If no hardware support this function should fail */
> -
>   if (!is_shader_version_supported((IWineD3DDeviceImpl*)This->baseShader.device, pToken))
> -
>   {
> -
>           static BOOL warned = FALSE;
> -
>           if (!warned)
> -
>           {
> -
>                   warned = TRUE;
> -
>                   ERR("No hardware support for shader version.\n");
> -
>           }
> -
>           return WINED3DERR_INVALIDCALL;
> -
>   }
> -
> -
> - if (!pToken)
> - {
> -
>    WARN("Got a NULL pFunction, returning.\n");
> -
>    This->baseShader.functionLength = 0;
> -
>    return WINED3D_OK;
> - }
> -
> -
>   /* get_registers_used is called on every compile on some 1.x shaders, which can result

I don't think shader_get_registers_used is the right place for this.
The version check should be in IWineD3DVertexShaderImpl_SetFunction()
and IWineD3DPixelShaderImpl_SetFunction(), after the call to
shader_get_registers_used(). Putting it there means you can use the
shader version in reg_maps.shader_version, and you can also check if
the shader is actually a vertexshader or pixelshader. Ie, passing a
pixel shader to  CreateVertexShader() should most likely fail.