Skip to content

Commit

Permalink
Fix small regression from merge
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Carroll <[email protected]>
  • Loading branch information
mjcarroll committed Jun 20, 2021
1 parent 5d1bbb6 commit 24a67ad
Showing 1 changed file with 10 additions and 6 deletions.
16 changes: 10 additions & 6 deletions ogre2/src/Ogre2Material.cc
Original file line number Diff line number Diff line change
Expand Up @@ -643,19 +643,23 @@ void Ogre2Material::SetTextureMapImpl(const std::string &_texture,

this->ogreDatablock->setTexture(_type, baseName, &samplerBlockRef);

this->dataPtr->hashName =
this->ogreDatablock->getTexture(0)->getName().getFriendlyText();
Ogre::Root *root = Ogre2RenderEngine::Instance()->OgreRoot();
Ogre::TextureGpuManager *textureMgr =
root->getRenderSystem()->getTextureGpuManager();
auto tex = textureMgr->findTextureNoThrow(baseName);

if (tex)
{
tex->waitForMetadata();
this->dataPtr->hashName = tex->getName().getFriendlyText();
}

This comment has been minimized.

Copy link
@darksylinc

darksylinc Jul 3, 2021

Contributor

Remarks:

  1. The texture name is not considered part of the metadata (because it's not part of the file, it's provided by the app), hence there's no need to call to call waitForMetadata
  2. If waitForMetadata call happens right after it started loading, it can strongly hamper our ability to hide loading times via background loading.
  3. What is hashName used for? I can't see the variable used anywhere in the codebase. Also please note that getFriendlyText is for purposes of displaying the name for UIs or logging; but it should not be used to drive logic because the text may get clipped; e.g. "Very long text 012" and "Very long text 789" may end up as "Very long text" with different hashes. If you need the full name (no clipping) you can use tex->getNameStr() but beware it can return a nullptr (that should be extremely rare and possibly a sign that something went terribly wrong)

This comment has been minimized.

Copy link
@ahcorde

ahcorde Jul 5, 2021

Contributor

we can remove hashName, I believe it's not being used

This comment has been minimized.

Copy link
@iche033

iche033 Jul 6, 2021

Contributor

+1, I think we can remove hashName and the logic here


// disable alpha from texture if texture does not have an alpha channel
// otherwise this becomes a transparent material
if (_type == Ogre::PBSM_DIFFUSE)
{
if (this->TextureAlphaEnabled())
{
Ogre::Root *root = Ogre2RenderEngine::Instance()->OgreRoot();
Ogre::TextureGpuManager *textureMgr =
root->getRenderSystem()->getTextureGpuManager();
auto tex = textureMgr->findTextureNoThrow(baseName);
if (tex)
{
tex->scheduleTransitionTo(Ogre::GpuResidency::Resident);
Expand Down

0 comments on commit 24a67ad

Please sign in to comment.