Merge pull request #2960 from phire/improve_efb2tex

Make efb2tex behave much more like efb2ram.

Source/Core/Core/FifoPlayer/FifoPlayer.cpp

@@ -19,6 +19,8 @@
 #include "Core/PowerPC/PowerPC.h"
 #include "VideoCommon/BPMemory.h"
 
+bool IsPlayingBackFifologWithBrokenEFBCopies = false;
+
 FifoPlayer::~FifoPlayer()
 {
 	delete m_File;
@@ -61,6 +63,9 @@ bool FifoPlayer::Play()
 	if (m_File->GetFrameCount() == 0)
 		return false;
 
+	// Currently these is no such thing as a Fifolog without broken EFB copies.
+	IsPlayingBackFifologWithBrokenEFBCopies = true;
+
 	m_CurrentFrame = m_FrameRangeStart;
 
 	LoadMemory();
@@ -100,6 +105,8 @@ bool FifoPlayer::Play()
 		}
 	}
 
+	IsPlayingBackFifologWithBrokenEFBCopies = false;
+
 	return true;
 }
 

Source/Core/Core/FifoPlayer/FifoPlayer.h

@@ -13,6 +13,33 @@ class FifoDataFile;
 struct MemoryUpdate;
 struct AnalyzedFrameInfo;
 
