[PATCH] d3dx9_36: Implement D3DXGetShaderInputSemantics + tests.

[Matteo Bruni]

2013/6/12 Christian Costa <titan.costa at gmail.com>:
>
>> > +HRESULT WINAPI D3DXGetShaderInputSemantics(const DWORD *byte_code,
>> > D3DXSEMANTIC *semantics, UINT *count)
>> > +{
>> > +    const DWORD *ptr = byte_code;
>> > +    UINT i = 0;
>> > +
>> > +    TRACE("byte_code = %p, semantics = %p, count = %p\n", byte_code,
>> > semantics, count);
>> > +
>> > +    if (!byte_code)
>> > +        return D3DERR_INVALIDCALL;
>> > +
>> > +    TRACE("Shader version: %#x\n", *ptr);
>> > +
>> > +    /* Look for the END token, skipping the VERSION token */
>> > +    while (*++ptr != D3DSIO_END)
>>
>> I think you should be a bit more careful in skipping non-opcode
>> DWORDs, otherwise you could e.g. potentially match with constants from
>> DEF instructions - very unlikely to happen in practice, sure, but
>> since it can be easily avoided, why not?
>> Take a look at shader_skip_opcode() from wined3d/shader_sm1.c. You can
>> probably get away without having to write a table with the parameters
>> count for each SM1 opcode by just skipping DWORDs with the most
>> significant bit set (see shader_skip_unrecognized() from the same
>> file). Of course you still have to special case DEF but that should be
>> it.
>
>
> I was thinking about this kind of problem but followed what
> D3DXGetShaderSize does.
> So D3DXGetShaderSize will have to be fixed as well.
> So if I summarize:
>
> if (sm > 2) handle instruction length
> else if (comment or def instruction) special handling for them
> else skip DWORD with bit 31 set
>
> Is this correct?
>

SM >= 2 for the instruction length case, but apart from that it should be good.

Ideally we'd use some tests, as Henri suggested, but that's not
blocking this patch I think.

>
>>
>> > +    {
>> > +        /* Skip comments */
>> > +        if ((*ptr & D3DSI_OPCODE_MASK) == D3DSIO_COMMENT)
>> > +        {
>> > +            ptr += ((*ptr & D3DSI_COMMENTSIZE_MASK) >>
>> > D3DSI_COMMENTSIZE_SHIFT);
>> > +        }
>> > +        else if ((*ptr & D3DSI_OPCODE_MASK) == D3DSIO_DCL)
>> > +        {
>> > +            DWORD param1 = *++ptr;
>> > +            DWORD param2 = *++ptr;
>> > +            DWORD usage = param1 & 0x1f;
>> > +            DWORD usage_index = (param1 >> 16) & 0xf;
>> > +            DWORD reg_type = (((param2 >> 11) & 0x3) << 3) | ((param2
>> > >> 28) & 0x7);
>> > +
>> > +            TRACE("D3DSIO_DCL param1: %#x, param2: %#x, usage: %u,
>> > usage_index: %u, reg_type: %u\n",
>> > +                   param1, param2, usage, usage_index, reg_type);
>> > +
>> > +            if (reg_type == D3DSPR_INPUT)
>> > +            {
>> > +                if (semantics)
>> > +                {
>> > +                    semantics[i].Usage = usage;
>> > +                    semantics[i].UsageIndex = usage_index;
>> > +                }
>> > +                i++;
>> > +            }
>> > +        }
>> > +    }
>> > +
>> > +    if (count)
>> > +        *count = i;
>> > +
>> > +    return D3D_OK;
>> > +}
>>
>> Have you tried to implement D3DXGetShaderOutputSemantics too? I
>> suspect most of the code will be pretty much the same, in that case
>> you could move the common code to a helper function and use it from
>> both.
>> You don't necessarily need to do this right now, just mentioning a
>> potential follow-up.
>>
> I tought about it too. There are indeed similar. I planned to do it later
> but I will do it in this patch since I have to update it anyway.
>
> Thanks
> Christian