Hello,

I have a little design problem.

I have a Material class, that always has a VertexShader and PixelShader and additionally tons of states (Textures, Culling etc.)
The problem is: Many objects will use the same Material but the Material should only created ONCE (the first time the Material is acquired) and all
other requests to that material the already created Material should just be returned.

The only way I can think of is this:

Material* mat = manager->getMaterial("MyMaterial");
if(mat == NULL) {
// This is the first time someone wants to use this material. So create it
mat = manager->getMaterial("MyMaterial");
VertexShader* vs = manager->loadVertexShader("shader.vsh");
PixelShader* ps = manager->loadVertexShader("shader.psh");
mat->setVS(vs);
mat->setPS(ps);
mat->setTexture(0, myTexture);
object->setMaterial(mat);
}
else {
object->setMaterial(mat);
}


The problem is: I have to use this whole code block EVERYTIME I need this material. Cause I can never be sure if the material is already loaded.
This is really ugly:(

Does anyone has an idea how I could simplify this?

If you need to do it every time, the obvious solution is to create a function that contains that functionality.

@japro: Actually I exactly have the same design, but that does not solve my problem. What if loadFoo() needs more parameters? Then you have to pass them to get() EVERY time, which is kinda ugly. Plus what if the returned Foo needs additional configuration after get (your Foo is like my Material), but only once?

[quote name='schupf' timestamp='1312735613' post='4845816']
@japro: Actually I exactly have the same design, but that does not solve my problem. What if loadFoo() needs more parameters? Then you have to pass them to get() EVERY time, which is kinda ugly. Plus what if the returned Foo needs additional configuration after get (your Foo is like my Material), but only once?

What would be an example where load needs more parameters? Either those parameters belong to the object in which case they should be loaded together with it, if they are global parameters (load every texture into this type of format) then that should be a parameter of the cache itself that you set with some "FooCache::setParameter" method globally or the parameters are situation dependent in that case they should be somehow passed to the Foo object itself or be applied via some sort of decorator pattern and not be passed to its cache. At least these are the cases I can think of.
[/quote]
For example loadShader(). My loadShader() method looks like this:
ShaderPtr loadShader(const string& filename, const string& entryFunction);

Usage:
obj->VS = manager->loadShader("myShader.vsh", "main");
In order to load (which involves compiling) the shader I need the name of the entry function (cause D3DXCompileShaderFromFile needs it). Its kinda awkward to pass the function name everytime I need the shader (even though actually I only need it for the first time)

Nitpicking on style:
Foo* get(std::string name)
{
std::map<std::string, Foo*> result = foomap.find(name);
if (result == foomap.end()) {
result = foomap.insert(result, make_pair(name, loadFoo(name)));
}
return result->second;

}


Technically, this is less efficient since it's pessimistic (item is expected to not be in cache) while CPU branch prediction expects optimistic branch (if statement should favor common case).

But since map doesn't allow fast query (such as hash comparisons for approximate match) and due to other implementation details it's a minor point, just something to be aware of.


Alternate reusable version:
template < class K, class T, class C, class A, class Factory >
T get_cached(std::map<K,T,C,A> & map, K key, Factory factory) {
typename std::map<K,T,C,A>::iterator = map.find(name);
if (result == map.end()) {
result = foomap.insert(result, make_pair(name, factory(key)));
}
return result->second; }

...
// example factory
Foo * file_loader(string filename) {
// load from file
return new Foo(file_data);
}


I'm not entirely sure if this falls under AOP or mixins or similar, but one augments an existing class with completely orthogonal functionality. get_cached allows any map to now act as cache simply by providing a factory function.

Approach is especially useful when working with third-party code where modification of existing classes isn't possible or desirable.

What if loadFoo() needs more parameters?[/quote]
If parameters don't change between invocations, then consider the Factory example above:

struct StatefulLoaderFactory {
StatefulLoaderFactory(X p1, X p2, X p3);

Foo * operator()(filename f) {
do_the_load(filename, param1, param2, param3);
}
private:
X param1;
X param2;
X param3;
};

...
StatefulLoaderFactory f(10, 200, 55);
...
// works because of templates:
get_cached(map, "some_file.txt", f);


Material* mat = manager->getMaterial("MyMaterial");
if(mat == NULL) {
// This is the first time someone wants to use this material. So create it


Don't do that.

You're putting in a branch point you don't need. I presume the second call to "getMaterial" should be "createMaterial". Also, you're not checking the error states of anything.

You're putting loads in unpredictable places. You now have several possible disk accesses (which are slow) plus shader compiles inside your rendering cycle. It means the first time a given material appears in view you're going to get chunky frame times.

Just go and load up all the stuff you need before you start. Presumably you are loading up the models anyway which name the material types, so just stick them in a list, then create them all. Then you can go through all the models setting the material pointer. Then you can start rendering, and now you know that all the bits are in place, so you don't need to worry about getting nulls back from your getMaterial call.

Don't put ANYTHING you don't need in your render loops -- anything you can do upfront, do it upfront.