From 417069f152e6f4e2f50e7b41b0505765302eb823 Mon Sep 17 00:00:00 2001
From: Xiaohong Bao <bao@lindenlab.com>
Date: Tue, 22 Feb 2011 11:22:50 -0700
Subject: [PATCH] more fix for SH-895/STORM-336: memory leaking. fixed vertex
 buffer caused leaking.

---
 indra/llrender/llvertexbuffer.cpp | 185 ++++++++++++++++--------------
 indra/llrender/llvertexbuffer.h   |  10 +-
 2 files changed, 100 insertions(+), 95 deletions(-)

diff --git a/indra/llrender/llvertexbuffer.cpp b/indra/llrender/llvertexbuffer.cpp
index 660dc14d026..71bdca60e55 100644
--- a/indra/llrender/llvertexbuffer.cpp
+++ b/indra/llrender/llvertexbuffer.cpp
@@ -213,11 +213,6 @@ void LLVertexBuffer::drawRange(U32 mode, U32 start, U32 end, U32 count, U32 indi
 {
 	llassert(mRequestedNumVerts >= 0);
 
-	if(mDirty)
-	{
-		postUpdate() ;
-	}
-
 	if (start >= (U32) mRequestedNumVerts ||
 	    end >= (U32) mRequestedNumVerts)
 	{
@@ -258,11 +253,6 @@ void LLVertexBuffer::draw(U32 mode, U32 count, U32 indices_offset) const
 {
 	llassert(mRequestedNumIndices >= 0);
 
-	if(mDirty)
-	{
-		postUpdate() ;
-	}
-
 	if (indices_offset >= (U32) mRequestedNumIndices ||
 	    indices_offset + count > (U32) mRequestedNumIndices)
 	{
@@ -295,11 +285,6 @@ void LLVertexBuffer::drawArrays(U32 mode, U32 first, U32 count) const
 {
 	llassert(mRequestedNumVerts >= 0);
 
-	if(mDirty)
-	{
-		postUpdate() ;
-	}
-
 	if (first >= (U32) mRequestedNumVerts ||
 	    first + count > (U32) mRequestedNumVerts)
 	{
@@ -326,7 +311,7 @@ void LLVertexBuffer::drawArrays(U32 mode, U32 first, U32 count) const
 void LLVertexBuffer::initClass(bool use_vbo, bool no_vbo_mapping)
 {
 	sEnableVBOs = use_vbo;
-	sDisableVBOMapping = no_vbo_mapping ;
+	sDisableVBOMapping = sEnableVBOs && no_vbo_mapping ;
 	LLGLNamePool::registerPool(&sDynamicVBOPool);
 	LLGLNamePool::registerPool(&sDynamicIBOPool);
 	LLGLNamePool::registerPool(&sStreamVBOPool);
@@ -388,8 +373,7 @@ LLVertexBuffer::LLVertexBuffer(U32 typemask, S32 usage) :
 	mFilthy(FALSE),
 	mEmpty(TRUE),
 	mResized(FALSE),
-	mDynamicSize(FALSE),
-	mDirty(FALSE)
+	mDynamicSize(FALSE)
 {
 	LLMemType mt2(LLMemType::MTYPE_VERTEX_CONSTRUCTOR);
 	if (!sEnableVBOs)
@@ -442,6 +426,8 @@ LLVertexBuffer::~LLVertexBuffer()
 	destroyGLBuffer();
 	destroyGLIndices();
 	sCount--;
+
+	llassert_always(!mMappedData && !mMappedIndexData) ;
 };
 
 //----------------------------------------------------------------------------
@@ -858,48 +844,24 @@ void LLVertexBuffer::freeClientBuffer()
 	}
 }
 
-void LLVertexBuffer::preUpdate()
+void LLVertexBuffer::allocateClientVertexBuffer()
 {
-	if(!useVBOs() || !sDisableVBOMapping)
-	{
-		return ;
-	}
-
 	if(!mMappedData)
 	{
 		U32 size = getSize() ;
 		mMappedData = new U8[size];
 		memset(mMappedData, 0, size);
 	}
+}
 
+void LLVertexBuffer::allocateClientIndexBuffer()
+{
 	if(!mMappedIndexData)
 	{
 		U32 size = getIndicesSize();
 		mMappedIndexData = new U8[size];
 		memset(mMappedIndexData, 0, size);
 	}
-
-	mDirty = TRUE ;
-}
-
-void LLVertexBuffer::postUpdate() const
-{
-	if(!useVBOs() || !sDisableVBOMapping)
-	{
-		return ;
-	}
-
-	llassert_always(mMappedData && mMappedIndexData) ;
-
-	//release the existing buffers
-	glBufferDataARB(GL_ARRAY_BUFFER_ARB, 0, NULL, mUsage);
-	glBufferDataARB(GL_ELEMENT_ARRAY_BUFFER_ARB, 0, NULL, mUsage);
-
-	//update to the new buffers
-	glBufferDataARB(GL_ARRAY_BUFFER_ARB, getSize(), mMappedData, mUsage);
-	glBufferDataARB(GL_ELEMENT_ARRAY_BUFFER_ARB, getIndicesSize(), mMappedIndexData, mUsage);
-
-	mDirty = FALSE ;
 }
 
 // Map for data access
@@ -915,12 +877,6 @@ U8* LLVertexBuffer::mapBuffer(S32 access)
 		llerrs << "LLVertexBuffer::mapBuffer() called on unallocated buffer." << llendl;
 	}
 		
-	if(useVBOs() && sDisableVBOMapping)
-	{
-		preUpdate() ;
-		return mMappedData ;
-	}
-
 	if (!mLocked && useVBOs())
 	{
 		{
@@ -928,12 +884,28 @@ U8* LLVertexBuffer::mapBuffer(S32 access)
 			setBuffer(0);
 			mLocked = TRUE;
 			stop_glerror();	
-			mMappedData = (U8*) glMapBufferARB(GL_ARRAY_BUFFER_ARB, GL_WRITE_ONLY_ARB);
+
+			if(sDisableVBOMapping)
+			{
+				allocateClientVertexBuffer() ;
+			}
+			else
+			{
+				mMappedData = (U8*) glMapBufferARB(GL_ARRAY_BUFFER_ARB, GL_WRITE_ONLY_ARB);
+			}
 			stop_glerror();
 		}
 		{
 			LLMemType mt_v(LLMemType::MTYPE_VERTEX_MAP_BUFFER_INDICES);
-			mMappedIndexData = (U8*) glMapBufferARB(GL_ELEMENT_ARRAY_BUFFER_ARB, GL_WRITE_ONLY_ARB);
+
+			if(sDisableVBOMapping)
+			{
+				allocateClientIndexBuffer() ;
+			}
+			else
+			{
+				mMappedIndexData = (U8*) glMapBufferARB(GL_ELEMENT_ARRAY_BUFFER_ARB, GL_WRITE_ONLY_ARB);
+			}
 			stop_glerror();
 		}
 
@@ -947,37 +919,51 @@ U8* LLVertexBuffer::mapBuffer(S32 access)
 			llinfos << "Available physical mwmory(KB): " << avail_phy_mem << llendl ; 
 			llinfos << "Available virtual memory(KB): " << avail_vir_mem << llendl;
 
-			//--------------------
-			//print out more debug info before crash
-			llinfos << "vertex buffer size: (num verts : num indices) = " << getNumVerts() << " : " << getNumIndices() << llendl ;
-			GLint size ;
-			glGetBufferParameterivARB(GL_ARRAY_BUFFER_ARB, GL_BUFFER_SIZE_ARB, &size) ;
-			llinfos << "GL_ARRAY_BUFFER_ARB size is " << size << llendl ;
-			//--------------------
+			if(!sDisableVBOMapping)
+			{
+				//--------------------
+				//print out more debug info before crash
+				llinfos << "vertex buffer size: (num verts : num indices) = " << getNumVerts() << " : " << getNumIndices() << llendl ;
+				GLint size ;
+				glGetBufferParameterivARB(GL_ARRAY_BUFFER_ARB, GL_BUFFER_SIZE_ARB, &size) ;
+				llinfos << "GL_ARRAY_BUFFER_ARB size is " << size << llendl ;
+				//--------------------
 
-			GLint buff;
-			glGetIntegerv(GL_ARRAY_BUFFER_BINDING_ARB, &buff);
-			if ((GLuint)buff != mGLBuffer)
+				GLint buff;
+				glGetIntegerv(GL_ARRAY_BUFFER_BINDING_ARB, &buff);
+				if ((GLuint)buff != mGLBuffer)
+				{
+					llerrs << "Invalid GL vertex buffer bound: " << buff << llendl;
+				}
+
+				
+				llerrs << "glMapBuffer returned NULL (no vertex data)" << llendl;
+			}
+			else
 			{
-				llerrs << "Invalid GL vertex buffer bound: " << buff << llendl;
+				llerrs << "memory allocation for vertex data failed." << llendl ;
 			}
-
-			
-			llerrs << "glMapBuffer returned NULL (no vertex data)" << llendl;
 		}
 
 		if (!mMappedIndexData)
 		{
 			log_glerror();
 
-			GLint buff;
-			glGetIntegerv(GL_ELEMENT_ARRAY_BUFFER_BINDING_ARB, &buff);
-			if ((GLuint)buff != mGLIndices)
+			if(!sDisableVBOMapping)
 			{
-				llerrs << "Invalid GL index buffer bound: " << buff << llendl;
-			}
+				GLint buff;
+				glGetIntegerv(GL_ELEMENT_ARRAY_BUFFER_BINDING_ARB, &buff);
+				if ((GLuint)buff != mGLIndices)
+				{
+					llerrs << "Invalid GL index buffer bound: " << buff << llendl;
+				}
 
-			llerrs << "glMapBuffer returned NULL (no index data)" << llendl;
+				llerrs << "glMapBuffer returned NULL (no index data)" << llendl;
+			}
+			else
+			{
+				llerrs << "memory allocation for Index data failed. " << llendl ;
+			}
 		}
 
 		sMappedCount++;
@@ -991,17 +977,31 @@ void LLVertexBuffer::unmapBuffer()
 	LLMemType mt2(LLMemType::MTYPE_VERTEX_UNMAP_BUFFER);
 	if (mMappedData || mMappedIndexData)
 	{
-		if(sDisableVBOMapping && useVBOs())
+		if (useVBOs() && mLocked)
 		{
-			return ;
-		}
-		else if (useVBOs() && mLocked)
-		{
-			stop_glerror();
-			glUnmapBufferARB(GL_ARRAY_BUFFER_ARB);
-			stop_glerror();
-			glUnmapBufferARB(GL_ELEMENT_ARRAY_BUFFER_ARB);
-			stop_glerror();
+			if(sDisableVBOMapping)
+			{
+				if(mMappedData)
+				{
+					stop_glerror();
+					glBufferSubDataARB(GL_ARRAY_BUFFER_ARB, 0, getSize(), mMappedData);
+					stop_glerror();
+				}
+				if(mMappedIndexData)
+				{
+					stop_glerror();
+					glBufferSubDataARB(GL_ELEMENT_ARRAY_BUFFER_ARB, 0, getIndicesSize(), mMappedIndexData);
+					stop_glerror();
+				}
+			}
+			else
+			{
+				stop_glerror();
+				glUnmapBufferARB(GL_ARRAY_BUFFER_ARB);
+				stop_glerror();
+				glUnmapBufferARB(GL_ELEMENT_ARRAY_BUFFER_ARB);
+				stop_glerror();
+			}
 
 			/*if (!sMapped)
 			{
@@ -1015,15 +1015,22 @@ void LLVertexBuffer::unmapBuffer()
 				//throw out client data (we won't be using it again)
 				mEmpty = TRUE;
 				mFinal = TRUE;
+
+				if(sDisableVBOMapping)
+				{
+					freeClientBuffer() ;
+				}
 			}
 			else
 			{
 				mEmpty = FALSE;
 			}
 
-			mMappedIndexData = NULL;
-			mMappedData = NULL;
-			
+			if(!sDisableVBOMapping)
+			{
+				mMappedIndexData = NULL;
+				mMappedData = NULL;
+			}
 			mLocked = FALSE;
 		}
 	}
@@ -1241,13 +1248,13 @@ void LLVertexBuffer::setBuffer(U32 data_mask)
 				}
 			}
 
-			if (mGLBuffer && !sDisableVBOMapping)
+			if (mGLBuffer)
 			{
 				stop_glerror();
 				glBufferDataARB(GL_ARRAY_BUFFER_ARB, getSize(), NULL, mUsage);
 				stop_glerror();
 			}
-			if (mGLIndices && !sDisableVBOMapping)
+			if (mGLIndices)
 			{
 				stop_glerror();
 				glBufferDataARB(GL_ELEMENT_ARRAY_BUFFER_ARB, getIndicesSize(), NULL, mUsage);
diff --git a/indra/llrender/llvertexbuffer.h b/indra/llrender/llvertexbuffer.h
index 18d50c87bb3..09a16d5b9dc 100644
--- a/indra/llrender/llvertexbuffer.h
+++ b/indra/llrender/llvertexbuffer.h
@@ -152,11 +152,10 @@ class LLVertexBuffer : public LLRefCount
 	void	allocateBuffer(S32 nverts, S32 nindices, bool create);
 	virtual void resizeBuffer(S32 newnverts, S32 newnindices);
 		
-	void preUpdate() ;
-	void postUpdate() const ;
 	void freeClientBuffer() ;
-	void dirty() {mDirty = TRUE;}
-
+	void allocateClientVertexBuffer() ;
+	void allocateClientIndexBuffer() ;
+	
 	// Only call each getVertexPointer, etc, once before calling unmapBuffer()
 	// call unmapBuffer() after calls to getXXXStrider() before any cals to setBuffer()
 	// example:
@@ -221,8 +220,7 @@ class LLVertexBuffer : public LLRefCount
 	S32		mOffsets[TYPE_MAX];
 	BOOL	mResized;		// if TRUE, client buffer has been resized and GL buffer has not
 	BOOL	mDynamicSize;	// if TRUE, buffer has been resized at least once (and should be padded)
-	mutable BOOL    mDirty ;
-
+	
 	class DirtyRegion
 	{
 	public:
-- 
GitLab