1 files changed, 122 insertions, 53 deletions

diff --git a/indra/newview/llglsandbox.cpp b/indra/newview/llglsandbox.cpp
index c80dec0e75..dc18d6c647 100644
--- a/indra/newview/llglsandbox.cpp
+++ b/indra/newview/llglsandbox.cpp
@@ -64,6 +64,8 @@
 #include "llspatialpartition.h"
 #include "llviewershadermgr.h"
 
+#include <vector>
+
 // Height of the yellow selection highlight posts for land
 const F32 PARCEL_POST_HEIGHT = 0.666f;
 
@@ -921,14 +923,67 @@ F32 gpu_benchmark()
 	//number of samples to take
 	const S32 samples = 64;
 
-	if (gGLManager.mHasTimerQuery)
+	// This struct is used to ensure that once we call initProfile(), it will
+	// definitely be matched by a corresponding call to finishProfile(). It's
+	// a struct rather than a class simply because every member is public.
+	struct ShaderProfileHelper
 	{
-		LLGLSLShader::initProfile();
-	}
+		ShaderProfileHelper()
+		{
+			LLGLSLShader::initProfile();
+		}
+		~ShaderProfileHelper()
+		{
+			LLGLSLShader::finishProfile(false);
+		}
+	};
+	ShaderProfileHelper initProfile;
+
+	// This helper class is used to ensure that each generateTextures() call
+	// is matched by a corresponding deleteTextures() call. It also handles
+	// the bindManual() calls using those textures.
+	class TextureHolder
+	{
+	public:
+		TextureHolder(U32 unit, U32 size):
+			texUnit(gGL.getTexUnit(unit)),
+			source(size)			// preallocate vector
+		{
+			// takes (count, pointer)
+			// &vector[0] gets pointer to contiguous array
+			LLImageGL::generateTextures(source.size(), &source[0]);
+		}
+
+		~TextureHolder()
+		{
+			// unbind
+			if (texUnit)
+			{
+				texUnit->unbind(LLTexUnit::TT_TEXTURE);
+			}
+			// ensure that we delete these textures regardless of how we exit
+			LLImageGL::deleteTextures(source.size(), &source[0]);
+		}
 
-	LLRenderTarget dest[count];
-	U32 source[count];
-	LLImageGL::generateTextures(count, source);
+		bool bind(U32 index)
+		{
+			if (texUnit) // should always be there with dummy (-1), but just in case
+			{
+				return texUnit->bindManual(LLTexUnit::TT_TEXTURE, source[index]);
+			}
+			return false;
+		}
+
+	private:
+		// capture which LLTexUnit we're going to use
+		LLTexUnit* texUnit;
+
+		// use std::vector for implicit resource management
+		std::vector<U32> source;
+	};
+
+	std::vector<LLRenderTarget> dest(count);
+	TextureHolder texHolder(0, count);
 	std::vector<F32> results;
 
 	//build a random texture
@@ -944,13 +999,27 @@ F32 gpu_benchmark()
 	LLGLDepthTest depth(GL_FALSE);
 
 	for (U32 i = 0; i < count; ++i)
