Sign in to follow this  
wforl

Manager Class

Recommended Posts

wforl    169
Im trying to create a single class that contains a container, with which holds pointers to a bunch of effect objects, such as
Manager Effects;

LightEffect* TwoLight = new LightEffect();

Effects.add(TwoLight);


Where, the manager class contains , a container of type

std::vector<effect*>FX_List;



and where effect is an effect class, with common variables, that all other effects derive from to customize themselves
class effect{
//common attributes
};

class TwoLight : public effect{
}

class InfrRedLight : public effect{
}

class Blur: public effect{
}

class Drunk: public effect{
}


now effects render and build effect etc methods will be virtual, so that each derived class implements them, themselves. But im wondering a few things. 1. How to actually access the manager? The only ways i can think of is one of these 3 Effects->FX_List[4]->render() //which is an waful way the other way is with an enum, but this requires the effects to be pushed back in order of the enum, which isnt great enum effects { effect1 = 0, blur, drunken, weird}; Effects->FX_List[effect]->render(); and finally this way Effects->add(blurEffect); int blur = Effects->FX_List.size() - 1; Effects->FX_List[blur]->render(); all of which still need wrapping up in nicer methods, but im still thinking there could be a better way. And the other question, is how to set up each effect before rendering, when you only have access to the EffectManager. With the vector containing a list of the base type pointer, and made virtual, i can correctly call the derived render() and build() methods, but i can't access the members that are defined in the derived classes, as i only have a base pointer, unless i cast them of course, but this is still a little awkward. any advice?

Share this post


Link to post
Share on other sites
rip-off    10979
I don't see what additional functionality the Manager adds to the vector it holds as a member. Why not (ownership* issues asside) use an instance of std::vector<Effect *> as "Effects".

As for your question, why not render all the objects in one go, by iterating over the entire vector, calling render() on all instances. Why call it on individual instances? Doing it that way isn't much different from having lots of different pointers and no collection at all.

There should be no need for a build() member, you should be able to do such work in the constructor.

Back to ownership issues, having a vector of raw pointers leaves the possibility of leaks. An instance of boost::ptr_vector<X> or std::vector< boost::shared_ptr<X> > (depending on whether sharing is required) is a better solution, in general.

* Ownership:

In C++, ownership is all about which objects destroy each other. For example, if the Manager destroys the Effect instances, then the Manager is said to "own" the pointers. If the Effects can be deleted by the Manager or by another class, the ownership is said to be "shared". There are ways of implementing sharing that make this easier than it sounds.

Well defined ownership policy is very important as it both ensures objects are deleted when they are no longer required, and ensures that objects aren't destroyed before their time.

Share this post


Link to post
Share on other sites
dmatter    4844
Use an associative container:

typedef boost::shared_ptr<effect> EffectHandle;
typedef std::map<std::string, EffectHandle> EffectMap;

EffectMap effects;

effects["blur"].reset(new Blur());



Quote:
Original post by wforl
And the other question, is how to set up each effect before rendering, when you only have access to the EffectManager.
At some point you have to construct these effects, do it through their constructors.

Quote:
i can correctly call the derived render() and build() methods, but i can't access the members that are defined in the derived classes, as i only have a base pointer, unless i cast them of course, but this is still a little awkward.
Try and design it so that you don't need access to the subclasses. The effect class provides it's interface, hopefully that should be all that is needed.

Share this post


Link to post
Share on other sites
wforl    169
Thanks for the reply

The manager class does a lot more than what i have showed, so i want to keep it in.

The build function is called in the constructor, i just took it out here to make things simpler.

I cant really just iterate over everything in one go, as thats not really what this class was designed for, it was designed for me to be able to create classes that do other stuff (terrain, water, etc) , and have them point to whatever effect i want in the manager for the effect i want them to render as.

The problem is that before an effect can be rendered it has to be set up, which will involve setting up members in both the base class and the derived class. Such as i might have to set up the World matrix in the base class, but also the light diffuse or light position etc in the derived class. And these adjustments have to be made every frame, hence my problematic situation

thankyou

Share this post


Link to post
Share on other sites
dmatter    4844
Quote:
Original post by wforl
The manager class does a lot more than what i have showed, so i want to keep it in.
Managers almost always do more than one thing. Both in the real world and in software design. That's why they're almost always not good designs. [smile]

Quote:
The build function is called in the constructor, i just took it out here to make things simpler.
Keep in mind that you cannot reliably use functions polymorphically from a constructor.

