Jump to content

  • Log In with Google      Sign In   
  • Create Account


#ActualJuliean

Posted 07 April 2013 - 05:42 PM

Premature optimization is the root of all evil. In this case you've sacrificed thread safety. While the original design wasn't implicitly thread safe, it wouldn't be too hard to apply locks before accessing a specific texture. With this design though, you'd have to lock the whole texture module everytime you read or write anything, which is ugly as hell and massively increases the potential for lock contention.



Map lookups are pretty quick, and how often do you really need to play around with textures outside the api specific code anyway? We're probably not talking about a massive number of calls per frame here. I wouldn't worry too much about optimizing this until you find the performance to be problematic with profiling. Furthermore, you could cache certain non-mutable, non-api specific data, such as texture size, in the api agnostic texture proxy object.

 

Thread safety isn't something I'm currently aiming for, but still, thanks for the hint, thankfully its easy to activate/deactive this caching system on the fly...

 

 

Map lookups are pretty quick, and how often do you really need to play around with textures outside the api specific code anyway? We're probably not talking about a massive number of calls per frame here. I wouldn't worry too much about optimizing this until you find the performance to be problematic with profiling. Furthermore, you could cache certain non-mutable, non-api specific data, such as texture size, in the api agnostic texture proxy object.

 

Awwh, that makes me a little bit worried about my current use of this system. Right now I'd e.g. use one of my component systems to render like this:

 

class GridRenderer : public EntitySystem<GridRenderer>
{
public:
	GridRenderer(IGfx3D& gfx3D): m_pGfx3D(&gfx3D), 
		m_pEffect( m_pGfx3D->effects().GetEffect(L"Game/Effects/Line.fx") ), m_pGrid( m_pGfx3D->meshes().GetMesh(L"debug grid") ), m_pScene(m_pGfx3D->textures().GetTexture(L"final scene") ) {};

	void Update(EntityManager& entityManager, MessageManager& messageManager)
	{
		m_pScene->SetRenderTarget(0);

		D3DXMATRIX mWorldViewProj;
		D3DXMatrixIdentity(&mWorldViewProj);
		mWorldViewProj *= m_pGfx3D->GetCamera()->GetViewProjectionMatrix();

		m_pEffect->SetMatrix("cWorldViewProj", mWorldViewProj);
		m_pEffect->SetFloat3("cColor", 0.8f, 0.8f, 0.8f);

		m_pEffect->Begin();

		m_pGrid->Render();

		m_pEffect->End();

		m_pGfx3D->textures().UnbindRenderTarget(0);
	}

private:

	IGfx3D* m_pGfx3D;
	Effect* m_pEffect;
	Texture* m_pScene;
	IRenderable* m_pGrid;
};

Interpreting what you just said I assume I should handling rendering differently? I mean, my previous implementation looked like this:

class GridRenderer : public EntitySystem<GridRenderer>
{
public:
	GridRenderer(Gfx3D& gfx3D): m_pGfx3D(&gfx3D) {};

	void Update(EntityManager& entityManager, MessageManager& messageManager)
	{
		m_pGfx3D->textures().SetRenderTarget(0, L"final scene");

		Effect* pEffect = m_pGfx3D->GetEffect(L"Line.fx");

		D3DXMATRIX mWorldViewProj;
		D3DXMatrixIdentity(&mWorldViewProj);
		mWorldViewProj *= m_pGfx3D->GetCamera()->GetViewProjectionMatrix();

		pEffect->SetMatrix("cWorldViewProj", mWorldViewProj);
		pEffect->SetFloat3("cColor", 0.8f, 0.8f, 0.8f);

		pEffect->Begin();

		m_pGfx3D->meshes().RenderByName(L"debug grid");

		pEffect->End();

		m_pGfx3D->textures().UnbindRenderTarget(0);
	}

private:

	Gfx3D* m_pGfx3D;
};

I know I have to, and soon will, implement a render queue, but aside from that I assumed this is the way graphics displaying should be handled, basically. What do you suggest instead (if you do so, that is), because if you said that I wouldn't be accessing stuff outside the API-specifiy code anyway... that gives me quite something to think, you know.

 

 

The hash code for the locator will be generated by the actionhandler, probably by hashing off the texture name. This doesn't even require a lookup. It also makes sense that the actionhandler take this responsibility, so it can maintain a class invariant - that a texture's hash code actually points to the texture it's meant to point to.

 

