Fixed an issue in ProfilerOverlay where element removal wasn't working right and would cause a crash

Significantly reduced amount of allocations in HString

BansheeEngine/Source/BsProfilerOverlay.cpp

@@ -32,8 +32,8 @@ namespace BansheeEngine
 			UINT32 excessEntries = (UINT32)rows.size() - curIdx;
 			for(UINT32 i = 0; i < excessEntries; i++)
 			{
-				labelLayout.removeChildAt(labelLayout.getNumChildren() - i - 1); // -1 because last element is flexible space and we want to skip it
-				contentLayout.removeChildAt(contentLayout.getNumChildren() - i - 1); // -1 because last element is flexible space and we want to skip it
+				labelLayout.removeChildAt(labelLayout.getNumChildren() - 2); // -2 because last element is flexible space and we want to skip it
+				contentLayout.removeChildAt(contentLayout.getNumChildren() - 2); // -2 because last element is flexible space and we want to skip it
 
 				ProfilerOverlay::BasicRow& row = rows[curIdx + i];
 

CamelotUtility/Include/CmHString.h

@@ -32,6 +32,7 @@ namespace CamelotFramework
 
 			mutable bool mIsDirty;
 			mutable WString mCachedString;
+			mutable WString* mStringPtr;
 
 			void updateString();
 		};

CamelotUtility/Include/CmStringTable.h

@@ -196,7 +196,7 @@ namespace CamelotFramework
 		Zhuang,
 		Chinese,
 		Zulu,
-		Count // Not a language, just a quick way to know total number of languages
+		Count // Number of entries
 	};
 
 	struct LocalizedStringData
@@ -230,7 +230,7 @@ namespace CamelotFramework
 
 		Common* commonData;
 
-		WString concatenateString(WString* parameters, UINT32 numParameterValues) const;
+		void concatenateString(WString& outputString, WString* parameters, UINT32 numParameterValues) const;
 		void updateString(const WString& string);
 	};
 

CamelotUtility/Source/CmHString.cpp

@@ -5,7 +5,7 @@
 namespace CamelotFramework
 {
 	HString::StringData::StringData()
-		:mParameters(nullptr), mIsDirty(true)
+		:mParameters(nullptr), mIsDirty(true), mStringPtr(nullptr)
 	{ }
 
 	HString::StringData::~StringData()
@@ -38,7 +38,10 @@ namespace CamelotFramework
 		mData = cm_shared_ptr<StringData>();
 
 		mData->mStringData = &StringTable::instance().getStringData(L"");
-		mData->mParameters = cm_newN<WString>(mData->mStringData->numParameters);
+
+		if(mData->mStringData->numParameters > 0)
+			mData->mParameters = cm_newN<WString>(mData->mStringData->numParameters);
+
 		mData->mUpdateConn = mData->mStringData->commonData->onStringDataModified.connect(boost::bind(&HString::StringData::updateString, mData.get()));
 	}
 
@@ -47,7 +50,10 @@ namespace CamelotFramework
 		mData = cm_shared_ptr<StringData>();
 
 		mData->mStringData = &StringTable::instance().getStringData(identifierString);
-		mData->mParameters = cm_newN<WString>(mData->mStringData->numParameters);
+
+		if(mData->mStringData->numParameters > 0)
+			mData->mParameters = cm_newN<WString>(mData->mStringData->numParameters);
+
 		mData->mUpdateConn = mData->mStringData->commonData->onStringDataModified.connect(boost::bind(&HString::StringData::updateString, mData.get()));
 	}
 
@@ -65,11 +71,20 @@ namespace CamelotFramework
 	{ 
 		if(mData->mIsDirty)
 		{
-			mData->mCachedString = mData->mStringData->concatenateString(mData->mParameters, mData->mStringData->numParameters);
+			if(mData->mParameters != nullptr)
+			{
+				mData->mStringData->concatenateString(mData->mCachedString, mData->mParameters, mData->mStringData->numParameters);
+				mData->mStringPtr = &mData->mCachedString;
+			}
+			else
+			{
+				mData->mStringPtr = &mData->mStringData->string;
+			}
+
 			mData->mIsDirty = false;
 		}
 
-		return mData->mCachedString; 
+		return *mData->mStringPtr; 
 	}
 
 	void HString::setParameter(UINT32 idx, const WString& value)

CamelotUtility/Source/CmStringTable.cpp

@@ -17,28 +17,50 @@ namespace CamelotFramework
 			cm_deleteN(parameterOffsets, numParameters);
 	}
 