Quote:
I cant really just iterate over everything in one go, as thats not really what this class was designed for, it was designed for me to be able to create classes that do other stuff (terrain, water, etc) , and have them point to whatever effect i want in the manager for the effect i want them to render as.
Hmm, so I don't understand why do you need a manager to do that? These other renderable objects could just hold shared_ptrs to your effects without the need of a central manager. That's not to say that having some kind of collection of all effects isn't useful, just that it isn't necessary to implement what you've just described.

Quote:
The problem is that before an effect can be rendered it has to be set up, which will involve setting up members in both the base class and the derived class. Such as i might have to set up the World matrix in the base class, but also the light diffuse or light position etc in the derived class. And these adjustments have to be made every frame, hence my problematic situation
Code outside of the effect classes shouldn't care about the attributes contained within the effect classes - that violates encapsulation.
Instead how about creating something along the lines of a RenderDescriptor that simply describes a bunch of high level states and have the effects set themselves up based on that descriptor.

So the descriptor might provide a transform that the effects can use to set up the world matrix for example. Or it might provide a way to access the nearest N lights to an object in order to set up the internal lighting attributes.

Share this post


Link to post
Share on other sites
wforl    169
Yes, im beginning to realise now why this whole thing is a bad idea, idont think its going to work as i imagined.

Basically im trying to create a box of tricks, by where anyone can access a trick from the box, and set any of that tricks parameters to their desire, and then use that trick.

But, as you've mentioned its going to involve making everything public, and having some sort of method or descripter is'nt going to work either, as it would require soooo much code, as the list of shaders is going to end up being in the 20's or more, with most shaders having completly different variables and functions,and it really is just going to make things error prone.

I will make a simplified version of what i want to do and post it up, and maybe it will make things clearer, as to the problems im facing, what i want to do, and what sucks about my system :)

Share this post


Link to post
Share on other sites
dmatter    4844
Quote:
Original post by wforl
But, as you've mentioned its going to involve making everything public, and having some sort of method or descripter is'nt going to work either, as it would require soooo much code, as the list of shaders is going to end up being in the 20's or more, with most shaders having completly different variables and functions,and it really is just going to make things error prone.
I suppose that depends how you implement it. I would be trying to find a data-centric approach based on rich high level states, naturally compromises would have to be made but that's what software engineering is all about [smile]

Purely as proof of concept it certainly is possible to do something akin to passing an arbitrary number of variables with variant or arbitrary types using a std::map and either boost::variant or boost::any, example:

typedef std::map<std::string, boost::any> PropertyMap;

PropertyMap params;
params["transform"] = transform();
params["age"] = age();
params["seven"] = 7;
params["name"] = "wforl";

// functions needn't care about parameters they're not interested in
someFunctionRequiringTransformAndAge(params);
someFunctionRequiringAgeAndName(params);
Quote:
I will make a simplified version of what i want to do and post it up, and maybe it will make things clearer, as to the problems im facing, what i want to do, and what sucks about my system :)
Good idea - A new thread for it might be best too.

Share this post


Link to post
Share on other sites
wforl    169
Well here it is, a *very* simplified version of what im trying to achive, to which the only way i can think of getting to work is through static casts

#include <windows.h>
#include <stdio.h>
#include <iostream>
#include <vector>

using namespace std;


class BaseEffect
{
public:
BaseEffect(const char * dir) : Tech(0), WVP(0){
this->Build(dir);
}

void SetStatistics() const {
cout << "In BaseEffect SetStatistics" << endl;
cout << "...storing vertex/triangle/texture count etc" << endl;
*(this->WhereToUpdateTris) += 55;
*(this->WhereToUpdateVerts) += 78;
}

virtual void Render() const {
this->SetStatistics();
cout << "In BaseEffect Render" << endl;
}
virtual void Build(const char * dir) {

cout << "In BaseEffect Building base part of effect" << endl;
}

int Tech;
int WVP;

int* WhereToUpdateTris;
int* WhereToUpdateVerts;
};

class HeightEffect : public BaseEffect
{
public:
HeightEffect(const char * dir) : BaseEffect(dir) , MaxHeight(10){
this->Build(dir);
}
virtual void Render() const {
BaseEffect::Render();
cout << "In HeightEffect Render" << endl;
cout << "Rendering with, MaxHeight:" << this->MaxHeight << endl;
}
virtual void Build(const char * dir) {
cout << "In HeightEffect Build" << endl;
cout << "Building Specific HeightEffect stuff" << endl;
}

int MaxHeight;
int MinHeight;
int Colour;

};

class FX_Manager
{
public:
vector<BaseEffect*>FX_List;
FX_Manager() : numTris(0), numVerts(0){}

void Add(BaseEffect*in){
in->WhereToUpdateTris = &this->numTris;
in->WhereToUpdateVerts = &this->numVerts;
this->FX_List.push_back(in);
}

void Stats(){
cout << "verts: " << this->numVerts
<< " Tris: " << this->numTris;
}
private:
int numTris;
int numVerts;

};