Good point, I currently used the size of the TextureVector from my texture module as "hash", yeah I know, I currently don't even support deletion of loaded textures, so it isn't .that bad (well, except for that non-dynamic resource management :S). As for hashing the name, what would you recommend me? MD5? SHA1/2? Some sort of "home made" solution specially for this "problem"?

 

 

Also, it's a style thing, but passing the action handler as a reference rather than a pointer prevents you from passing NULL, saving you from a whole class of bugs and all the checking and error handling that goes with it. Make that compiler do the work for you.

 

Hah, I clearly missed that. As you can see, I actually do prefer to pass by reference where possible, too. Quess that one slipped me.

 

I don't really see the need of the locator map. You just need a map of hash/api specific texture, and as I mentioned above, generate the hash off the texture name.

 

Yeah, thats the thing. Since I used a linear only-so-named-because-it-sounds-cool "hash", I actually required that map to actually access my textures again after loading them safely. So you'd suggest to create a hash of the texture name on loading - and then, when I later access that file (loading and accessing is a seperate task now, no more lazy loading, the one time in the texturemodule was a slip), simply create a new Texture-object, like this?

 

Texture* TextureModule::GetTexture(const std::wstring& stName) const
{
return new Texture(stName, actionHandler);
}

Well you can call me on premature optimization there too, since I assumed that it was better not to create a new texture handle (can we call it that anyway?) every time I accessed a texture by file name? You could also call me on having currently a stupid messed up rendering system that at least for the entities doesn't store the textures but accesses them per frame per entites (no, not even material sorting currently, yiekes :/) so that would be quite a bit memory creation and deletion per frame. That is, if you are suggesting me to handle it that way as in the code, aren't you?


#2Juliean

Posted 07 April 2013 - 05:42 PM

Premature optimization is the root of all evil. In this case you've sacrificed thread safety. While the original design wasn't implicitly thread safe, it wouldn't be too hard to apply locks before accessing a specific texture. With this design though, you'd have to lock the whole texture module everytime you read or write anything, which is ugly as hell and massively increases the potential for lock contention.



Map lookups are pretty quick, and how often do you really need to play around with textures outside the api specific code anyway? We're probably not talking about a massive number of calls per frame here. I wouldn't worry too much about optimizing this until you find the performance to be problematic with profiling. Furthermore, you could cache certain non-mutable, non-api specific data, such as texture size, in the api agnostic texture proxy object.

 

Thread safety isn't something I'm currently aiming for, but still, thanks for the hint, thankfully its easy to activate/deactive this caching system on the fly...

 

 

Map lookups are pretty quick, and how often do you really need to play around with textures outside the api specific code anyway? We're probably not talking about a massive number of calls per frame here. I wouldn't worry too much about optimizing this until you find the performance to be problematic with profiling. Furthermore, you could cache certain non-mutable, non-api specific data, such as texture size, in the api agnostic texture proxy object.

 

Awwh, that makes me a little bit worried about my current use of this system. Right now I'd e.g. use one of my component systems to render like this:

 

class GridRenderer : public EntitySystem<GridRenderer>
{
public:
	GridRenderer(IGfx3D& gfx3D): m_pGfx3D(&gfx3D), 
		m_pEffect( m_pGfx3D->effects().GetEffect(L"Game/Effects/Line.fx") ), m_pGrid( m_pGfx3D->meshes().GetMesh(L"debug grid") ), m_pScene(m_pGfx3D->textures().GetTexture(L"final scene") ) {};

	void Update(EntityManager& entityManager, MessageManager& messageManager)
	{
		m_pScene->SetRenderTarget(0);

		D3DXMATRIX mWorldViewProj;
		D3DXMatrixIdentity(&mWorldViewProj);
		mWorldViewProj *= m_pGfx3D->GetCamera()->GetViewProjectionMatrix();

		m_pEffect->SetMatrix("cWorldViewProj", mWorldViewProj);
		m_pEffect->SetFloat3("cColor", 0.8f, 0.8f, 0.8f);

		m_pEffect->Begin();

		m_pGrid->Render();

		m_pEffect->End();

		m_pGfx3D->textures().UnbindRenderTarget(0);
	}

private:

	IGfx3D* m_pGfx3D;
	Effect* m_pEffect;
	Texture* m_pScene;
	IRenderable* m_pGrid;
};

Interpreting what you just said I assume I should handling rendering differently? I mean, my previous implementation looked like this:

class GridRenderer : public EntitySystem<GridRenderer>
{
public:
	GridRenderer(Gfx3D& gfx3D): m_pGfx3D(&gfx3D) {};

