Make sure to properly filter glUseProgramStages chunks before removing

* This fixes a problem where removing "redundant" glUseProgramStages
  chunks actually removed other chunks that were legitimate.
* It's kind of hacky, but it's better than the other alternatives.

renderdoc/driver/gl/gl_driver.cpp

@@ -336,14 +336,34 @@ GLInitParams::GLInitParams()
 	height = 32;
 }
 
+// handling for these versions is scattered throughout the code (as relevant to enable/disable bits of serialisation
+// and set some defaults if necessary).
+// Here we list which non-current versions we support, and what changed
+const uint32_t GLInitParams::GL_OLD_VERSIONS[GLInitParams::GL_NUM_SUPPORTED_OLD_VERSIONS] = {
+	0x000010, // from 0x10 to 0x11, we added a dummy marker value used to identify serialised data in glUseProgramStages (hack :( )
+};
+
 ReplayCreateStatus GLInitParams::Serialise()
 {
 	SERIALISE_ELEMENT(uint32_t, ver, GL_SERIALISE_VERSION); SerialiseVersion = ver;
-
+	
 	if(ver != GL_SERIALISE_VERSION)
 	{
-		RDCERR("Incompatible OpenGL serialise version, expected %d got %d", GL_SERIALISE_VERSION, ver);
-		return eReplayCreate_APIIncompatibleVersion;
+		bool oldsupported = false;
+		for(uint32_t i=0; i < GL_NUM_SUPPORTED_OLD_VERSIONS; i++)
+		{
+			if(ver == GL_OLD_VERSIONS[i])
+			{
+				oldsupported = true;
+				RDCWARN("Old OpenGL serialise version %d, latest is %d. Loading with possibly degraded features/support.", ver, GL_SERIALISE_VERSION);
+			}
+		}
+
+		if(!oldsupported)
+		{
+			RDCERR("Incompatible OpenGL serialise version, expected %d got %d", GL_SERIALISE_VERSION, ver);
+			return eReplayCreate_APIIncompatibleVersion;
+		}
 	}
 	
 	m_pSerialiser->Serialise("Color bits", colorBits);
@@ -801,6 +821,8 @@ void WrappedOpenGL::Initialise(GLInitParams &params)
 	// deliberately want to go through our own wrappers to set up e.g. m_Textures members
 	WrappedOpenGL &gl = *this;
 
+	m_InitParams = params;
+
 	// as a concession to compatibility, generate a 'fake' VBO to act as VBO 0.
 	// consider making it an error/warning for programs to use this?
 	gl.glGenVertexArrays(1, &m_FakeVAO);

renderdoc/driver/gl/gl_driver.h

@@ -58,7 +58,11 @@ struct GLInitParams : public RDCInitParams
 	uint32_t width;
 	uint32_t height;
 	
-	static const uint32_t GL_SERIALISE_VERSION = 0x0000010;
+	static const uint32_t GL_SERIALISE_VERSION = 0x0000011;
+	
+	// backwards compatibility for old logs described at the declaration of this array
+	static const uint32_t GL_NUM_SUPPORTED_OLD_VERSIONS = 1;
+	static const uint32_t GL_OLD_VERSIONS[GL_NUM_SUPPORTED_OLD_VERSIONS];
 
 	// version number internal to opengl stream
 	uint32_t SerialiseVersion;
@@ -313,6 +317,7 @@ class WrappedOpenGL : public IFrameCapturer
 		//GLRenderState *m_CurrentPipelineState;
 		
 		Serialiser *GetSerialiser() { return m_pSerialiser; }
+		uint32_t GetLogVersion() { return m_InitParams.SerialiseVersion; }
 
 		void ProcessChunk(uint64_t offset, GLChunkType context);
 		void ContextReplayLog(LogState readType, uint32_t startEventID, uint32_t endEventID, bool partial);

renderdoc/driver/gl/gl_replay_linux.cpp

@@ -238,12 +238,10 @@ ReplayCreateStatus GL_CreateReplayDevice(const char *logfile, IReplayDriver **dr
 	RDCDriver driverType = RDC_OpenGL;
 	string driverName = "OpenGL";
 	if(logfile)
-		RenderDoc::Inst().FillInitParams(logfile, driverType, driverName, (RDCInitParams *)&initParams);
-
-	if(initParams.SerialiseVersion != GLInitParams::GL_SERIALISE_VERSION)
 	{
-		RDCERR("Incompatible OpenGL serialise version, expected %d got %d", GLInitParams::GL_SERIALISE_VERSION, initParams.SerialiseVersion);
-		return eReplayCreate_APIIncompatibleVersion;
+		auto status = RenderDoc::Inst().FillInitParams(logfile, driverType, driverName, (RDCInitParams *)&initParams);
+		if(status != eReplayCreate_Success)
+			return status;
 	}
 
 	int attribs[64] = {0};

renderdoc/driver/gl/gl_replay_win32.cpp

@@ -249,12 +249,10 @@ ReplayCreateStatus GL_CreateReplayDevice(const char *logfile, IReplayDriver **dr
 	RDCDriver driverType = RDC_OpenGL;
 	string driverName = "OpenGL";
 	if(logfile)
-		RenderDoc::Inst().FillInitParams(logfile, driverType, driverName, (RDCInitParams *)&initParams);
-
-	if(initParams.SerialiseVersion != GLInitParams::GL_SERIALISE_VERSION)
 	{
-		RDCERR("Incompatible OpenGL serialise version, expected %d got %d", GLInitParams::GL_SERIALISE_VERSION, initParams.SerialiseVersion);
-		return eReplayCreate_APIIncompatibleVersion;
+		auto status = RenderDoc::Inst().FillInitParams(logfile, driverType, driverName, (RDCInitParams *)&initParams);
+		if(status != eReplayCreate_Success)
+			return status;
 	}
 	
 	PIXELFORMATDESCRIPTOR pfd = { 0 };

renderdoc/driver/gl/gl_resources.h

@@ -170,6 +170,24 @@ struct GLResourceRecord : public ResourceRecord
 		int64_t persistentMaps; // counter indicating how many coherent maps are 'live'
 	} Map;
 
+	template<typename ChunkFilter>
+	void FilterChunks(ChunkFilter& filter)
+	{
+		LockChunks();
+		std::vector<std::map<int32_t, Chunk *>::iterator> deletions;
+		for(auto it=m_Chunks.begin(); it != m_Chunks.end(); ++it)
+		{
+			if(filter(it->second))
+				deletions.push_back(it);
+		}
+		for(size_t i=0; i < deletions.size(); i++)
+		{
+			SAFE_DELETE(deletions[i]->second);
+			m_Chunks.erase(deletions[i]);
+		}
+		UnlockChunks();
+	}
+
 	void VerifyDataType(GLenum target)
 	{
 #if !defined(RELEASE)

renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp

@@ -934,8 +934,15 @@ void WrappedOpenGL::glProgramBinary(GLuint program, GLenum binaryFormat, const v
 
 #pragma region Program Pipelines
 
+static const uint64_t marker_glUseProgramStages_hack = 0xffbbcc0014151617ULL;
+
 bool WrappedOpenGL::Serialise_glUseProgramStages(GLuint pipeline, GLbitfield stages, GLuint program)
 {
+	if(GetLogVersion() >= 0x000011)
+	{
+		// this marker value is used below to identify where the serialised data sits.
+		SERIALISE_ELEMENT(uint64_t, marker, marker_glUseProgramStages_hack);
+	}
 	SERIALISE_ELEMENT(ResourceId, pipe, GetResourceManager()->GetID(ProgramPipeRes(GetCtx(), pipeline)));
 	SERIALISE_ELEMENT(uint32_t, Stages, stages);
 	SERIALISE_ELEMENT(ResourceId, prog, (program ? GetResourceManager()->GetID(ProgramRes(GetCtx(), program)) : ResourceId()));
@@ -1005,33 +1012,74 @@ void WrappedOpenGL::glUseProgramStages(GLuint pipeline, GLbitfield stages, GLuin
 		GLResourceRecord *record = GetResourceManager()->GetResourceRecord(ProgramPipeRes(GetCtx(), pipeline));
 		RDCASSERT(record);
 
+		Chunk *chunk = scope.Get();
+
 		if(m_State == WRITING_CAPFRAME)
 		{
-			m_ContextRecord->AddChunk(scope.Get());
+			m_ContextRecord->AddChunk(chunk);
 		}
 		else
 		{
 			// USE_PROGRAMSTAGES is one of the few kinds of chunk that are
 			// recorded to pipeline records, so we can probably find previous
 			// uses (if it's been constantly rebound instead of once at init
-			// time) that can be popped as redundant
-			record->LockChunks();
-			while(true)
+			// time) that can be popped as redundant.
+			// We do have to be careful though to make sure we only remove
+			// redundant calls, not other different USE_PROGRAMSTAGES calls!
+			struct FilterChunkClass
 			{
-				Chunk *end = record->GetLastChunk();
+				FilterChunkClass(uint32_t s) : stages(s) {}
+				uint32_t stages;
 
-				if(end->GetChunkType() == USE_PROGRAMSTAGES)
+				// this is kind of a hack, but it would be really awkward
+				// to make a general solution just for this one case, and
+				// we also can't really afford to drop it entirely.
+				// we search for the marker serialised above, skip over the
+				// pipeline id (as it will be the same in all chunks in this
+				// record), and check if the Stages bitfield afterwards is
+				// the same - if so we remove that chunk as replaced by
+				// this one
+				bool operator () (Chunk *c)
 				{
-					SAFE_DELETE(end);
-					record->PopChunk();
-					continue;
+					if(c->GetChunkType() != USE_PROGRAMSTAGES)
+						return false;
+
+					byte *b = c->GetData();
+					byte *end = b + c->GetLength();
+
+					// 'fast' path, rather than searching byte-by-byte from
+					// the start to be safe, check the exact difference it should
+					// always be first.
+					if( *(uint64_t *)(b+6) == marker_glUseProgramStages_hack)
+						b += 6;
+
+					while(b + sizeof(uint64_t) < end)
+					{
+						uint64_t *marker = (uint64_t *)b;
+						if(*marker == marker_glUseProgramStages_hack)
+						{
+							// increment to point to pipeline id
+							marker++;
+							// increment to point to stages field
+							marker++;
+
+							// now compare
+							uint32_t *chunkStages = (uint32_t *)marker;
+
+							if(*chunkStages == stages)
+								return true;
+							return false;
+						}
+
+						b++;
+					}
+					RDCERR("Didn't find marker value! This should not happen, check Serialise_glUseProgramStages serialisation");
+					return false;
 				}
+			};
+			record->FilterChunks(FilterChunkClass(stages));
 
-				break;
-			}
-			record->UnlockChunks();
-
-			record->AddChunk(scope.Get());
+			record->AddChunk(chunk);
 		}
 
 		if(program)