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