RC1 Bug - DDS textures are not cached

#1 We had to make a few changes to get DDS to be properly handled on Interiors, but otherwise, the issue with not being cached does exist, as there isn't proper DDS support (as noted by the comment in the function).

GFXTextureObject *GFXTextureManager::createTexture( const char *filename, GFXTextureProfile *profile )
{
   // Check the cache first...
   ResourceObject * ro = GBitmap::findBmpResource(filename);
   if (ro)
   {
      StringTableEntry fileName = ro->getFullPath();
      GFXTextureObject * cacheHit = hashFind(fileName);
      if (cacheHit)
         return cacheHit;

	  // DDS hack.. originally GG's, but moved here to support it a bit easier
	  // still crappy though.. - byte
	  if( dStrstr( fileName, ".dds" ) )
	  {
		  GFXTextureObject *obj = _loadDDSHack( fileName, profile );
		  if( !obj )
			  return NULL;

		  linkTexture( obj );

		  // Return the new texture!
		  return obj;
	  }
   }

   // Find and load the texture.
   GBitmap *bmp = GBitmap::load(filename);

   if(!bmp)
   {
      Con::warnf("GFXTextureManager::createTexture - Unable to load Texture: %s", filename);
      return NULL;
   }

   return createTexture(bmp, profile, true);
}

You can see I made a modification as to how it was looking for the texture and it fixed interiors using dds, as they don't normally pass an extension, thus .dds loading wouldn't happen properly.

As for the loadDDSHack function and being a security issue, I'm not too sure if its of a major concern. As the filename is passed off to DirectX functions for handling. The issue would only come from referencing something elsewhere, via some rogue functionality, but a large part of that would be internally handled

#2 Yeah, DDS really isn't supported well, we definitely do need this to work properly. You interested in contracting on that Tom? I think we might want some tools related to DDS as well, although nVIDIA might have something that works now.

#3 Haha... maybe after GDC and we expand the team. ;)

Still is Jeremiah's change a good temporary fix? Can we at least get that in the final release?

#4 Just tested Jeremiah's code, doesn't work. For some reason cacheHit returns NULL on the second instance of the same .dds file. I don't have time to track down the problem. Have either of you tested this? Is there something missing Jeremiah?

#5 Let me check my codebase, as i know it works, but I'm not sure if it works as intended.

It was more a way to properly get DDS support for Interiors, as their filenames don't have extensions when loaded in, so it would end up finding the DDS later in the code and then try doing a load on it...

#6 The one thing I forgot was we added dds to the array of extensions in gBitmap, otherwise findBmpResource wouldn't properly locate handle .dds files.

gfx\gBitmap.cpp:

#define EXT_ARRAY_SIZE 6
static const char* extArray[EXT_ARRAY_SIZE] = { "", ".jpg", ".png", ".gif", ".bmp", ".dds" };

Not sure if that alone will keep it from returning NULL, but it will allow it to properly search for the filename (properly)

#7 I discovered this to my horror when loading a massive DDS file that was used everywhere. Here's a quick'n'dirty fix:

Comment this out:

GFXTextureObject *obj = _loadDDSHack( filename, profile );
      if( !obj ) return NULL;

      linkTexture( obj );

And use this instead:

GFXTextureObject *temp = mListHead;	  
	  while( temp != NULL ) 
	  {
		  if (temp->mTextureFileName && (dStrcmp(temp->mTextureFileName, filename) == 0))
			  return temp;

		  temp = temp->mNext;
	  }

	  //If the file doesn't exist, return NULL
	  if (!Platform::isFile(filename))
		  return NULL;

	  //Create and load a texture
	  DDSFile *dds = new DDSFile();
	  dds->read(filename);
	  GFXTextureObject *obj = createTexture(dds, profile, true);
	  obj->mTextureFileName = StringTable->insert(filename);

This hasn't been throughly tested, though.

#8 Ok i got this working and it seems to be solid. It combines Jeremiah's code with one line from Manoel's:

In gfx/gfxTextureManager.cpp:

GFXTextureObject *GFXTextureManager::createTexture( const char *filename, GFXTextureProfile *profile )
{
   // Check the cache first...
   ResourceObject * ro = GBitmap::findBmpResource(filename);
   if (ro)
   {
      StringTableEntry fileName = ro->getFullPath();
      GFXTextureObject * cacheHit = hashFind(fileName);
      if (cacheHit)
         return cacheHit;
    
      // DDS hack.. originally GG's, but moved here to support it a bit easier
      // still crappy though.. - byte
      if( dStrstr( fileName, ".dds" ) )
      {
         GFXTextureObject *obj = _loadDDSHack( fileName, profile );
         if( !obj )
            return NULL;
 
         obj->mTextureFileName = fileName; // Needs a name!
         linkTexture( obj );
  
         // Return the new texture!
         return obj;
      }
   }
   
   // Find and load the texture.
   GBitmap *bmp = GBitmap::load(filename);
   
   if(!bmp)
   {
      Con::warnf("GFXTextureManager::createTexture - Unable to load Texture: %s", filename);
      return NULL;
   }
   
   return createTexture(bmp, profile, true);
}

and in gfx\gBitmap.cpp:

#define EXT_ARRAY_SIZE 6
static const char* extArray[EXT_ARRAY_SIZE] = { "", ".jpg", ".png", ".gif", ".bmp", ".dds" };

That's it... it works pretty good from what i can tell.

#9 Awesome, I'll merge in the changes and see how it works, which we're not using dds right now so :p

#10 Mmm... I just wrote a relief map shader based on the one posted around here somewhere and was wondering why I was taking such a performance hit with only one relief map in the scene.

Once I get back to the lab I'll run it through it's paces.

#11 While this does technically work, it creates a bunch of console warnings about failure to load misc textures (although they seem to actually be loading). It also raises the following warning:

ResourceObject::construct: NULL resource create function for 'solidmetal.dds'.

for every dds file.

If I had more time, I'd dig into this.

#12 Hmm surprised it is making it to the ResourceObject::construct error, as that shouldn't be hit in that function... but I guess it could be happening in the other functions.

a lack of a create function for DDS is actually the larger issue, as with a create function all of the special checking for DDS wouldn't be needed... as it would be handled in the creation function like JPG, PNG, etc

#13 @Brian - I'm not getting any error messages other than for textures that i know don't exist. We have maybe 24 DDS files in our project and it seems to be working great.

#14 I'm going to mark this as a bug right now and I'll look into it later when I have more time. There's prob some small thing that's missing that you guys just already implemented.

#15 I get the same texture errors as brian. They do seem to be loading fine, but it still spits errors to the console for each one.

#16 Is this in the current version of TGEA?

#17 No as this is a user hack to expand GGs DDS implementation hack.

#18 OK, I figured out what the console errors were. The generated .dds needs to have a lower class file extension. Ie. many tools create them as .DDS by default, but Torque doesn't like it if it's not .dds.

#19 That would explain it and make sense, as I verified all of our extensions were lowercase, but just in case, I was making sure it was making everything lowercase when checking.