Ogre high performance thread support - request for comments

bysparkprime » Fri Mar 28, 2008 2:50 am

There isn't much to do in the Texture class itself, but I did add some things to the Texture manager, as I did with the Mesh and Material managers.

Code: Select all

diff -ruN ../cvs_fresh/OgreMain/include/OgreTextureManager.h ./OgreMain/include/OgreTextureManager.h--- ../cvs_fresh/OgreMain/include/OgreTextureManager.h   2008-02-11 11:10:38.000000000 +0000+++ ./OgreMain/include/OgreTextureManager.h   2008-03-27 22:08:43.202498807 +0000@@ -59,6 +59,71 @@         TextureManager(void);         virtual ~TextureManager(); +        /** Create a new texture, or retrieve an existing one with the same+            name if it already exists.+            @param+                texType The type of texture to load/create, defaults to normal 2D textures+            @param+                numMipmaps The number of pre-filtered mipmaps to generate. If left to MIP_DEFAULT then+                the TextureManager's default number of mipmaps will be used (see setDefaultNumMipmaps())+                If set to MIP_UNLIMITED mipmaps will be generated until the lowest possible+                level, 1x1x1.+            @param+                gamma The gamma adjustment factor to apply to this texture (brightening/darkening)+            @param +                isAlpha Only applicable to greyscale images. If true, specifies that+                the image should be loaded into an alpha texture rather than a+                single channel colour texture - useful for fixed-function systems.+            @param +                desiredFormat The format you would like to have used instead of+                the format being based on the contents of the texture+         @param hwGammaCorrection Pass 'true' to enable hardware gamma correction+            (sRGB) on this texture. The hardware will convert from gamma space+            to linear space when reading from this texture. Only applicable for +            8-bits per channel textures, will be ignored for other types. Has the advantage+            over pre-applied gamma that the texture precision is maintained.+            @see ResourceManager::createOrRetrieve+        */+        virtual ResourceCreateOrRetrieveResult createOrRetrieve(+            const String &name, const String& group, bool isManual = false,+            ManualResourceLoader* loader = 0, const NameValuePairList* createParams = 0,+            TextureType texType = TEX_TYPE_2D, int numMipmaps = MIP_DEFAULT, +            Real gamma = 1.0f, bool isAlpha = false,+            PixelFormat desiredFormat = PF_UNKNOWN, bool hwGammaCorrection = false);++        /** Prepares to loads a texture from a file.+            @param+                name The file to load, or a String identifier in some cases+            @param+                group The name of the resource group to assign the texture to+            @param+                texType The type of texture to load/create, defaults to normal 2D textures+            @param+                numMipmaps The number of pre-filtered mipmaps to generate. If left to MIP_DEFAULT then+                the TextureManager's default number of mipmaps will be used (see setDefaultNumMipmaps())+                If set to MIP_UNLIMITED mipmaps will be generated until the lowest possible+                level, 1x1x1.+            @param+                gamma The gamma adjustment factor to apply to this texture (brightening/darkening)+            @param +                isAlpha Only applicable to greyscale images. If true, specifies that+                the image should be loaded into an alpha texture rather than a+                single channel colour texture - useful for fixed-function systems.+            @param +                desiredFormat The format you would like to have used instead of+                the format being based on the contents of the texture+         @param hwGammaCorrection Pass 'true' to enable hardware gamma correction+            (sRGB) on this texture. The hardware will convert from gamma space+            to linear space when reading from this texture. Only applicable for +            8-bits per channel textures, will be ignored for other types. Has the advantage+            over pre-applied gamma that the texture precision is maintained.+        */+        virtual TexturePtr prepare( +            const String& name, const String& group, +            TextureType texType = TEX_TYPE_2D, int numMipmaps = MIP_DEFAULT, +            Real gamma = 1.0f, bool isAlpha = false,+            PixelFormat desiredFormat = PF_UNKNOWN, bool hwGammaCorrection = false);+         /** Loads a texture from a file.             @param                 name The file to load, or a String identifier in some casesdiff -ruN ../cvs_fresh/OgreMain/src/OgreTextureManager.cpp ./OgreMain/src/OgreTextureManager.cpp--- ../cvs_fresh/OgreMain/src/OgreTextureManager.cpp   2007-12-08 18:37:27.000000000 +0000+++ ./OgreMain/src/OgreTextureManager.cpp   2008-03-27 22:08:43.202498807 +0000@@ -60,15 +60,17 @@      }     //------------------------------------------------------------------------    TexturePtr TextureManager::load(const String &name, const String& group,-        TextureType texType, int numMipmaps, Real gamma, bool isAlpha, PixelFormat desiredFormat, -      bool hwGamma)+    TextureManager::ResourceCreateOrRetrieveResult TextureManager::createOrRetrieve(+            const String &name, const String& group, bool isManual, ManualResourceLoader* loader,+            const NameValuePairList* createParams, TextureType texType, int numMipmaps, Real gamma,+            bool isAlpha, PixelFormat desiredFormat, bool hwGamma)     {-      ResourceCreateOrRetrieveResult res = createOrRetrieve(name, group);-        TexturePtr tex = res.first;+      ResourceCreateOrRetrieveResult res =+            Ogre::ResourceManager::createOrRetrieve(name, group, isManual, loader, createParams);       // Was it created?       if(res.second)         {+            TexturePtr tex = res.first;             tex->setTextureType(texType);             tex->setNumMipmaps((numMipmaps == MIP_DEFAULT)? mDefaultNumMipmaps :             static_cast<size_t>(numMipmaps));@@ -77,8 +79,28 @@             tex->setFormat(desiredFormat);          tex->setHardwareGammaEnabled(hwGamma);         }+        return res;+    }+    //-----------------------------------------------------------------------+    TexturePtr TextureManager::prepare(const String &name, const String& group, TextureType texType,+                                       int numMipmaps, Real gamma, bool isAlpha,+                                       PixelFormat desiredFormat, bool hwGamma)+    {+      ResourceCreateOrRetrieveResult res =+            createOrRetrieve(name,group,false,0,0,texType,numMipmaps,gamma,isAlpha,desiredFormat,hwGamma);+        TexturePtr tex = res.first;+      tex->prepare();+        return tex;+    }+    //-----------------------------------------------------------------------+    TexturePtr TextureManager::load(const String &name, const String& group, TextureType texType,+                                    int numMipmaps, Real gamma, bool isAlpha, PixelFormat desiredFormat,+                                    bool hwGamma)+    {+      ResourceCreateOrRetrieveResult res =+            createOrRetrieve(name,group,false,0,0,texType,numMipmaps,gamma,isAlpha,desiredFormat,hwGamma);+        TexturePtr tex = res.first;       tex->load();-         return tex;     }

This code that interfaces directly with the resource loading system, so needs updating to use the AtomicScalar instance. I also suppress the logging output using getVerbose().

Code: Select all

diff -ruN ../cvs_fresh/OgreMain/src/OgreTexture.cpp ./OgreMain/src/OgreTexture.cpp--- ../cvs_fresh/OgreMain/src/OgreTexture.cpp   2008-01-18 21:58:33.000000000 +0000+++ ./OgreMain/src/OgreTexture.cpp   2008-03-27 23:14:53.727473653 +0000@@ -93,16 +93,11 @@    //--------------------------------------------------------------------------        void Texture::loadImage( const Image &img )    {-      // Scope lock over load status-      {-         OGRE_LOCK_MUTEX(mLoadingStatusMutex)-         if (mLoadingState != LOADSTATE_UNLOADED)-         {-            // no loading to be done-            return;-         }-         mLoadingState = LOADSTATE_LOADING;-      }++        LoadingState old = mLoadingState.get();+        if (old!=LOADSTATE_UNLOADED && old!=LOADSTATE_PREPARED) return;++        if (!mLoadingState.cas(old,LOADSTATE_LOADING)) return;        // Scope lock for actual loading       try@@ -116,19 +111,12 @@       catch (...)       {          // Reset loading in-progress flag in case failed for some reason-         OGRE_LOCK_MUTEX(mLoadingStatusMutex)-         mLoadingState = LOADSTATE_UNLOADED;+         mLoadingState.set(old);          // Re-throw          throw;       } -      // Scope lock for loading progress-      {-         OGRE_LOCK_MUTEX(mLoadingStatusMutex)--         // Now loaded-         mLoadingState = LOADSTATE_LOADED;-      }+        mLoadingState.set(LOADSTATE_LOADED);        // Notify manager       if(mCreator)@@ -257,38 +245,40 @@       if(faces > getNumFaces())          faces = getNumFaces();       -      // Say what we're doing-      StringUtil::StrStreamType str;-      str << "Texture: " << mName << ": Loading " << faces << " faces"-         << "(" << PixelUtil::getFormatName(images[0]->getFormat()) << "," <<-         images[0]->getWidth() << "x" << images[0]->getHeight() << "x" << images[0]->getDepth() <<-         ") with ";-      if (!(mMipmapsHardwareGenerated && mNumMipmaps == 0))-         str << mNumMipmaps;-      if(mUsage & TU_AUTOMIPMAP)-      {-         if (mMipmapsHardwareGenerated)-            str << " hardware";+        if (TextureManager::getSingleton().getVerbose()) {+            // Say what we're doing+            StringUtil::StrStreamType str;+            str << "Texture: " << mName << ": Loading " << faces << " faces"+                << "(" << PixelUtil::getFormatName(images[0]->getFormat()) << "," <<+                images[0]->getWidth() << "x" << images[0]->getHeight() << "x" << images[0]->getDepth() <<+                ") with ";+            if (!(mMipmapsHardwareGenerated && mNumMipmaps == 0))+                str << mNumMipmaps;+            if(mUsage & TU_AUTOMIPMAP)+            {+                if (mMipmapsHardwareGenerated)+                    str << " hardware"; -         str << " generated mipmaps";-      }-      else-      {-         str << " custom mipmaps";-      }-       if(multiImage)-         str << " from multiple Images.";-      else-         str << " from Image.";-      // Scoped-      {-         // Print data about first destination surface-         HardwarePixelBufferSharedPtr buf = getBuffer(0, 0); -         str << " Internal format is " << PixelUtil::getFormatName(buf->getFormat()) << -         "," << buf->getWidth() << "x" << buf->getHeight() << "x" << buf->getDepth() << ".";-      }-      LogManager::getSingleton().logMessage( -            LML_NORMAL, str.str());+                str << " generated mipmaps";+            }+            else+            {+                str << " custom mipmaps";+            }+            if(multiImage)+                str << " from multiple Images.";+            else+                str << " from Image.";+            // Scoped+            {+                // Print data about first destination surface+                HardwarePixelBufferSharedPtr buf = getBuffer(0, 0); +                str << " Internal format is " << PixelUtil::getFormatName(buf->getFormat()) << +                "," << buf->getWidth() << "x" << buf->getHeight() << "x" << buf->getDepth() << ".";+            }+            LogManager::getSingleton().logMessage( +                    LML_NORMAL, str.str());+        }              // Main loading loop         // imageMips == 0 if the image has no custom mipmaps, otherwise contains the number of custom mips

I didn't touch the D3D9 textures except to suppress the logging if the app requests it.

Code: Select all

diff -ruN ../cvs_fresh/RenderSystems/Direct3D9/src/OgreD3D9Texture.cpp ./RenderSystems/Direct3D9/src/OgreD3D9Texture.cpp--- ../cvs_fresh/RenderSystems/Direct3D9/src/OgreD3D9Texture.cpp   2008-02-28 11:28:13.000000000 +0000+++ ./RenderSystems/Direct3D9/src/OgreD3D9Texture.cpp   2008-03-27 22:10:35.189097455 +0000@@ -1018,6 +1018,7 @@       mSrcDepth = depth;         mSrcFormat = format;       // say to the world what we are doing+        if (!TextureManager::getSingleton().getVerbose()) return;       switch (this->getTextureType())       {       case TEX_TYPE_1D:

Finally, I implemented prepareImpl for GLTexture. It uses Image objects (sometimes 1 of them, but for cube maps, 6 of them), rather than a MemoryDataStream, to cache the loaded data in RAM between calls to prepareImpl() and loadImpl(). I pulled some duplicated code out into a little function to make managing it easier. The diff doesn't work very well here, because of changes to whitespace. The changes are actually prety minimal.

Code: Select all

diff -ruN ../cvs_fresh/RenderSystems/GL/include/OgreGLTexture.h ./RenderSystems/GL/include/OgreGLTexture.h--- ../cvs_fresh/RenderSystems/GL/include/OgreGLTexture.h   2007-02-03 21:59:16.000000000 +0000+++ ./RenderSystems/GL/include/OgreGLTexture.h   2008-03-25 21:53:37.055048490 +0000@@ -63,6 +63,10 @@     protected:       /// @copydoc Texture::createInternalResourcesImpl       void createInternalResourcesImpl(void);+        /// @copydoc Resource::prepareImpl+        void prepareImpl(void);+        /// @copydoc Resource::unprepareImpl+        void unprepareImpl(void);         /// @copydoc Resource::loadImpl         void loadImpl(void);         /// @copydoc Resource::freeInternalResourcesImpl@@ -74,6 +78,14 @@          actually allocate the buffer       */       void _createSurfaceList();++        /** Vector of pointers to images that were pulled from disk by+            prepareLoad  but have yet to be pushed into texture memory+            by loadImpl.  Images should be deleted by loadImpl.+        */+        ConstImagePtrList mLoadImagePtrs;++     private:         GLuint mTextureID;         GLSupport& mGLSupport;diff -ruN ../cvs_fresh/RenderSystems/GL/src/OgreGLTexture.cpp ./RenderSystems/GL/src/OgreGLTexture.cpp--- ../cvs_fresh/RenderSystems/GL/src/OgreGLTexture.cpp   2008-01-18 21:58:33.000000000 +0000+++ ./RenderSystems/GL/src/OgreGLTexture.cpp   2008-03-26 02:12:48.689038996 +0000@@ -235,95 +235,100 @@       // This already does everything neccessary         createInternalResources();     }++    static inline void do_image_io(const String &name, const String &group,+                                   const String &ext,+                                   Image *img, ConstImagePtrList &imagePtrs,+                                   Resource *r)+    {+        DataStreamPtr dstream = +            ResourceGroupManager::getSingleton().openResource(+                name, group, true, r);+                +        img->load(dstream, ext);+        imagePtrs.push_back(img);+    }+    -    void GLTexture::loadImpl()+    void GLTexture::prepareImpl()     {-        if( mUsage & TU_RENDERTARGET )+        if( mUsage & TU_RENDERTARGET ) return;++        String baseName, ext;+        size_t pos = mName.find_last_of(".");+        baseName = mName.substr(0, pos);+        if( pos != String::npos )+            ext = mName.substr(pos+1);++        if(mTextureType == TEX_TYPE_1D || mTextureType == TEX_TYPE_2D || +            mTextureType == TEX_TYPE_3D)         {-            createRenderTexture();+            Image *img = new Image;++            do_image_io(mName, mGroup, ext, img, mLoadImagePtrs, this);++            // If this is a cube map, set the texture type flag accordingly.+            if (img->hasFlag(IF_CUBEMAP))+                mTextureType = TEX_TYPE_CUBE_MAP;+            // If this is a volumetric texture set the texture type flag accordingly.+            if(img->getDepth() > 1)+                mTextureType = TEX_TYPE_3D;+         }-        else+        else if (mTextureType == TEX_TYPE_CUBE_MAP)         {-         String baseName, ext;-         size_t pos = mName.find_last_of(".");-         baseName = mName.substr(0, pos);-         if( pos != String::npos )-            ext = mName.substr(pos+1);--         if(mTextureType == TEX_TYPE_1D || mTextureType == TEX_TYPE_2D || -                mTextureType == TEX_TYPE_3D)+            if(getSourceFileType() == "dds")             {-                Image img;-               // find & load resource data intro stream to allow resource-            // group changes if required-            DataStreamPtr dstream = -               ResourceGroupManager::getSingleton().openResource(-                  mName, mGroup, true, this);--                img.load(dstream, ext);--            // If this is a cube map, set the texture type flag accordingly.-                if (img.hasFlag(IF_CUBEMAP))-               mTextureType = TEX_TYPE_CUBE_MAP;-            // If this is a volumetric texture set the texture type flag accordingly.-            if(img.getDepth() > 1)-               mTextureType = TEX_TYPE_3D;--            // Call internal _loadImages, not loadImage since that's external and -            // will determine load status etc again-            ConstImagePtrList imagePtrs;-            imagePtrs.push_back(&img);-            _loadImages( imagePtrs );+                // XX HACK there should be a better way to specify whether +                // all faces are in the same file or not+                do_image_io(mName, mGroup, ext, new Image(), mLoadImagePtrs, this);             }-            else if (mTextureType == TEX_TYPE_CUBE_MAP)+            else             {-            if(getSourceFileType() == "dds")-            {-               // XX HACK there should be a better way to specify whether -               // all faces are in the same file or not-               Image img;-                  // find & load resource data intro stream to allow resource-               // group changes if required-               DataStreamPtr dstream = -                  ResourceGroupManager::getSingleton().openResource(-                     mName, mGroup, true, this);--                   img.load(dstream, ext);-               // Call internal _loadImages, not loadImage since that's external and -               // will determine load status etc again-               ConstImagePtrList imagePtrs;-               imagePtrs.push_back(&img);-               _loadImages( imagePtrs );-            }-            else-            {-               std::vector<Image> images(6);-               ConstImagePtrList imagePtrs;-               static const String suffixes[6] = {"_rt", "_lf", "_up", "_dn", "_fr", "_bk"};-   -               for(size_t i = 0; i < 6; i++)-               {-                  String fullName = baseName + suffixes[i];-                  if (!ext.empty())-                     fullName = fullName + "." + ext;-                     // find & load resource data intro stream to allow resource-                  // group changes if required-                  DataStreamPtr dstream = -                     ResourceGroupManager::getSingleton().openResource(-                        fullName, mGroup, true, this);-   -                  images[i].load(dstream, ext);-                  imagePtrs.push_back(&images[i]);-               }-   -               _loadImages( imagePtrs );-            }+                std::vector<Image> images(6);+                ConstImagePtrList imagePtrs;+                static const String suffixes[6] = {"_rt", "_lf", "_up", "_dn", "_fr", "_bk"};++                for(size_t i = 0; i < 6; i++)+                {+                    String fullName = baseName + suffixes[i];+                    if (!ext.empty())+                        fullName = fullName + "." + ext;+                    // find & load resource data intro stream to allow resource+                    // group changes if required+                    do_image_io(fullName,mGroup,ext,new Image(),mLoadImagePtrs,this);+                }             }-            else-                OGRE_EXCEPT( Exception::ERR_NOT_IMPLEMENTED, "**** Unknown texture type ****", "GLTexture::load" );         }+        else+            OGRE_EXCEPT( Exception::ERR_NOT_IMPLEMENTED, "**** Unknown texture type ****", "GLTexture::prepare" );     }    +    void GLTexture::unprepareImpl()+    {+        mLoadImagePtrs.clear();+    }++    void GLTexture::loadImpl()+    {+        if( mUsage & TU_RENDERTARGET )+        {+            createRenderTexture();+            return;+        }++        // Call internal _loadImages, not loadImage since that's external and +        // will determine load status etc again+        _loadImages(mLoadImagePtrs);++        for (ConstImagePtrList::iterator i=mLoadImagePtrs.begin(),+                                         j=mLoadImagePtrs.end() ; i!=j ; ++i) {+            delete (*i);+        }++        mLoadImagePtrs.clear();+    }+    //*************************************************************************          void GLTexture::freeInternalResourcesImpl()

I will put these patches into the tracker so people can get at them directly from there. It is probably best not to use these directly as they may have errors due to my editting into neat sections.