Jump to content

  • Log In with Google      Sign In   
  • Create Account

#ActualSandman

Posted 07 April 2013 - 05:21 PM

Juliean, on 07 Apr 2013 - 22:42, said:snapback.png

 


As in, completely pointless - or just pointless regarding this topic?

 
Definitely the latter, and most probably the former as well.

 

 

Juliean, on 07 Apr 2013 - 22:42, said:snapback.png

 


Note that I, due to performance reasons, did a "SetActiveTexture"-function instead of passing the locator every time.

 

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.

 

 

Juliean, on 07 Apr 2013 - 22:42, said:snapback.png

 


 

Texture::Texture(unsigned int hash, TextureActionHandler* pActionHandler): m_pActionHandler(pActionHandler), m_locator(hash) //todo: important, change!
{
}

I'd imagined something more like this for the Texture constructor

 

 

Texture::Texture(std::string textureName, TextureActionHandler& rHandler) :
    m_Locator(rHandler.CreateLocator(textureName)),
    m_pActionHandler(&rHandler)
{
}

 
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. 
 
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. 
 

Juliean, on 07 Apr 2013 - 22:42, said:snapback.png

 


 

typedef std::vector TextureVector;
typedef std::map LocatorMap;

 

 

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.

: Ugh! formatting fubar'd. I give up


#5Sandman

Posted 07 April 2013 - 05:17 PM

As in, completely pointless - or just pointless regarding this topic?

 

 
Definitely the latter, and most probably the former as well.

 

 

Note that I, due to performance reasons, did a "SetActiveTexture"-function instead of passing the locator every time.

 

 

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.

 

 

Texture::Texture(unsigned int hash, TextureActionHandler* pActionHandler): m_pActionHandler(pActionHandler), m_locator(hash) //todo: important, change!
{
}

: fixed formatting


#4Sandman

Posted 07 April 2013 - 05:17 PM

As in, completely pointless - or just pointless regarding this topic?

 

 
Definitely the latter, and most probably the former as well.

 

 

Note that I, due to performance reasons, did a "SetActiveTexture"-function instead of passing the locator every time.

 

 

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.

 

 

Texture::Texture(unsigned int hash, TextureActionHandler* pActionHandler): m_pActionHandler(pActionHandler), m_locator(hash) //todo: important, change!
{
}

 

I'd imagined something more like this for the Texture constructor

 

 

Texture::Texture(std::string textureName, TextureActionHandler& rHandler) :
    m_Locator(rHandler.CreateLocator(textureName)),
    m_pActionHandler(&rHandler)
{
}

 
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. 
 
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. 
 

typedef std::vector TextureVector;
typedef std::map LocatorMap;

 

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.

: fixed formatting


#3Sandman

Posted 07 April 2013 - 05:16 PM

As in, completely pointless - or just pointless regarding this topic?

 

 
Definitely the latter, and most probably the former as well.

 

 

Note that I, due to performance reasons, did a "SetActiveTexture"-function instead of passing the locator every time.

 

 

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.

 

 

Texture::Texture(unsigned int hash, TextureActionHandler* pActionHandler): m_pActionHandler(pActionHandler), m_locator(hash) //todo: important, change!
{
}

 

I'd imagined something more like this for the Texture constructor

 

Texture::Texture(std::string textureName, TextureActionHandler& rHandler) :
    m_Locator(rHandler.CreateLocator(textureName)),
    m_pActionHandler(&rHandler)
{
}

 
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. 
 
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. 
 

[code=auto:0]
typedef std::vector TextureVector;
typedef std::map LocatorMap;

 

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.

: fixed formatting


#2Sandman

Posted 07 April 2013 - 05:15 PM

As in, completely pointless - or just pointless regarding this topic?

 

 
Definitely the latter, and most probably the former as well.

 

 

Note that I, due to performance reasons, did a "SetActiveTexture"-function instead of passing the locator every time.

 

 

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.

 

 


Texture::Texture(unsigned int hash, TextureActionHandler* pActionHandler): m_pActionHandler(pActionHandler), m_locator(hash) //todo: important, change!
{
}

 

I'd imagined something more like this for the Texture constructor

 

Texture::Texture(std::string textureName, TextureActionHandler& rHandler) :
    m_Locator(rHandler.CreateLocator(textureName)),
    m_pActionHandler(&rHandler)
{
}
[/code]
 
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. 
 
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. 
 
[quote name='Juliean' timestamp='1365370981' post='5050981']
[code]
typedef std::vector TextureVector;
typedef std::map LocatorMap;

[/quote]

 

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.


#1Sandman

Posted 07 April 2013 - 05:15 PM

As in, completely pointless - or just pointless regarding this topic?

 
Definitely the latter, and most probably the former as well.

 

Note that I, due to performance reasons, did a "SetActiveTexture"-function instead of passing the locator every time.

 

 

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.

 

Texture::Texture(unsigned int hash, TextureActionHandler* pActionHandler): m_pActionHandler(pActionHandler), m_locator(hash) //todo: important, change!
{
}

 

I'd imagined something more like this for the Texture constructor

Texture::Texture(std::string textureName, TextureActionHandler& rHandler) :
    m_Locator(rHandler.CreateLocator(textureName)),
    m_pActionHandler(&rHandler)
{
}
[code]
 
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. 
 
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. 
 
[quote name='Juliean' timestamp='1365370981' post='5050981']
[code]
typedef std::vector TextureVector;
typedef std::map LocatorMap;

[/quote]

 

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.


PARTNERS