[Fixed]Wrong calc of number mipmaps in Texture 2D array [GL]

Minor issues with the Ogre API that can be trivial to fix

[Fixed]Wrong calc of number mipmaps in Texture 2D array [GL]

Postby m2codeGEN » Mon Apr 16, 2012 4:20 pm

Try to create and fill the texture 2D array with mipmap levels > 0.
You receive the erroneous appeal to memory.
I corrected this issue for GL RenderSystem. However time to deal with others Render Systems (DX, GLES) at me isn't present, therefore created this question.

In GL render system need fix OgreGLTexture.cpp
void GLTexture::createInternalResourcesImpl(void) for 1.8.0 line 220 and 259

Code: Select all
if (depth>1 && mTextureType != TEX_TYPE_2D_ARRAY)


http://www.ogre3d.org/mantis/view.php?id=527
Code: Select all
@@ -127,23 +127,24 @@
       if(mNumMipmaps>maxMips)
          mNumMipmaps = maxMips;
       
-      // Generate texture name
-        glGenTextures( 1, &mTextureID );
-      
+      // Generate texture object
+        glGenTextures(1, &mTextureID);
+
       // Set texture type
-      glBindTexture( getGLTextureTarget(), mTextureID );
+      GLenum nGlTexTarget = getGLTextureTarget();
+      glBindTexture(nGlTexTarget, mTextureID);
         
       // This needs to be set otherwise the texture doesn't get rendered
       if (GLEW_VERSION_1_2)
-         glTexParameteri( getGLTextureTarget(), GL_TEXTURE_MAX_LEVEL, mNumMipmaps );
+         glTexParameteri(nGlTexTarget, GL_TEXTURE_MAX_LEVEL, mNumMipmaps);
         
         // Set some misc default parameters so NVidia won't complain, these can of course be changed later
