GSdx: Valkyrie Profile 2 fix (discussed under r5010 and r5012), this bug could have broken much more games, strange that it did not.

git-svn-id: http://pcsx2.googlecode.com/svn/trunk@5015 96395faa-99c1-11dd-bbfe-3dabce05a288

plugins/GSdx/GSRendererSW.cpp

@@ -304,7 +304,17 @@ void GSRendererSW::Sync(int reason)
 
 	m_rl->Sync();
 
-	memset(m_fzb_pages, 0, sizeof(m_fzb_pages));
+	// NOTE: m_fzb_pages is refcounted, zeroing is done automatically
+
+	#ifdef _DEBUG
+
+	for(size_t i = 0; i < countof(m_fzb_pages); i++)
+	{
+		ASSERT(m_fzb_pages[i] == 0);
+	}
+
+	#endif
+
 	memset(m_tex_pages, 0, sizeof(m_tex_pages));
 }
 
@@ -366,6 +376,8 @@ void GSRendererSW::ReleaseTargetPages(GSOffset* o, const GSVector4i& rect)
 
 	for(list<uint32>::iterator i = pages->begin(); i != pages->end(); i++)
 	{
+		ASSERT(m_fzb_pages[*i] > 0);
+
 		_InterlockedDecrement(&m_fzb_pages[*i]);
 	}
 }
@@ -374,12 +386,10 @@ void GSRendererSW::UseSourcePages(const GSTextureCacheSW::Texture* t)
 {
 	for(list<uint32>::const_iterator i = t->m_pages.n.begin(); i != t->m_pages.n.end(); i++)
 	{
-		if(m_fzb_pages[*i]) // currently being drawn to? => sync
+		if(m_fzb_pages[*i]) // currently being drawn to? => sync (could even spin and wait until it hits 0, not sure if it's worth though, or just create 512 condvars? :D)
 		{
-			//
 			Sync(7);
 
-			//
 			return;
 		}
 		
@@ -518,7 +528,7 @@ bool GSRendererSW::GetScanlineGlobalData(GSScanlineGlobalData& gd)
 
 				string s;
 
-				if(s_save && s_n >= s_saven && PRIM->TME)
+				if(s_save && s_n >= s_saven)
 				{
 					s = format("c:\\temp1\\_%05d_f%lld_tex32_%05x_%d.bmp", s_n, frame, (int)m_context->TEX0.TBP0, (int)m_context->TEX0.PSM);
 

plugins/GSdx/GSThread.h

@@ -166,7 +166,7 @@ protected:
 	int m_count;
 	queue<T> m_queue;
 	volatile bool m_exit;
-	struct {GSCritSec lock; GSEvent notempty, empty;} m_ev;
+	struct {GSCritSec lock; GSEvent notempty; volatile long count;} m_ev;
 	#ifdef _WINDOWS
 	struct {SRWLOCK lock; CONDITION_VARIABLE notempty, empty; bool available;} m_cv;
 	HMODULE m_kernel32;
@@ -203,15 +203,19 @@ protected:
 					if(m_exit) {pReleaseSRWLockExclusive(&m_cv.lock); return;}
 				}
 
-				T item = m_queue.front();
+				{
+					// NOTE: this is scoped because we must make sure the last "item" is no longer around when Wait detects an empty queue
 
-				pReleaseSRWLockExclusive(&m_cv.lock);
+					T item = m_queue.front();
 
-				Process(item);
+					pReleaseSRWLockExclusive(&m_cv.lock);
 
-				pAcquireSRWLockExclusive(&m_cv.lock);
+					Process(item);
 
-				m_queue.pop();
+					pAcquireSRWLockExclusive(&m_cv.lock);
+
+					m_queue.pop();
+				}
 
 				if(m_queue.empty())
 				{
@@ -224,28 +228,36 @@ protected:
 
 		#endif
 
-			while(m_ev.notempty.Wait())
+			m_ev.lock.Lock();
+
+			while(true)
 			{
-				if(m_exit) break;
-
-				while(!m_queue.empty())
+				while(m_queue.empty())
 				{
-					T item;
+					m_ev.lock.Unlock();
 
-					{
-						GSAutoLock l(&m_ev.lock);
+					m_ev.notempty.Wait();
 
-						item = m_queue.front();
-					}
+					if(m_exit) {return;}
+
+					m_ev.lock.Lock();
+				}
+
+				{
+					// NOTE: this is scoped because we must make sure the last item is no longer around when Wait detects an empty queue
+
+					T item = m_queue.front();
+
+					m_ev.lock.Unlock();
 
 					Process(item);
 
-					{
-						GSAutoLock l(&m_ev.lock);
+					m_ev.lock.Lock();
 
-						m_queue.pop();
-					}
+					m_queue.pop();
 				}
+
+				_InterlockedDecrement(&m_ev.count);
 			}
 
 		#ifdef _WINDOWS
@@ -260,6 +272,8 @@ public:
 		: m_count(0)
 		, m_exit(false)
 	{
+		m_ev.count = 0;
+
 		#ifdef _WINDOWS
 
 		m_cv.available = false;
@@ -345,6 +359,8 @@ public:
 
 			m_queue.push(item);
 
+			_InterlockedIncrement(&m_ev.count);
+
 			m_ev.notempty.Set();
 
 		#ifdef _WINDOWS
@@ -376,7 +392,9 @@ public:
 
 		#endif
 			
-			while(!m_queue.empty()) _mm_pause();
+			// NOTE: it is the safest to have our own counter because m_queue.pop() might decrement its own before the last item runs out of its scope and gets destroyed (implementation dependent)
+
+			while(m_ev.count > 0) _mm_pause();
 
 		#ifdef _WINDOWS