void main()
{
FX_Manager Effects;

BaseEffect *t = new BaseEffect("dir");
Effects.Add(t);

HeightEffect *r = new HeightEffect("dir");
Effects.Add(r);

cout << endl;

Effects.FX_List[0]->Render();

cout << endl;
cout << endl;

for( int i = 0; i < 5; ++i){
static_cast<HeightEffect*>(Effects.FX_List[1])->MaxHeight = i;
Effects.FX_List[1]->Render();
}

Effects.Stats();
}




Share this post


Link to post
Share on other sites
dmatter    4844
I'll have a go at transforming your example into something that is closer to the type of thing I was talking about:

  • Firstly, your manager is handling two jobs, it's maintaing statistical data and it's maintaing a collection of effects.
    So let's give the statistical stuff to a PrimitiveStats class and use a boost::ptr_map for the collection of effects and ditch the manager.

    The PrimitiveStats class:
    class PrimitiveStats
    {
    public:
    typedef unsigned long SizeType;

    PrimitiveStats() : tris(0), verts(0) { }

    void advanceTris(SizeType tris) { this->tris += tris; verts += (tris * 3); }
    void advanceTriStrip(SizeType tris) { this->tris += tris; verts += (tris + 2); }

    SizeType triCount() const { return tris; }
    SizeType vertCount() const { return verts; }

    void reset() { tris = verts = 0; }

    private:
    SizeType tris;
    SizeType verts;
    };

    // Allow us to write the stats to an output stream
    std::ostream & operator<<(std::ostream & os, const PrimitiveStats & ps)
    {
    os << "Tris: " << ps.triCount() << "\n";
    os << "Verts: " << ps.vertCount() << std::endl;
    return os;
    }

    The collection of effects:

    typedef boost::ptr_map<std::string, BaseEffect> EffectDepository;

  • Secondly, let's tidy up your effect classes; lots to do here. The build functions are useless; let the constructor do it. Root base classes really shouldn't have any data attributes to avoid slicing; nor should they have any public virtual functions, apart from the destructor, for a stable interface - your existing render function could do with a refactoring anyway. I've moved the implementation of setStatistics function up to the derived HeightEffect class and also renamed it to updateStatistics. Root base classes should be abstract too so we now have pure virtual render and updateStatistics functions in EffectBase.

  • Thirdly, the PrimitiveStats object will now be passed to the render function (and therefore to the updateStatistics function), that gives you more flexibility with the retrieval of stats.

  • Fourthly, I've made an extremely crappy RenderDescriptor class. It holds a Box instance (meant to represent a bounding box) which is also a pretty poor implementation but you get the idea. The descriptor also has the ability to dynamically attach arbitrary attributes for added flexibility with unforeseen future effects or rapid prototyping. The purpose of the RenderDescriptor is to store high-level but generalised state like, in this case, the bounding box of some renderable entity. Effects can then use this high-level information to configure the graphics pipeline using low-level states (shaders, textures, vertex buffers, etc).

    The RenderDescriptor and Box classes:
    class Box
    {
    public:
    Box(int x1, int y1, int x2, int y2) : x1(x1), y1(y1), x2(x2), y2(y2) { }
    int x1, y1, x2, y2;
    };

    // Calculates the height of a box
    int height(const Box & b)
    {
    return (b.y2 > b.y1) ? b.y2 - b.y1 : b.y1 - b.y2;
    }

    class RenderDescriptor
    {
    public:
    RenderDescriptor(const Box & bounding) : bounding(bounding) { }

    Box bounding;
    // other fixed properties here

    // offers the ability to add new properties dynamically
    typedef std::map<std::string, boost::any> PropertyMap;
    PropertyMap attribs;
    };


    A RenderDescriptor object will also need to be passed to the render function.

  • Penultimately, I have made the HeightEffect class use the bounding box from the RenderDescriptor to calculate it's maxHeight within the render function. The implication is that the class no longer needs to keep hold of such things as member attributes because they're now calculated dynamically - it certainly could have member attributes if the need arised however.

  • Finally, the main function needs to change. We can no longer instantiate a BaseEffect as a standalone effect (but why would you want to really?) because it is now not a concrete class. In order to construct a suitable RenderDescriptor from the for-loops counter (i) I've made a convenience function called makeRenderData which takes the counter and returns a descriptor instance.


This is the (untested, sorry) code in full:
#include <iostream>
#include <string>
#include <map>

#include <boost/any.hpp>
#include <boost/ptr_container/ptr_map.hpp>