-        glTexParameteri(getGLTextureTarget(), GL_TEXTURE_MIN_FILTER, GL_NEAREST);
-        glTexParameteri(getGLTextureTarget(), GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+        glTexParameteri(nGlTexTarget, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+        glTexParameteri(nGlTexTarget, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
       if (GLEW_VERSION_1_2)
       {
-         glTexParameteri(getGLTextureTarget(), GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
-         glTexParameteri(getGLTextureTarget(), GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+         glTexParameteri(nGlTexTarget, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+         glTexParameteri(nGlTexTarget, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
       }
       
       // If we can do automip generation and the user desires this, do so
@@ -163,11 +164,8 @@
          mMipmapsHardwareGenerated = false;
       }
 #endif
-      if((mUsage & TU_AUTOMIPMAP) &&
-          mNumRequestedMipmaps && mMipmapsHardwareGenerated)
-        {
-            glTexParameteri( getGLTextureTarget(), GL_GENERATE_MIPMAP, GL_TRUE );
-        }
+      if ((mUsage & TU_AUTOMIPMAP) != 0 && mNumRequestedMipmaps && mMipmapsHardwareGenerated)
+            glTexParameteri(nGlTexTarget, GL_GENERATE_MIPMAP, GL_TRUE);
       
       // Allocate internal buffer so that glTexSubImageXD can be used
       // Internal format
@@ -176,7 +174,7 @@
       size_t height = mHeight;
       size_t depth = mDepth;
 
-      if(PixelUtil::isCompressed(mFormat))
+      if (PixelUtil::isCompressed(mFormat))
       {
          // Compressed formats
          size_t size = PixelUtil::getMemorySize(mWidth, mHeight, mDepth, mFormat);
@@ -185,83 +183,87 @@
          // Run through this process for every mipmap to pregenerate mipmap piramid
          uint8 *tmpdata = new uint8[size];
          memset(tmpdata, 0, size);
-         
-         for(size_t mip=0; mip<=mNumMipmaps; mip++)
+
+         for (size_t mip = 0; mip <= mNumMipmaps; ++mip)
          {
             size = PixelUtil::getMemorySize(width, height, depth, mFormat);
             switch(mTextureType)
             {
-               case TEX_TYPE_1D:
-                  glCompressedTexImage1DARB(GL_TEXTURE_1D, mip, format,
-                     width, 0,
-                     size, tmpdata);
-                  break;
-               case TEX_TYPE_2D:
-                  glCompressedTexImage2DARB(GL_TEXTURE_2D, mip, format,
+            case TEX_TYPE_1D:
+               glCompressedTexImage1DARB(GL_TEXTURE_1D, mip, format,
+                  width, 0,
+                  size, tmpdata);
+               break;
+            case TEX_TYPE_2D:
+               glCompressedTexImage2DARB(GL_TEXTURE_2D, mip, format,
+                  width, height, 0,
+                  size, tmpdata);
+               break;
+            case TEX_TYPE_2D_ARRAY:
+            case TEX_TYPE_3D:
+               glCompressedTexImage3DARB(nGlTexTarget, mip, format,
+                  width, height, depth, 0,
+                  size, tmpdata);
+               break;
+            case TEX_TYPE_CUBE_MAP:
+               for (int face = 0; face < 6; ++face)
+               {
+                  glCompressedTexImage2DARB(GL_TEXTURE_CUBE_MAP_POSITIVE_X + face, mip, format,
                      width, height, 0,
                      size, tmpdata);
-                  break;
-               case TEX_TYPE_2D_ARRAY:
-               case TEX_TYPE_3D:
-                  glCompressedTexImage3DARB(getGLTextureTarget(), mip, format,
-                     width, height, depth, 0,
-                     size, tmpdata);
-                  break;
-               case TEX_TYPE_CUBE_MAP:
-                  for(int face=0; face<6; face++) {
-                     glCompressedTexImage2DARB(GL_TEXTURE_CUBE_MAP_POSITIVE_X + face, mip, format,
-                        width, height, 0,
-                        size, tmpdata);
-                  }
-                  break;
+               }
+               break;
             };
-            if(width>1)      width = width/2;
-            if(height>1)   height = height/2;
-            if(depth>1)      depth = depth/2;
+
+            width = width > 1 ? width / 2 : width;
+            height = height > 1 ? height / 2 : height;
+            depth =  depth > 1 ? (mTextureType != TEX_TYPE_2D_ARRAY ? depth / 2 : depth) : depth;
          }
          delete [] tmpdata;
       }
       else
       {
          // Run through this process to pregenerate mipmap pyramid
-         for(size_t mip=0; mip<=mNumMipmaps; mip++)
+         for (size_t mip = 0; mip <= mNumMipmaps; ++mip)
          {
             // Normal formats
             switch(mTextureType)
             {
-               case TEX_TYPE_1D:
-                  glTexImage1D(GL_TEXTURE_1D, mip, format,
-                     width, 0,
-                     GL_RGBA, GL_UNSIGNED_BYTE, 0);
-   
-                  break;
-               case TEX_TYPE_2D:
-                  glTexImage2D(GL_TEXTURE_2D, mip, format,
+            case TEX_TYPE_1D:
+               glTexImage1D(GL_TEXTURE_1D, mip, format,
+                  width, 0,
+                  GL_RGBA, GL_UNSIGNED_BYTE, 0);
+
+               break;
+            case TEX_TYPE_2D:
+               glTexImage2D(GL_TEXTURE_2D, mip, format,
+                  width, height, 0,
+                  GL_RGBA, GL_UNSIGNED_BYTE, 0);
+               break;
+            case TEX_TYPE_2D_ARRAY:
+            case TEX_TYPE_3D:
+               glTexImage3D(nGlTexTarget, mip, format,
+                  width, height, depth, 0,
+                  GL_RGBA, GL_UNSIGNED_BYTE, 0);
+               break;
+            case TEX_TYPE_CUBE_MAP:
+               for (int face = 0; face < 6; ++face)
+               {
+                  glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + face, mip, format,
                      width, height, 0,
                      GL_RGBA, GL_UNSIGNED_BYTE, 0);
-                  break;
-               case TEX_TYPE_2D_ARRAY:
-               case TEX_TYPE_3D:
-                  glTexImage3D(getGLTextureTarget(), mip, format,
-                     width, height, depth, 0,
-                     GL_RGBA, GL_UNSIGNED_BYTE, 0);
-                  break;
-               case TEX_TYPE_CUBE_MAP:
-                  for(int face=0; face<6; face++) {
-                     glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + face, mip, format,
-                        width, height, 0,
-                        GL_RGBA, GL_UNSIGNED_BYTE, 0);
-                  }
-                  break;
+               }
+               break;
             };
-            if(width>1)      width = width/2;
-            if(height>1)   height = height/2;
-            if(depth>1)      depth = depth/2;
+
+            width = width > 1 ? width / 2 : width;
+            height = height > 1 ? height / 2 : height;
+            depth =  depth > 1 ? (mTextureType != TEX_TYPE_2D_ARRAY ? depth / 2 : depth) : depth;
          }
       }
+
       _createSurfaceList();
-      // Get final internal format
-      mFormat = getBuffer(0,0)->getFormat();
+      mFormat = getBuffer(0,0)->getFormat(); // Get final internal format
    }
    
     void GLTexture::createRenderTexture(void)
@@ -394,16 +396,20 @@
       // only when mipmap generation is desired.
       bool doSoftware = wantGeneratedMips && !mMipmapsHardwareGenerated && getNumMipmaps();
       
-      for(size_t face=0; face<getNumFaces(); face++)
+      GLenum nGlTexTarget = getGLTextureTarget();
+      HardwareBuffer::Usage nUsage = static_cast<HardwareBuffer::Usage>(mUsage);
+
+      for (size_t face = 0, numFaces = getNumFaces(); face < numFaces; ++face)
       {
-         for(size_t mip=0; mip<=getNumMipmaps(); mip++)
+         for (size_t mip = 0, numMipmaps = getNumMipmaps(); mip <= numMipmaps; ++mip)
          {
-                GLHardwarePixelBuffer *buf = new GLTextureBuffer(mName, getGLTextureTarget(), mTextureID, face, mip,
-                  static_cast<HardwareBuffer::Usage>(mUsage), doSoftware && mip==0, mHwGamma, mFSAA);
+                GLHardwarePixelBuffer *buf = new GLTextureBuffer(mName, nGlTexTarget, mTextureID, face,
+               mip, nUsage, doSoftware && mip == 0, mHwGamma, mFSAA);
+
             mSurfaceList.push_back(HardwarePixelBufferSharedPtr(buf));
                 
                 /// Check for error
-                if(buf->getWidth()==0 || buf->getHeight()==0 || buf->getDepth()==0)
+                if (buf->getWidth() == 0 || buf->getHeight() == 0 || buf->getDepth() == 0)
                 {
                     OGRE_EXCEPT(
                         Exception::ERR_RENDERINGAPI_ERROR,
Last edited by m2codeGEN on Tue Apr 24, 2012 8:20 am, edited 3 times in total.
User avatar
m2codeGEN
Halfling
 
Posts: 52
Kudos: 2
Joined: 26 Apr 2011
Location: Russia, Tver

Re: Wrong calculation of number mipmaps in Texture 2D array

Postby masterfalcon » Mon Apr 16, 2012 4:25 pm

Well, don't worry about GL ES, it doesn't support texture arrays at this time. What exactly do you mean by "erroneous appeal to memory"?

Also, this doesn't appear to be a valid patch file.
User avatar
masterfalcon
OGRE Team Member
OGRE Team Member
 
Posts: 4264
Kudos: 129
Joined: 25 Feb 2007
Location: Bloomington, MN

Re: Wrong calculation of number mipmaps in Texture 2D array

Postby m2codeGEN » Tue Apr 17, 2012 9:54 am

Incorrect memory access.
Let's consider on textural array of dimensionality 1024 on 1024 with depth 32.
Mipmaps number is 10.

Iteration dimension (with patch)
1 1024 x 1024 x 32 (1024 x 1024 x 32)
2 512 x 512 x 16 (512 x 512 x 32)
3 256 x 256 x 8 (256 x 256 x 32)
4 128 x 128 x 4 (128 x 128 x 32)

etc...

Further when I want to fill a textural array with textures with mipmap levels I call methods HardwarePixelBufferSharedPtr Texture::getBuffer(0, mipmap)
then void* HardwarePixelBuffer::lock(0, bufLength, discard);

For mipmap level 1 HardwarePixelBuffer::getDepth() return 16 instead 32
It occurs because in constructor of class GLTextureBuffer there is a determination of the real extent of depth
Code: Select all
   // Get depth
   if(target != GL_TEXTURE_3D && target != GL_TEXTURE_2D_ARRAY_EXT)
      value = 1; // Depth always 1 for non-3D textures
   else
      glGetTexLevelParameteriv(mFaceTarget, level, GL_TEXTURE_DEPTH, &value);
   mDepth = value;


Initialization of depth occurs in a class void GLTexture::createInternalResourcesImpl(void)
glCompressedTexImage3DARB
Code: Select all
         // Compressed formats
         size_t size = PixelUtil::getMemorySize(mWidth, mHeight, mDepth, mFormat);
         // Provide temporary buffer filled with zeroes as glCompressedTexImageXD does not
         // accept a 0 pointer like normal glTexImageXD
         // Run through this process for every mipmap to pregenerate mipmap piramid
         uint8 *tmpdata = new uint8[size];
         memset(tmpdata, 0, size);
         
         for(size_t mip=0; mip<=mNumMipmaps; mip++)
         {
            size = PixelUtil::getMemorySize(width, height, depth, mFormat);
            switch(mTextureType)
            {
               case TEX_TYPE_1D:
                  glCompressedTexImage1DARB(GL_TEXTURE_1D, mip, format,
                     width, 0,
                     size, tmpdata);
                  break;
               case TEX_TYPE_2D:
                  glCompressedTexImage2DARB(GL_TEXTURE_2D, mip, format,
                     width, height, 0,
                     size, tmpdata);
                  break;
               case TEX_TYPE_2D_ARRAY:
               case TEX_TYPE_3D:
                  glCompressedTexImage3DARB(getGLTextureTarget(), mip, format,
                     width, height, depth, 0,
                     size, tmpdata);
                  break;
               case TEX_TYPE_CUBE_MAP:
                  for(int face=0; face<6; face++) {
                     glCompressedTexImage2DARB(GL_TEXTURE_CUBE_MAP_POSITIVE_X + face, mip, format,
                        width, height, 0,
                        size, tmpdata);
                  }
                  break;
            };
            if(width>1)      width = width/2;
            if(height>1)   height = height/2;
            if(depth>1)      depth = depth/2;
         }
         delete [] tmpdata;


and uncompressed glTexImage3D

Code: Select all
// Run through this process to pregenerate mipmap pyramid
         for(size_t mip=0; mip<=mNumMipmaps; mip++)
         {
            // Normal formats
            switch(mTextureType)
            {
               case TEX_TYPE_1D:
                  glTexImage1D(GL_TEXTURE_1D, mip, format,
                     width, 0,
                     GL_RGBA, GL_UNSIGNED_BYTE, 0);
   
                  break;
               case TEX_TYPE_2D:
                  glTexImage2D(GL_TEXTURE_2D, mip, format,
                     width, height, 0,
                     GL_RGBA, GL_UNSIGNED_BYTE, 0);
                  break;
               case TEX_TYPE_2D_ARRAY:
               case TEX_TYPE_3D:
                  glTexImage3D(getGLTextureTarget(), mip, format,
                     width, height, depth, 0,
                     GL_RGBA, GL_UNSIGNED_BYTE, 0);
                  break;
               case TEX_TYPE_CUBE_MAP:
                  for(int face=0; face<6; face++) {
                     glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + face, mip, format,
                        width, height, 0,
                        GL_RGBA, GL_UNSIGNED_BYTE, 0);
                  }
                  break;
            };
            if(width>1)      width = width/2;
            if(height>1)   height = height/2;
            if(depth>1)      depth = depth/2;
         }
User avatar
m2codeGEN
Halfling
 
Posts: 52
Kudos: 2
Joined: 26 Apr 2011
Location: Russia, Tver

Re: Wrong calculation of number mipmaps in Texture 2D array

Postby m2codeGEN » Tue Apr 17, 2012 10:03 am

I suspect that textural arrays aren't supported or aren't implemented for DX9 void D3D9Texture::createInternalResourcesImpl(IDirect3DDevice9* d3d9Device)
Code: Select all
      // load based on tex.type
      switch (getTextureType())
      {
      case TEX_TYPE_1D:
      case TEX_TYPE_2D:
         _createNormTex(d3d9Device);
         break;
      case TEX_TYPE_CUBE_MAP:
         _createCubeTex(d3d9Device);
         break;
      case TEX_TYPE_3D:
         _createVolumeTex(d3d9Device);
         break;
      default:
         freeInternalResources();
         OGRE_EXCEPT( Exception::ERR_INTERNAL_ERROR, "Unknown texture type", "D3D9Texture::createInternalResources" );
      }
User avatar
m2codeGEN
Halfling
 
Posts: 52
Kudos: 2
Joined: 26 Apr 2011
Location: Russia, Tver

Re: Wrong calculation of number mipmaps in Texture 2D array

Postby m2codeGEN » Tue Apr 17, 2012 10:13 am

masterfalcon wrote:Also, this doesn't appear to be a valid patch file.

This is diff from our SVN Ogre repository clone

Microscopic optimization :D
I replaced method calls of getGLTextureTarget with local variable of nGlTexTarget
Code: Select all
GLenum nGlTexTarget = getGLTextureTarget();
glBindTexture(nGlTexTarget, mTextureID);
User avatar
m2codeGEN
Halfling
 
Posts: 52
Kudos: 2
Joined: 26 Apr 2011
Location: Russia, Tver

Re: Wrong calc of number mipmaps in Texture 2D array [GL]

Postby masterfalcon » Tue Apr 17, 2012 3:36 pm

If I read your patch correctly, the change is to this piece of code in the two places, correct?

Code: Select all
            if(depth>1)      depth = depth/2;
User avatar
masterfalcon
OGRE Team Member
OGRE Team Member
 
Posts: 4264
Kudos: 129
Joined: 25 Feb 2007
Location: Bloomington, MN

Re: Wrong calc of number mipmaps in Texture 2D array [GL]

Postby m2codeGEN » Wed Apr 18, 2012 8:02 am

Yes, change to
Code: Select all
if (depth > 1 && mTextureType != TEX_TYPE_2D_ARRAY) depth /= 2;
User avatar
m2codeGEN
Halfling
 
Posts: 52
Kudos: 2
Joined: 26 Apr 2011
Location: Russia, Tver

Re: [Fixed]Wrong calc of number mipmaps in Texture 2D array

Postby m2codeGEN » Tue Apr 24, 2012 8:21 am

fix in Commit 0e5366ff994d
https://bitbucket.org/sinbad/ogre/chang ... 366ff994d/
Thanks to masterfalcon
User avatar
m2codeGEN
Halfling
 
Posts: 52
Kudos: 2
Joined: 26 Apr 2011
Location: Russia, Tver


Return to Papercuts

Who is online

Users browsing this forum: No registered users and 2 guests