Re: d3dx9: Implementation of D3DXGetShaderSize

[H. Verbeet]

On 06/04/2008, Luis Busquets  wrote:
> This function obtains the size of a shader bytestream. It does so looking
> for the End token.
> +UINT D3DXGetShaderSize(DWORD * pFunction)

I would expect this to take a const pointer. Also, there's no reason
to use Microsoft's (broken) naming conventions for function arguments.
> - while (*address!=0x0000ffff) {
> - /*Let's check if the next DWORD is a comment*/
> - if ((*address&0x0000ffff)==0x0000fffe) {
> -
>            address=address+((*address >> 0x10) & 0x00007fff);
> -
>            }
> - address=address+0x1;
> - }

It would help readability quite a bit to use the proper constants
here. Also, please try to use an indentation style consistent with the
rest of the dll and wine (this goes for the other patches as well).
> - size=(address-pFunction+0x1)*sizeof(DWORD);

Why the +1 here? Does it return the size without the end token itself?
Tests and comments are always helpful there.

The following (untested!) implementation looks more reasonable to me:

UINT D3DXGetShaderSize(const DWORD *byte_code)
{
    const DWORD *ptr = byte_code;

if (!ptr) return 0;

/* Look for the END token, skipping the VERSION token */
    while (*++ptr != D3DSIO_END) {
        /* Skip comments */
        if ((*ptr & D3DSI_OPCODE_MASK) == D3DSIO_COMMENT) {
            ptr += ((*ptr & D3DSI_COMMENTSIZE_MASK) >> D3DSI_COMMENTSIZE_SHIFT);
        }
    }

/* Return the shader size in bytes, excluding the END token. */
    return (ptr - (byte_code + 1)) * sizeof(*ptr);
}