class PrimitiveStats
{
public:
typedef unsigned long SizeType;

PrimitiveStats() : tris(0), verts(0) { }

void advanceTris(SizeType tris) { this->tris += tris; verts += (tris * 3); }
void advanceTriStrip(SizeType tris) { this->tris += tris; verts += (tris + 2); }

SizeType triCount() const { return tris; }
SizeType vertCount() const { return verts; }

void reset() { tris = verts = 0; }

private:
SizeType tris;
SizeType verts;
};

std::ostream & operator<<(std::ostream & os, const PrimitiveStats & ps)
{
os << "Tris: " << ps.triCount() << "\n";
os << "Verts: " << ps.vertCount() << std::endl;
return os;
}


class Box
{
public:
Box(int x1, int y1, int x2, int y2) : x1(x1), y1(y1), x2(x2), y2(y2) { }
int x1, y1, x2, y2;
};

int height(const Box & b)
{
return (b.y2 > b.y1) ? b.y2 - b.y1 : b.y1 - b.y2;
}

class RenderDescriptor
{
public:
RenderDescriptor(const Box & bounding) : bounding(bounding) { }

Box bounding;
// other fixed properties

// misc properties
typedef std::map<std::string, boost::any> PropertyMap;
PropertyMap attribs;
};


class BaseEffect
{
public:
BaseEffect()
{
std::cout << "In BaseEffect Building base part of effect" << std::endl;
}

virtual ~BaseEffect() { }

void render(PrimitiveStats & stats, const RenderDescriptor & desc) const
{
updateStatistics(stats);
std::cout << "In BaseEffect Render" << std::endl;
render(desc);
}

private:
virtual void render(const RenderDescriptor & desc) const = 0;
virtual void updateStatistics(PrimitiveStats & stats) const = 0;
};

class HeightEffect : public BaseEffect
{
public:
HeightEffect()
{
std::cout << "In HeightEffect Building Specific HeightEffect stuff" << endl;
}

private:
virtual void render(const RenderDescriptor & desc) const
{
int maxHeight = height(desc.bounding);
std::cout << "In HeightEffect Render" << endl;
std::cout << "Rendering with, MaxHeight:" << maxHeight << endl;
}

virtual void updateStatistics(PrimitiveStats & stats)
{
std::cout << "In HeightEffect updateStatistics" << std::endl;
std::cout << "...storing vertex/triangle/texture count etc" << std::endl;
stats.advanceTris(24);
stats.advanceTriStrip(4);
}
};

typedef boost::ptr_map<std::string, BaseEffect> EffectDepository;

RenderDescriptor makeRenderData(int i)
{
return RenderDescriptor(Box(0, 0, i, i));
}

int main()
{
EffectDepository effects;
effects.insert("height", new HeightEffect());

PrimitiveStats stats;

for( int i = 0; i < 5; ++i)
{
effects["height"].render(stats, makeRenderData(i));
}

std::cout << stats;
}


There are other improvements that can be made at a design level, such as having the render function actually return yet another structure containing the low-level rendering state (RenderChunks?). These low-level states blocks can be put into a renderqueue and sorted to minimise state changes before actually being renderered by a renderer. In this model the job of effects is simply to transform high-level state from high-level entities into low-level rendering state for the graphics pipeline.

I've used strings as the key type in the effect map. You might want to hash strings or use some other kind of identifier for efficiency.

[Edited by - dmatter on August 21, 2008 3:55:11 PM]

Share this post


Link to post
Share on other sites
wforl    169
dmatter, first of all, a big whopping thankyou for the time you have taken to look at, and modify my code, it is very much appreciated.

I've had a quick skim through, but haven't really read it properly, as i just wanted to say than you in advance.

I will now look over it, and edit this post with my queries. It will take me some time though, as i have never used any of the boost libraries, so i will need to look up what you have used.

Thanks again dmatter :)

Share this post


Link to post
Share on other sites
dmatter    4844
Quote:
Original post by wforl
i have never used any of the boost libraries, so i will need to look up what you have used.
It's something I've been warming up to slowly and hoping to embrace fully soon enough.

boost::any works a like a union, or in some ways a void pointer, in that it can be assigned any value of any type. It gives C++ the chance to be typeless which also means it should only be used responsibly. [smile]

boost::ptr_map works similarly to a normal std::map of smart pointers (like boost::shared_ptr). Internally it'll store pointers to your objects but the container will automate their deletion for you at appropriate times. It also means the interface is nicer, you don't have to dereference the pointers yourself (using * or ->) like you would with a regular std::map of pointers because the boost::ptr_map does it for you and just hands you a reference to use.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this