-	WString LocalizedStringData::concatenateString(WString* parameters, UINT32 numParameterValues) const
+	void LocalizedStringData::concatenateString(WString& outputString, WString* parameters, UINT32 numParameterValues) const
 	{
-		WStringStream fullString;
-		UINT32 prevIdx = 0;
-
 		// A safeguard in case translated strings have different number of parameters
 		UINT32 actualNumParameters = std::min(numParameterValues, numParameters);
-
+		
 		if(parameters != nullptr)
 		{
+			UINT32 totalNumChars = 0;
+			UINT32 prevIdx = 0;
 			for(UINT32 i = 0; i < actualNumParameters; i++)
 			{
-				fullString<<string.substr(prevIdx, parameterOffsets[i].location - prevIdx);
-				fullString<<parameters[parameterOffsets[i].paramIdx];
+				totalNumChars += (parameterOffsets[i].location - prevIdx) + (UINT32)parameters[parameterOffsets[i].paramIdx].size();;
 
 				prevIdx = parameterOffsets[i].location;
 			}
-		}
 
-		fullString<<string.substr(prevIdx, string.size() - prevIdx);
+			totalNumChars += (UINT32)string.size() - prevIdx;
+
+			outputString.resize(totalNumChars);
+			wchar_t* strData = &outputString[0]; // String contiguity required by C++11, but this should work elsewhere as well
+
+			prevIdx = 0;
+			for(UINT32 i = 0; i < actualNumParameters; i++)
+			{
+				UINT32 strSize = parameterOffsets[i].location - prevIdx;
+				memcpy(strData, &string[prevIdx], strSize * sizeof(wchar_t));
+				strData += strSize;
+
+				WString& param = parameters[parameterOffsets[i].paramIdx];
+				memcpy(strData, &param[0], param.size() * sizeof(wchar_t));
+				strData += param.size();
 
-		return fullString.str();
+				prevIdx = parameterOffsets[i].location;
+			}
+
+			memcpy(strData, &string[prevIdx], (string.size() - prevIdx) * sizeof(wchar_t));
+		}
+		else
+		{
+			outputString.resize(string.size());
+			wchar_t* strData = &outputString[0]; // String contiguity required by C++11, but this should work elsewhere as well
+
+			memcpy(strData, &string[0], string.size() * sizeof(wchar_t));
+		}
 	}
 
 	void LocalizedStringData::updateString(const WString& _string)

TODO.txt

@@ -11,33 +11,7 @@ Optimization notes:
  - submitToCoreThread calls are EXTREMELY slow. In 10-50ms range.
  - GUIManager updateMeshes seems to be executing every frame
 
- ProfilerOverlay causes a crash when too many elements are present
-  - For a test case, try adding 4-5 begin/endSamples to TextUtility::getTextData
-
-WEIRDNESS:
-In GUIManager in 2 places I compare stuff by address. Make sure to use mesh or material address instead of material group address is isn't completely random
-A lot of stuff is still using GSyncedMainCA but it should be using gMainCA
-
-
-My idea of what is happening:
- Update mesh commands for GUI elements get submitted on sync CA
- Render commands for GUI elements gets submitted non-sync CA
- Update mesh commands are executed before non-sync CA, old render commands now containing wrong mesh/material combinations
-
-
- Temporarily disabled:
- ProfilerOverlay
- Helper windows
- TestTextSprite
- DebugAABoxDraw
- Main menu items
- Window frame
-
-Added a couple of debug logs in GUIManager::render
-
-
-
-
+A lot of stuff is still using GSyncedMainCA but it should be using gMainCA - Find and replace
 
 TODO: Viewport can be modified from the sim thread, but is used on the core thread without any syncronization mechanisms. Maybe add a method that returns VIEWPORT_DATA, and have that used on the core thread.
 

TextOpts.txt

@@ -1,13 +1,5 @@
 Make sure to also update TextSprite and ImageSprite and anything else in UpdateMesh, then don't forget to find the issue that causes elements to get marked as dirty every single frame
 
-Profiler does a lot of allocations of its own which can seriously skew the profiling results. Try to eliminate them all or add alloc/free overhead fields
-
 When optimizing UpdateLayout make sure to mark elements that are fully culled as Culled
  - But in order to determine that I first need to update the sprite to find out the elements bounds which defeats the point
  - TODO - FIgure this out
-
-TODO: TextData does an extra mem alloc in this bit of code: PageBuffer[NextFreePageInfo].texture = HTexture();, try to avoid it
-
-Performance stats:
- Pre-opt: 1500 allocs, 600 frees, 0.5ms
- Post-first-stage-opt: 408 alloc, 136 frees, 0.15ms