	void Update(EntityManager& entityManager, MessageManager& messageManager)
	{
		m_pGfx3D->textures().SetRenderTarget(0, L"final scene");

		Effect* pEffect = m_pGfx3D->GetEffect(L"Line.fx");

		D3DXMATRIX mWorldViewProj;
		D3DXMatrixIdentity(&mWorldViewProj);
		mWorldViewProj *= m_pGfx3D->GetCamera()->GetViewProjectionMatrix();

		pEffect->SetMatrix("cWorldViewProj", mWorldViewProj);
		pEffect->SetFloat3("cColor", 0.8f, 0.8f, 0.8f);

		pEffect->Begin();

		m_pGfx3D->meshes().RenderByName(L"debug grid");

		pEffect->End();

		m_pGfx3D->textures().UnbindRenderTarget(0);
	}

private:

	Gfx3D* m_pGfx3D;
};

I know I have to, and soon will, implement a render queue, but aside from that I assumed this is the way graphics displaying should be handled, basically. What do you suggest instead (if you do so, that is), because if you said that I wouldn't be accessing stuff outside the API-specifiy code anyway... that gives me quite something to think, you know.

 

 

The hash code for the locator will be generated by the actionhandler, probably by hashing off the texture name. This doesn't even require a lookup. It also makes sense that the actionhandler take this responsibility, so it can maintain a class invariant - that a texture's hash code actually points to the texture it's meant to point to.

 

Good point, I currently used the size of the TextureVector from my texture module as "hash", yeah I know, I currently don't even support deletion of loaded textures, so it isn't that bad. As for hashing the name, what would you recommend me? MD5? SHA1/2? Some sort of "home made" solution specially for this "problem"?

 

 

Also, it's a style thing, but passing the action handler as a reference rather than a pointer prevents you from passing NULL, saving you from a whole class of bugs and all the checking and error handling that goes with it. Make that compiler do the work for you.

 

Hah, I clearly missed that. As you can see, I actually do prefer to pass by reference where possible, too. Quess that one slipped me.

 

I don't really see the need of the locator map. You just need a map of hash/api specific texture, and as I mentioned above, generate the hash off the texture name.

 

Yeah, thats the thing. Since I used a linear only-so-named-because-it-sounds-cool "hash", I actually required that map to actually access my textures again after loading them safely. So you'd suggest to create a hash of the texture name on loading - and then, when I later access that file (loading and accessing is a seperate task now, no more lazy loading, the one time in the texturemodule was a slip), simply create a new Texture-object, like this?

 

Texture* TextureModule::GetTexture(const std::wstring& stName) const
{
return new Texture(stName, actionHandler);
}

Well you can call me on premature optimization there too, since I assumed that it was better not to create a new texture handle (can we call it that anyway?) every time I accessed a texture by file name? You could also call me on having currently a stupid messed up rendering system that at least for the entities doesn't store the textures but accesses them per frame per entites (no, not even material sorting currently, yiekes :/) so that would be quite a bit memory creation and deletion per frame. That is, if you are suggesting me to handle it that way as in the code, aren't you?


#1Juliean

Posted 07 April 2013 - 05:40 PM

Premature optimization is the root of all evil. In this case you've sacrificed thread safety. While the original design wasn't implicitly thread safe, it wouldn't be too hard to apply locks before accessing a specific texture. With this design though, you'd have to lock the whole texture module everytime you read or write anything, which is ugly as hell and massively increases the potential for lock contention.



Map lookups are pretty quick, and how often do you really need to play around with textures outside the api specific code anyway? We're probably not talking about a massive number of calls per frame here. I wouldn't worry too much about optimizing this until you find the performance to be problematic with profiling. Furthermore, you could cache certain non-mutable, non-api specific data, such as texture size, in the api agnostic texture proxy object.

 

Thread safety isn't something I'm currently aiming for, but still, thanks for the hint, thankfully its easy to activate/deactive this caching system on the fly...

 

Map lookups are pretty quick, and how often do you really need to play around with textures outside the api specific code anyway? We're probably not talking about a massive number of calls per frame here. I wouldn't worry too much about optimizing this until you find the performance to be problematic with profiling. Furthermore, you could cache certain non-mutable, non-api specific data, such as texture size, in the api agnostic texture proxy object.

 

Awwh, that makes me a little bit worried about my current use of this system. Right now I'd e.g. use one of my component systems to render like this:

 

