Skip to main content
gamedev.net
🔒 Locked

Pattern alternative to singleton?

Started by Denzin Jun 22, 2010 at 7:14 AM 32 replies 5.9k views
Original Post
Denzin
Denzin
I'm currently working on a project with a small team (~5 programmers).
One of my team members insists we use a singleton for our renderer to avoid passing the d3d device pointer in every object's render function.
I implemented a quick mock up of the singleton, however I am not happy with it.

game render function
void CShell::Render(){	//start the render process	if(CDirectXRenderer::GetInstance()->BeginScene())	{		terrain.Render();		CDirectXRenderer::GetInstance()->EndScene();	}}


terrain render function
void CD3DTerrain::Render(){	//set buffers as source	CDirectXRenderer::GetInstance()->m_pD3DDevice->SetStreamSource(0,m_pVertexBuffer, 0, sizeof(sCUSTOMVERTEX));	CDirectXRenderer::GetInstance()->m_pD3DDevice->SetFVF(D3DFVF_CUSTOMVERTEX);	CDirectXRenderer::GetInstance()->m_pD3DDevice->SetIndices(m_pIndexBuffer);	//draw terrain	D3DXMATRIX	worldMat;	D3DXMatrixIdentity(&worldMat);	CDirectXRenderer::GetInstance()->m_pD3DDevice->SetTransform(D3DTS_WORLD, &worldMat);	CDirectXRenderer::GetInstance()->m_pD3DDevice->DrawIndexedPrimitive(D3DPT_TRIANGLELIST, 0, 0,numVertsX * numVertsZ, 0, cellsWide*cellsHigh*2);}


It works but seems messy to have the renderer so out in the open. Also note most of the rendering code will change once I implement a scene graph system.
Any advice on a clean (not having to pass pointers every line) but safer system than having the rendering object global?
Hodgman
Hodgman
What's wrong with passing it as an argument? IMO this looks much cleaner:
void CShell::Render(CDirectXRenderer& renderer){	//start the render process	if(renderer.BeginScene())	{		terrain.Render(renderer);		renderer.EndScene();	}}void CD3DTerrain::Render(CDirectXRenderer& renderer){	//set buffers as source	renderer.D3DDevice().SetStreamSource(0,m_pVertexBuffer, 0, sizeof(sCUSTOMVERTEX));	renderer.D3DDevice().SetFVF(D3DFVF_CUSTOMVERTEX);	renderer.D3DDevice().SetIndices(m_pIndexBuffer);	//draw terrain	D3DXMATRIX	worldMat;	D3DXMatrixIdentity(&worldMat);	renderer.D3DDevice().SetTransform(D3DTS_WORLD, &worldMat);	renderer.D3DDevice().DrawIndexedPrimitive(D3DPT_TRIANGLELIST, 0, 0,numVertsX * numVertsZ, 0, cellsWide*cellsHigh*2);}
Denzin
Denzin
for one object it does look cleaner just to do
terrain.Render(renderer->GetDevicePtr());


however, the large amount of objects calling render muck it up with parameters(my teams opinion, not my own)

Hodgman
Hodgman
Quote:
the objects calling render muck it up with parameters
Can you explain what the problem is exactly? People pass in the wrong parameters? It gets messy with lots of parameters?
Madhed
Madhed
IMO you should take the render function out of your game objects.
The code you have provided clearly shows that the render function needs way to much information about the internals of the renderer class.

Instead game objects should register more low-level objects (meshes, etc.) with the renderer which has the functionality to draw them.

This way you can decouple the game objects from the actual renderering process and decrease dependancy.
SimonForsman
SimonForsman
There shouldn't be a reason for a singleton, either redesign your system so the dependancy is less of an issue or use a global instance. (Singletons are just as bad as globals but take more effort to implement and has a bunch of unnecessary restrictions)
[size="1"]I don't suffer from insanity, I'm enjoying every minute of it.
The voices in my head may not be real, but they have some good ideas!
Denzin
Denzin
terrain may not be the best example since it is specifically declared as d3dterrain. It depends heavily on d3d objects for rendering and generation.
Most of the dependancies in its render, mainly
CDirectXRenderer::GetInstance()->m_pD3DDevice->SetStreamSource(0,m_pVertexBuffer, 0, sizeof(sCUSTOMVERTEX));	CDirectXRenderer::GetInstance()->m_pD3DDevice->SetFVF(D3DFVF_CUSTOMVERTEX);	CDirectXRenderer::GetInstance()->m_pD3DDevice->SetIndices(m_pIndexBuffer);


will be handled in the scene node's pre render which also handles the transformations needed, then reset back to its original status in the post render.

In theory once the scene graph is running, the terrain will only need

CDirectXRenderer::GetInstance()->m_pD3DDevice->DrawIndexedPrimitive(D3DPT_TRIANGLELIST, 0, 0,numVertsX * numVertsZ, 0, cellsWide*cellsHigh*2);

in its render function.

Madhed
Madhed
What about something like this? (Pseudocode)

class Mesh {  VertexBuffer vb;  IndexBuffer ib;  Material mat;}class Terrain : GameObject {  Mesh terrainMesh;  Matrix transform;  Render(Renderer renderer) {    renderer->registerMesh(terrainMesh, transform);  }  }class D3DRenderer : Renderer {  list<Mesh,Matrix> drawOperations;  RenderScene() {    foreach(drawOperations as op) {      d3ddevice->SetPixelShader(op.mesh.mat.ps);      d3ddevice->SetVertexShader(op.mesh.mat.vs);      d3ddevice->SetTexture(op.mesh.mat.texture);      d3ddevice->SetIndexBuffer(op.mesh.ib);      d3ddevice->SetVertexBuffer(op.mesh.vb);      ...      ...      d3ddevice->DrawIndexedPrimitive(...);    }    drawOperations.clear();  }  registerMesh(Mesh, Matrix) {    drawOperations.add(Mesh, Matrix);  }}main() {  list<GameObject> objects;  Renderer renderer;  while(runGame) {    foreach(objects as object) {      object.render(renderer);    }    renderer.drawScene();  }}


This would remove the need to call d3d specific function from your game objects.
Cheers
sheep19
sheep19
I would do this:

class Object{Renderer *const renderer;public:Object(Renderer *const _renderer = nullptr) : renderer(_renderer){if( !renderer ) throw anExpection;  // or leave it like that, but the object won't be "renderable"};void Render(){//renderer->...}


This way the object owns the pointer of the renderer, but it cannot change it(as it's a constant pointer).
My Portfolio
Denzin
Denzin
thanks for the tips, I'll definitely run these by the rest of my team and hopefully we can reach a consensus.
jpetrie
jpetrie
Quote:

One of my team members insists we use a singleton for our renderer to avoid passing the d3d device pointer in every object's render function.

Madhed already hinted at this, but the problem is here is that your objects are designed in such a way as to require access to the singleton. Passing the renderer to objects that need it is the preferred approach (it isolates dependencies and makes them explicit, and avoids the use of a global access point, which can be promiscuously accessed).

Your team is probably correct that it would be "ugly" to pass the renderer around that much, but the solution isn't a singleton (or global), the solution is a better API design that doesn't involve nearly so many types depending directly on the renderer.

A singleton is almost always the wrong choice; a global is at least a little better because it doesn't require the extra boilerplate code (which is admittedly usually trivial, but non-zero). A better design for the dependent systems is usually the most ideal option.

For rendering systems, a good step towards such a design is to take the ability for objects to render themselves away. This is good in general, because it creates less (ideally one) code paths that could be traversed for rendering, and thus improves maintainability. Instead give the responsibility to a dedicated render object, and allow a higher level system to translate appropriately between properties of the game logic objects and the render objects prior to rendering or enqueueing the objects for render.
Hodgman
Hodgman
Quote:
Original post by sheep19
This way the object owns the pointer of the renderer, but it cannot change it(as it's a constant pointer).
Instead of checking for NULL and throwing exceptions, you can just ensure at compile-time that it won't be NULL by using a reference:
class Object{  Renderer& renderer;public:  Object(Renderer& _renderer) : renderer(_renderer){}};
Burnhard
Burnhard
The question I haven't seen answered here is whether it is better to pass objects to the renderer, or to pass the renderer to objects? I would prefer the former, assuming the model is broadly unified. Implementing an abstract interface, "IRenderable" on all renderable objects would allow this.
owl
owl
Quote:
Original post by Madhed
IMO you should take the render function out of your game objects.
The code you have provided clearly shows that the render function needs way to much information about the internals of the renderer class.

Instead game objects should register more low-level objects (meshes, etc.) with the renderer which has the functionality to draw them.

This way you can decouple the game objects from the actual renderering process and decrease dependancy.


Quoted in agreement.
[size="2"]I like the Walrus best.
sheep19
sheep19
Quote:
Original post by Hodgman
Quote:
Original post by sheep19
This way the object owns the pointer of the renderer, but it cannot change it(as it's a constant pointer).
Instead of checking for NULL and throwing exceptions, you can just ensure at compile-time that it won't be NULL by using a reference:*** Source Snippet Removed ***


goo point :)
My Portfolio
sheep19
sheep19
Quote:
Original post by HodgmanInstead of checking for NULL and throwing exceptions, you can just ensure at compile-time that it won't be NULL by using a reference[...]


Hmm I have a question here, let's consider this:

class Object{Renderer &rendererpublic:Object(Renderer &_renderer) : renderer(_renderer) {}};


int main(){Renderer *myRenderer = nullptr;Object obj(*myRenderer); // what happen's here? nullptr is passed!!obj->doSomething();}


Would that work?
My Portfolio
incin
incin
Quote:
Would that work?


Yes it will compile. References are just pointers with some compile time restrictions, and different syntax. But the compiler can never ensure you don't make a reference to a bad pointer.
jpetrie
jpetrie
Quote:

Would that work?

Won't even compile unless you're using C++0x (nullptr isn't a C++ keyword prior to 0x), and if you're using C++/CLI then it's the incorrect keyword for the context... and C++/CLI.

But I assume you meant a 'a null pointer constant' instead (i.e., 0 instead of nullptr).

In which case, dereferencing null is undefined behavior, so the behavior of that program is undefined. C++ references can never be null in a well-defined C++ program.

Quote:

References are just pointers with some compile time restrictions, and different syntax

This isn't neccessarily true; they behave similar, but the standard doesn't require that they be implemented with pointers and indeed, in some cases, they are not.

[Edited by - jpetrie on June 23, 2010 11:16:35 AM]
RabblePants
RabblePants
Quote:
Original post by jpetrie
Instead give the responsibility to a dedicated render object, and allow a higher level system to translate appropriately between properties of the game logic objects and the render objects prior to rendering or enqueueing the objects for render.


While not relevant to the discussion, one thing I want to mention is that taking the jpetrie's approach from the start also makes it much easier to implement a multi-threaded engine, which is gaining more and more popularity nowadays with multi-core systems. Having objects render themselves makes it -tremendously- difficult to split the render and game logic unless you place mutex/semaphore guards all over the place.
Denzin
Denzin
would any of you have some resources on a method you recommend?

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.