Re: [2/6] d3dx9: Implement D3DXCheckTextureRequirements

[Henri Verbeet]

On 20 May 2010 23:05, Owen Rudge <orudge(a)codeweavers.com> 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, ¶ms);
> +
> +        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;
}