+// Story time:
+// When FifoRecorder was created, efb copies weren't really used or they used efb2tex which ignored
+// the underlying memory, so FifoRecorder didn't do anything special about the memory backing efb
+// copies. This means the memory underlying efb copies go treated like regular textures and was
+// baked into the fifo log. If you recorded with efb2ram on, the result of efb2ram would be baked
+// into the fifo. If you recorded with efb2tex or efb off, random data would be included in the fifo
+// log.
+// Later the behaviour of efb2tex was changed to zero the underlying memory and check the hash of that.
+// But this broke a whole lot of fifologs due to the following sequence of events:
+//    1. fifoplayer would trigger the efb copy
+//    2. Texture cache would zero the memory backing the texture and hash it.
+//    3. Time passes.
+//    4. fifoplayer would encounter the drawcall using the efb copy
+//    5. fifoplayer would overwrite the memory backing the efb copy back to it's state when recording.
+//    6. Texture cache would hash the memory and see that the hash no-longer matches
+//    7. Texture cache would load whatever data was now in memory as a texture either a baked in
+//       efb2ram copy from recording time or just random data.
+//    8. The output of fifoplayer would be wrong.
+
+// To keep compatibility with old fifologs, we have this flag which signals texture cache to not bother
+// hashing the memory and just assume the hash matched.
+// At a later point proper efb copy support should be added to fiforecorder and this flag will change
+// based on the version of the .dff file, but until then it will always be true when a fifolog is playing.
+
+// Shitty global to fix a shitty problem
+extern bool IsPlayingBackFifologWithBrokenEFBCopies;
+
 class FifoPlayer
 {
 public:

Source/Core/VideoBackends/D3D/TextureCache.cpp

@@ -237,10 +237,10 @@ void TextureCache::TCacheEntry::FromRenderTarget(u8* dst, unsigned int dstFormat
 
 	g_renderer->RestoreAPIState();
 
-	if (!g_ActiveConfig.bSkipEFBCopyToRam)
-	{
+	if (g_ActiveConfig.bSkipEFBCopyToRam)
+		this->Zero(dst);
+	else
 		g_encoder->Encode(dst, this, srcFormat, srcRect, isIntensity, scaleByHalf);
-	}
 }
 
 const char palette_shader[] =

Source/Core/VideoBackends/OGL/TextureCache.cpp

@@ -262,7 +262,11 @@ void TextureCache::TCacheEntry::FromRenderTarget(u8* dstPointer, unsigned int ds
 
 	glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
 
-	if (!g_ActiveConfig.bSkipEFBCopyToRam)
+	if (g_ActiveConfig.bSkipEFBCopyToRam)
+	{
+		this->Zero(dstPointer);
+	}
+	else
 	{
 		TextureConverter::EncodeToRamFromTexture(
 			dstPointer,

Source/Core/VideoCommon/TextureCacheBase.cpp

@@ -10,6 +10,7 @@
 #include "Common/StringUtil.h"
 
 #include "Core/ConfigManager.h"
+#include "Core/FifoPlayer/FifoPlayer.h"
 #include "Core/HW/Memmap.h"
 
 #include "VideoCommon/Debugger.h"
@@ -439,11 +440,10 @@ TextureCache::TCacheEntryBase* TextureCache::Load(const u32 stage)
 		TCacheEntryBase* entry = iter->second;
 		if (entry->IsEfbCopy())
 		{
-			// EFB copies have slightly different rules: the hash doesn't need to match
-			// in EFB2Tex mode, and EFB copy formats have different meanings from texture
-			// formats.
-			if (g_ActiveConfig.bSkipEFBCopyToRam ||
-				(tex_hash == entry->hash && (!isPaletteTexture || g_Config.backend_info.bSupportsPaletteConversion)))
+			// EFB copies have slightly different rules as EFB copy formats have different
+			// meanings from texture formats.
+			if ((tex_hash == entry->hash && (!isPaletteTexture || g_Config.backend_info.bSupportsPaletteConversion)) ||
+				IsPlayingBackFifologWithBrokenEFBCopies)
 			{
 				// TODO: We should check format/width/height/levels for EFB copies. Checking
 				// format is complicated because EFB copy formats don't exactly match
@@ -986,11 +986,13 @@ void TextureCache::CopyRenderTargetToTexture(u32 dstAddr, unsigned int dstFormat
 	unsigned int scaled_tex_h = g_ActiveConfig.bCopyEFBScaled ? Renderer::EFBToScaledY(tex_h) : tex_h;
 
 	// remove all texture cache entries at dstAddr
-	std::pair<TexCache::iterator, TexCache::iterator> iter_range = textures_by_address.equal_range((u64)dstAddr);
-	TexCache::iterator iter = iter_range.first;
-	while (iter != iter_range.second)
 	{
-		iter = FreeTexture(iter);
+		std::pair<TexCache::iterator, TexCache::iterator> iter_range = textures_by_address.equal_range((u64)dstAddr);
+		TexCache::iterator iter = iter_range.first;
+		while (iter != iter_range.second)
+		{
+			iter = FreeTexture(iter);
+		}
 	}
 
 	// create the texture
@@ -1012,24 +1014,20 @@ void TextureCache::CopyRenderTargetToTexture(u32 dstAddr, unsigned int dstFormat
 
 	entry->FromRenderTarget(dst, dstFormat, dstStride, srcFormat, srcRect, isIntensity, scaleByHalf, cbufid, colmat);
 
-	if (!g_ActiveConfig.bSkipEFBCopyToRam)
+	entry->hash = GetHash64(dst, (int)entry->size_in_bytes, g_ActiveConfig.iSafeTextureCache_ColorSamples);
+
+	// Invalidate all textures that overlap the range of our efb copy.
+	// Unless our efb copy has a weird stride, then we want avoid invalidating textures which
+	// we might be able to do a partial texture update on.
+	if (entry->memory_stride == entry->CacheLinesPerRow() * 32)
 	{
-		entry->hash = GetHash64(dst, (int)entry->size_in_bytes, g_ActiveConfig.iSafeTextureCache_ColorSamples);
-
-		// Invalidate all textures that overlap the range of our texture
-		TexCache::iterator
-			iter = textures_by_address.begin();
-
+		TexCache::iterator iter = textures_by_address.begin();
 		while (iter != textures_by_address.end())
 		{
 			if (iter->second->OverlapsMemoryRange(dstAddr, entry->size_in_bytes))
-			{
 				iter = FreeTexture(iter);
-			}
 			else
-			{
 				++iter;
-			}
 		}
 	}
 
@@ -1110,3 +1108,13 @@ void TextureCache::TCacheEntryBase::SetEfbCopy(u32 stride)
 
 	size_in_bytes = memory_stride * NumBlocksY();
 }
+
+// Fill gamecube memory backing this texture with zeros.
+void TextureCache::TCacheEntryBase::Zero(u8* ptr)
+{
+	for (u32 i = 0; i < NumBlocksY(); i++)
+	{
+		memset(ptr, 0, CacheLinesPerRow() * 32);
+		ptr += memory_stride;
+	}
+}

Source/Core/VideoCommon/TextureCacheBase.h

@@ -111,7 +111,7 @@ public:
 		u32 NumBlocksY() const;
 		u32 CacheLinesPerRow() const;
 
-		void Memset(u8* ptr, u32 tag);
+		void Zero(u8* ptr);
 	};
 
 	virtual ~TextureCache(); // needs virtual for DX11 dtor