Re: [2/6] d3dx9: Implement D3DXCheckTextureRequirements

[Henri Verbeet]

On 20 May 2010 23:05, Owen Rudge  wrote:
> +BOOL is_pow2(UINT num);
> +UINT make_pow2(UINT num);

I'm not so convinced these are going to be very useful for anything
outside texture.c.
> +BOOL is_pow2(UINT num)
> +{
> - if (!num) return FALSE;
> -
> - return !(num & (num - 1));
>
> +}

Do you really care that 0 is not a real power of two for these texture
functions? It's not a valid width / height anyway.
> -
>    if ((caps.TextureCaps & D3DPTEXTURECAPS_POW2) && (!is_pow2(*width)))
> -
>        make_pow2(*width);

...
> -
>    if ((caps.TextureCaps & D3DPTEXTURECAPS_POW2) && (!is_pow2(*height)))
> -
>        make_pow2(*height);

You have to assign to result to something for it to be of any use.
> -
>        max_mipmaps = 9;

Where does that 9 come from? (I can easily guess, but...)
> -
>    hr = IDirect3DDevice9_GetDirect3D(device, &d3d);
> -
> -
>    if (SUCCEEDED(hr))
> -
>        IDirect3DDevice9_GetCreationParameters(device, &params);
> -
> -
>    if (SUCCEEDED(hr))
> -
>        IDirect3DDevice9_GetDisplayMode(device, 0, &mode);
> -
> -
>    if (SUCCEEDED(hr))

It's probably nicer to check for failure and jump to some cleanup
code. Also, you should check all the calls for errors, not just
GetDirect3D().

The width / height handling looks a bit complicated to me. You should
probably first calculate the width and height using local variables,
and only assign those to *width/*height afterwards. Something along
these lines:

{
    UINT w = (width && *width) ? *width : 1;
    UINT h = (height && *height) ? *height : 1;

if (w == D3DX_DEFAULT && h == D3DX_DEFAULT)
        w = h = 256;
    else if (w == D3DX_DEFAULT) w = h;
    else if (h == D3DX_DEFAULT) h = w;

...

if (width) *width = w;
    if (height) *height = h;
}