Re: [PATCH] d3dx9_36: Implement D3DXGetShaderInputSemantics + tests.

[Christian Costa]

> > +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?
> > - {
> > -
> >    /* 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
> > > - & 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