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

Minor issues with the Ogre API that can be trivial to fix
Post Reply
User avatar
m2codeGEN
Halfling
Posts: 52
Joined: Tue Apr 26, 2011 9:13 am
Location: Russia, Tver
x 2

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

Post by m2codeGEN »

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
masterfalcon
OGRE Team Member
OGRE Team Member
Posts: 4270
Joined: Sun Feb 25, 2007 4:56 am
Location: Bloomington, MN
x 126
Contact:

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

Post by masterfalcon »

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
m2codeGEN
Halfling
Posts: 52
Joined: Tue Apr 26, 2011 9:13 am
Location: Russia, Tver
x 2

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

Post by m2codeGEN »

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
Joined: Tue Apr 26, 2011 9:13 am
Location: Russia, Tver
x 2

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

Post by m2codeGEN »

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
Joined: Tue Apr 26, 2011 9:13 am
Location: Russia, Tver
x 2

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

Post by m2codeGEN »

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
masterfalcon
OGRE Team Member
OGRE Team Member
Posts: 4270
Joined: Sun Feb 25, 2007 4:56 am
Location: Bloomington, MN
x 126
Contact:

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

Post by masterfalcon »

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
m2codeGEN
Halfling
Posts: 52
Joined: Tue Apr 26, 2011 9:13 am
Location: Russia, Tver
x 2

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

Post by m2codeGEN »

Yes, change to

Code: Select all

if (depth > 1 && mTextureType != TEX_TYPE_2D_ARRAY) depth /= 2;
User avatar
m2codeGEN
Halfling
Posts: 52
Joined: Tue Apr 26, 2011 9:13 am
Location: Russia, Tver
x 2

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

Post by m2codeGEN »

fix in Commit 0e5366ff994d
https://bitbucket.org/sinbad/ogre/chang ... 366ff994d/
Thanks to masterfalcon
Post Reply