class GridRenderer : public EntitySystem<GridRenderer>
{
public:
	GridRenderer(IGfx3D& gfx3D): m_pGfx3D(&gfx3D), 
		m_pEffect( m_pGfx3D->effects().GetEffect(L"Game/Effects/Line.fx") ), m_pGrid( m_pGfx3D->meshes().GetMesh(L"debug grid") ), m_pScene(m_pGfx3D->textures().GetTexture(L"final scene") ) {};

	void Update(EntityManager& entityManager, MessageManager& messageManager)
	{
		m_pScene->SetRenderTarget(0);

		D3DXMATRIX mWorldViewProj;
		D3DXMatrixIdentity(&mWorldViewProj);
		mWorldViewProj *= m_pGfx3D->GetCamera()->GetViewProjectionMatrix();

		m_pEffect->SetMatrix("cWorldViewProj", mWorldViewProj);
		m_pEffect->SetFloat3("cColor", 0.8f, 0.8f, 0.8f);

		m_pEffect->Begin();

		m_pGrid->Render();

		m_pEffect->End();

		m_pGfx3D->textures().UnbindRenderTarget(0);
	}

private:

	IGfx3D* m_pGfx3D;
	Effect* m_pEffect;
	Texture* m_pScene;
	IRenderable* m_pGrid;
};

Interpreting what you just said I assume I should handling rendering differently? I mean, my previous implementation looked like this:

class GridRenderer : public EntitySystem<GridRenderer>
{
public:
	GridRenderer(Gfx3D& gfx3D): m_pGfx3D(&gfx3D) {};

	void Update(EntityManager& entityManager, MessageManager& messageManager)
	{
		m_pGfx3D->textures().SetRenderTarget(0, L"final scene");

		Effect* pEffect = m_pGfx3D->GetEffect(L"Line.fx");

		D3DXMATRIX mWorldViewProj;
		D3DXMatrixIdentity(&mWorldViewProj);
		mWorldViewProj *= m_pGfx3D->GetCamera()->GetViewProjectionMatrix();

		pEffect->SetMatrix("cWorldViewProj", mWorldViewProj);
		pEffect->SetFloat3("cColor", 0.8f, 0.8f, 0.8f);

		pEffect->Begin();

		m_pGfx3D->meshes().RenderByName(L"debug grid");

		pEffect->End();

		m_pGfx3D->textures().UnbindRenderTarget(0);
	}

private:

	Gfx3D* m_pGfx3D;
};

I know I have to, and soon will, implement a render queue, but aside from that I assumed this is the way graphics displaying should be handled, basically. What do you suggest instead (if you do so, that is).

 

The hash code for the locator will be generated by the actionhandler, probably by hashing off the texture name. This doesn't even require a lookup. It also makes sense that the actionhandler take this responsibility, so it can maintain a class invariant - that a texture's hash code actually points to the texture it's meant to point to.

 

Good point, I currently used the size of the TextureVector from my texture module as "hash", yeah I know, I currently don't even support deletion of loaded textures, so it isn't that bad. As for hashing the name, what would you recommend me? MD5? SHA1/2? Some sort of "home made" solution specially for this "problem"?

 

Also, it's a style thing, but passing the action handler as a reference rather than a pointer prevents you from passing NULL, saving you from a whole class of bugs and all the checking and error handling that goes with it. Make that compiler do the work for you.

 

Hah, I clearly missed that. As you can see, I actually do prefer to pass by reference where possible, too. Quess that one slipped me.

I don't really see the need of the locator map. You just need a map of hash/api specific texture, and as I mentioned above, generate the hash off the texture name.

 

Yeah, thats the thing. Since I used a linear only-so-named-because-it-sounds-cool "hash", I actually required that map to actually access my textures again after loading them safely. So you'd suggest to create a hash of the texture name on loading - and then, when I later access that file (loading and accessing is a seperate task now, no more lazy loading, the one time in the texturemodule was a slip), simply create a new Texture-object, like this?

 

Texture* TextureModule::GetTexture(const std::wstring& stName) const
{
return new Texture(stName, actionHandler);
}

Well you can call me on premature optimization there too, since I assumed that it was better not to create a new texture handle (can we call it that anyway?) every time I accessed a texture by file name? You could also call me on having currently a stupid messed up rendering system that at least for the entities doesn't store the textures but accesses them per frame per entites (no, not even material sorting currently, yiekes :/) so that would be quite a bit memory creation and deletion per frame. That is, if you are suggesting me to handle it that way as in the code, aren't you?


PARTNERS