Re: [PATCH] d3dx9: Move object initialization into a separate function.
On 09.10.2013 01:12, Matteo Bruni wrote: Hi Rico, 2013/10/8 Rico Schüller kgbric...@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. 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. 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
Re: [PATCH] d3dx9: Move object initialization into a separate function.
2013/10/9 Rico Schüller kgbric...@web.de: On 09.10.2013 01:12, Matteo Bruni wrote: Hi Rico, 2013/10/8 Rico Schüller kgbric...@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
Re: [PATCH] d3dx9: Move object initialization into a separate function.
Hi Rico, 2013/10/8 Rico Schüller kgbric...@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. 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. 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. Cheers Rico --- dlls/d3dx9_36/effect.c | 98 ++ 1 Datei geändert, 34 Zeilen hinzugefügt(+), 64 Zeilen entfernt(-) Cheers, Matteo