-	{ //allocate render targets and textures
-		dest[i].allocate(res,res,GL_RGBA,false, false, LLTexUnit::TT_TEXTURE, true);
+	{
+		//allocate render targets and textures
+		if (!dest[i].allocate(res, res, GL_RGBA, false, false, LLTexUnit::TT_TEXTURE, true))
+		{
+			LL_WARNS() << "Failed to allocate render target." << LL_ENDL;
+			// abandon the benchmark test
+			delete[] pixels;
+			return -1.f;
+		}
 		dest[i].bindTarget();
 		dest[i].clear();
 		dest[i].flush();
 
-		gGL.getTexUnit(0)->bindManual(LLTexUnit::TT_TEXTURE, source[i]);
+		if (!texHolder.bind(i))
+		{
+			// can use a dummy value mDummyTexUnit = new LLTexUnit(-1);
+			LL_WARNS() << "Failed to bind tex unit." << LL_ENDL;
+			// abandon the benchmark test
+			delete[] pixels;
+			return -1.f;
+		}
 		LLImageGL::setManualImage(GL_TEXTURE_2D, 0, GL_RGBA, res,res,GL_RGBA, GL_UNSIGNED_BYTE, pixels);
 	}
 
@@ -958,22 +1027,52 @@ F32 gpu_benchmark()
 
 	//make a dummy triangle to draw with
 	LLPointer<LLVertexBuffer> buff = new LLVertexBuffer(LLVertexBuffer::MAP_VERTEX | LLVertexBuffer::MAP_TEXCOORD0, GL_STATIC_DRAW_ARB);
-	buff->allocateBuffer(3, 0, true);
+
+	if (!buff->allocateBuffer(3, 0, true))
+	{
+		LL_WARNS() << "Failed to allocate buffer during benchmark." << LL_ENDL;
+		// abandon the benchmark test
+		return -1.f;
+	}
 
 	LLStrider<LLVector3> v;
 	LLStrider<LLVector2> tc;
 
-	buff->getVertexStrider(v);
-	
-	v[0].set(-1,1,0);
-	v[1].set(-1,-3,0);
-	v[2].set(3,1,0);
+	if (! buff->getVertexStrider(v))
+	{
+		LL_WARNS() << "GL LLVertexBuffer::getVertexStrider() returned false, "
+				   << "buff->getMappedData() is"
+				   << (buff->getMappedData()? " not" : "")
+				   << " NULL" << LL_ENDL;
+		// abandon the benchmark test
+		return -1.f;
+	}
+
+	// generate dummy triangle
+	v[0].set(-1, 1, 0);
+	v[1].set(-1, -3, 0);
+	v[2].set(3, 1, 0);
 
 	buff->flush();
 
-	gBenchmarkProgram.bind();
-	
-	bool busted_finish = false;
+	// ensure matched pair of bind() and unbind() calls
+	class ShaderBinder
+	{
+	public:
+		ShaderBinder(LLGLSLShader& shader):
+			mShader(shader)
+		{
+			mShader.bind();
+		}
+		~ShaderBinder()
+		{
+			mShader.unbind();
+		}
+
+	private:
+		LLGLSLShader& mShader;
+	};
+	ShaderBinder binder(gBenchmarkProgram);
 
 	buff->setBuffer(LLVertexBuffer::MAP_VERTEX);
 	glFinish();
@@ -986,24 +1085,13 @@ F32 gpu_benchmark()
 		for (U32 i = 0; i < count; ++i)
 		{
 			dest[i].bindTarget();
-			gGL.getTexUnit(0)->bindManual(LLTexUnit::TT_TEXTURE, source[i]);
+			texHolder.bind(i);
 			buff->drawArrays(LLRender::TRIANGLES, 0, 3);
 			dest[i].flush();
 		}
-		
+
 		//wait for current batch of copies to finish
-		if (busted_finish)
-		{
-			//read a pixel off the last target since some drivers seem to ignore glFinish
-			dest[count-1].bindTarget();
-			U32 pixel = 0;
-			glReadPixels(0,0,1,1,GL_RGBA, GL_UNSIGNED_BYTE, &pixel);
-			dest[count-1].flush();
-		}
-		else
-		{
-			glFinish();
-		}
+		glFinish();
 
 		F32 time = timer.getElapsedTimeF32();
 
@@ -1011,30 +1099,11 @@ F32 gpu_benchmark()
 		{ 
 			//store result in gigabytes per second
 			F32 gb = (F32) ((F64) (res*res*8*count))/(1000000000);
-
 			F32 gbps = gb/time;
-
-			if (!gGLManager.mHasTimerQuery && !busted_finish && gbps > 128.f)
-			{ //unrealistically high bandwidth for a card without timer queries, glFinish is probably ignored
-				busted_finish = true;
-				LL_WARNS() << "GPU Benchmark detected GL driver with broken glFinish implementation." << LL_ENDL;
-			}
-			else
-			{
-				results.push_back(gbps);
-			}		
+			results.push_back(gbps);
 		}
 	}
 
-	gBenchmarkProgram.unbind();
-
-	if (gGLManager.mHasTimerQuery)
-	{
-		LLGLSLShader::finishProfile(false);
-	}
-
-	LLImageGL::deleteTextures(count, source);
-
 	std::sort(results.begin(), results.end());
 
 	F32 gbps = results[results.size()/2];
@@ -1046,7 +1115,7 @@ F32 gpu_benchmark()
     { 
         LL_WARNS() << "Memory bandwidth is improbably high and likely incorrect; discarding result." << LL_ENDL;
         //OSX is probably lying, discard result
-        gbps = -1.f;
+        return -1.f;
     }
 #endif