[PATCH] d3dx9: Move object initialization into a separate function.

[Matteo Bruni]

2013/10/9 Rico Schüller <kgbricola at web.de>:
> On 09.10.2013 01:12, Matteo Bruni wrote:
>>
>> Hi Rico,
>>
>> 2013/10/8 Rico Schüller <kgbricola at web.de>:
>>>
>>> Hi,
>>>
>>> this moves the object initialization into a separate function, so it
>>> could
>>> be used for strings and resources. It also removes the STATE_TYPE as we
>>> could distinguish the types at the object level.
>>>
>>> 1. When an object has a destination, it points to another shader
>>> variable.
>>> This was state ST_PARAMETER.
>>>
>>> 2. If a variable has something in data, it is fxlc, shader (preshader) or
>>> a
>>> string. This was state ST_CONSTANT and ST_FXLC.
>>>
>>> 3. If it has both (destination and data), it points to an array of shader
>>> variables. The name is in the destination, the index could be calculated
>>> with the data. This will be added in a later patch.
>>>
>>
>> There's still the issue of distinguishing between ST_CONSTANT and
>> ST_FXLC, checking object_id and type might cover that though.
>
> I think we could distinguish between them. I forgot to add, if both are 0,
> then it is a constant and the parameter data has already the correct value.
> Marking the shader as ST_CONSTANT was a little bit wrong, as we would need
> to set the shader/preshader variables.
>
>
>>
>>> Also saving the destination parameter in the object gains some speed when
>>> we
>>> need to access the variable as we don't need to run
>>> get_parameter_by_name()
>>> each time we need the variable ...
>>>
>>
>> I'm not sure storing additional info into the objects is the right
>> thing to do. Take a look at the D3DXFX_NOT_CLONEABLE flag from
>>
>> http://msdn.microsoft.com/en-us/library/windows/desktop/bb172855%28v=vs.85%29.aspx.
>> Notice that GetPassDesc() doesn't return the shader data if the object
>> was created with the flag. What I've been thinking is that it simply
>> can't because the original shader data, stored in an object, were
>> freed after parsing.
>
> Yeah, I'm aware of D3DXFX_NOT_CLONEABLE. But we need to hold the shader blob
> or something similar (a reflection of the used preshader variables) to set
> the correct values. We also need it in the case for when the parameter needs
> to be calculated (fxlc) and in for strings. So imho, we could only release
> the blob partially when we have a preshader, nowhere else (and in this case
> we still need the "reflection" somehow). So let me conclude: We need to
> store the "destination" and the "reflection" somewhere. We may release the
> full shader binary.
>

Yeah, we need to store something for those cases, but not necessarily
the original shader blob (we could store some derived information
instead). So I essentially agree here.

>
>> While nothing forces us to do the same (except probably avoiding to
>> use more memory than strictly necessary) I think it's better not to
>> put additional stuff into the objects or generally assume that the
>> objects are still available after parsing. That means creating the
>> shaders and the strings at parse time or right after that and storing
>> any additional required information (e.g. preshader) in the parameter.
>>
>> So, directly storing the referenced parameter is a good idea but I'd
>> prefer that pointer to be in d3dx_parameter.
>
> I haven't put it in the parameter as there are much more parameters than
> objects. Each state, each array element and each structure member has a
> parameter while there are only some parameters that have objects. So we may
> use some more bytes when putting it in the parameter than putting it in the
> object.
>

True, my point is that the memory reclaimed by freeing the objects is
probably more than the memory wasted by some additional pointers. I
don't have any hard data though (and applications might "forget" to
specify D3DXFX_NOT_CLONEABLE) so I might be wrong.

> I'm fine with both ways, because each object is tight to a specific
> parameter, it's mostly a matter of taste where the data is stored.
>
> Cheers
> Rico