Merge pull request #18001 from hrydgard/tactics-ogre-texture-replacement

Enable the FakeMipmapChange flag for US/EU Tactics Ogre, fixing replacement problem.

Common/Data/Format/IniFile.cpp

@@ -403,6 +403,17 @@ std::map<std::string, std::string> Section::ToMap() const
 	return outMap;
 }
 
+std::vector<std::pair<std::string, std::string>> Section::ToVec() const {
+	std::vector<std::pair<std::string, std::string>> outVec;
+	for (std::vector<std::string>::const_iterator iter = lines.begin(); iter != lines.end(); ++iter)
+	{
+		std::string lineKey, lineValue;
+		if (ParseLine(*iter, &lineKey, &lineValue, NULL)) {
+			outVec.push_back(std::pair<std::string, std::string>(lineKey, lineValue));
+		}
+	}
+	return outVec;
+}
 
 bool Section::Delete(const char *key)
 {

Common/Data/Format/IniFile.h

@@ -28,6 +28,7 @@ public:
 	void Clear();
 
 	std::map<std::string, std::string> ToMap() const;
+	std::vector<std::pair<std::string, std::string>> ToVec() const;  // Often more appropriate than ToMap() - doesn't artifically remove duplicates.
 
 	std::string *GetLine(const char* key, std::string* valueOut, std::string* commentOut);
 	const std::string *GetLine(const char* key, std::string* valueOut, std::string* commentOut) const;

GPU/Common/TextureCacheCommon.cpp

@@ -1114,8 +1114,8 @@ bool TextureCacheCommon::MatchFramebuffer(
 				return true;
 			}
 		} else {
-			WARN_LOG_ONCE(diffFormat2, G3D, "Ignoring possible texturing from framebuffer with incompatible format %s != %s at %08x (+%dx%d)",
-				GeTextureFormatToString(entry.format), GeBufferFormatToString(fb_format), fb_address, matchInfo->xOffset, matchInfo->yOffset);
+			WARN_LOG_ONCE(diffFormat2, G3D, "Ignoring possible texturing from framebuffer at %08x with incompatible format %s != %s (+%dx%d)",
+				fb_address, GeTextureFormatToString(entry.format), GeBufferFormatToString(fb_format), matchInfo->xOffset, matchInfo->yOffset);
 			return false;
 		}
 	}
@@ -2769,9 +2769,10 @@ bool TextureCacheCommon::PrepareBuildTexture(BuildTexturePlan &plan, TexCacheEnt
 		plan.scaleFactor = plan.scaleFactor > 4 ? 4 : (plan.scaleFactor > 2 ? 2 : 1);
 	}
 
-	bool isFakeMipmapChange = IsFakeMipmapChange();
-
+	bool isFakeMipmapChange = false;
 	if (plan.badMipSizes) {
+		isFakeMipmapChange = IsFakeMipmapChange();
+
 		// Check for pure 3D texture.
 		int tw = gstate.getTextureWidth(0);
 		int th = gstate.getTextureHeight(0);
@@ -2784,6 +2785,7 @@ bool TextureCacheCommon::PrepareBuildTexture(BuildTexturePlan &plan, TexCacheEnt
 				}
 			}
 		} else {
+			// We don't want to create a volume texture, if this is a "fake mipmap change".
 			pure3D = false;
 		}
 
@@ -2911,6 +2913,13 @@ bool TextureCacheCommon::PrepareBuildTexture(BuildTexturePlan &plan, TexCacheEnt
 	if (isFakeMipmapChange) {
 		// NOTE: Since the level is not part of the cache key, we assume it never changes.
 		plan.baseLevelSrc = std::max(0, gstate.getTexLevelOffset16() / 16);
+		// Tactics Ogre: If this is an odd level and it has the same texture address the below even level,
+		// let's just say it's the even level for the purposes of replacement.
+		// I assume this is done to avoid blending between levels accidentally?
+		// The Japanese version of Tactics Ogre uses multiple of these "double" levels to fit more characters.
+		if ((plan.baseLevelSrc & 1) && gstate.getTextureAddress(plan.baseLevelSrc) == gstate.getTextureAddress(plan.baseLevelSrc & ~1)) {
+			plan.baseLevelSrc &= ~1;
+		}
 		plan.levelsToCreate = 1;
 		plan.levelsToLoad = 1;
 		// Make sure we already decided not to do a 3D texture above.

GPU/Common/TextureReplacer.cpp

@@ -226,31 +226,50 @@ bool TextureReplacer::LoadIniValues(IniFile &ini, VFSBackend *dir, bool isOverri
 		ERROR_LOG(G3D, "Unsupported texture replacement version %d, trying anyway", version);
 	}
 
-	bool filenameWarning = false;
+	int badFileNameCount = 0;
 
 	std::map<ReplacementCacheKey, std::map<int, std::string>> filenameMap;
+	std::string badFilenames;
 
 	if (ini.HasSection("hashes")) {
-		auto hashes = ini.GetOrCreateSection("hashes")->ToMap();
+		auto hashes = ini.GetOrCreateSection("hashes")->ToVec();
 		// Format: hashname = filename.png
 		bool checkFilenames = g_Config.bSaveNewTextures && !g_Config.bIgnoreTextureFilenames && !vfsIsZip_;
 
 		for (const auto &item : hashes) {
 			ReplacementCacheKey key(0, 0);
-			int level = 0;  // sscanf might fail to pluck the level, but that's ok, we default to 0. sscanf doesn't write to non-matched outputs.
+			// sscanf might fail to pluck the level if omitted from the line, but that's ok, we default level to 0.
+			// sscanf doesn't write to non-matched outputs.
+			int level = 0;
 			if (sscanf(item.first.c_str(), "%16llx%8x_%d", &key.cachekey, &key.hash, &level) >= 1) {
+				if (item.second.empty()) {
+					WARN_LOG(G3D, "Texture replacement: Ignoring hash mapping to empty filename: '%s ='", item.first.c_str());
+					continue;
+				}
 				filenameMap[key][level] = item.second;
 				if (checkFilenames) {
+					// TODO: We should check for the union of these on all platforms, really.
 #if PPSSPP_PLATFORM(WINDOWS)
+					bool bad = item.second.find_first_of("\\ABCDEFGHIJKLMNOPQRSTUVWXYZ:<>|?*") != std::string::npos;
 					// Uppercase probably means the filenames don't match.
 					// Avoiding an actual check of the filenames to avoid performance impact.
-					filenameWarning = filenameWarning || item.second.find_first_of("\\ABCDEFGHIJKLMNOPQRSTUVWXYZ:<>|?*") != std::string::npos;
 #else
-					filenameWarning = filenameWarning || item.second.find_first_of("\\:<>|?*") != std::string::npos;
+					bool bad = item.second.find_first_of("\\:<>|?*") != std::string::npos;
 #endif
+					if (bad) {
+						badFileNameCount++;
+						if (badFileNameCount == 10) {
+							badFilenames.append("...");
+						} else if (badFileNameCount < 10) {
+							badFilenames.append(item.second);
+							badFilenames.push_back('\n');
+						}
+					}
 				}
+			} else if (item.first.empty()) {
+				INFO_LOG(G3D, "Ignoring [hashes] line with empty key: '= %s'", item.second.c_str());
 			} else {
-				ERROR_LOG(G3D, "Unsupported syntax under [hashes]: %s", item.first.c_str());
+				ERROR_LOG(G3D, "Unsupported syntax under [hashes], ignoring: %s = ", item.first.c_str());
 			}
 		}
 	}
@@ -308,9 +327,10 @@ bool TextureReplacer::LoadIniValues(IniFile &ini, VFSBackend *dir, bool isOverri
 		aliases_[pair.first] = alias;
 	}
 
-	if (filenameWarning) {
+	if (badFileNameCount > 0) {
 		auto err = GetI18NCategory(I18NCat::ERRORS);
-		g_OSD.Show(OSDType::MESSAGE_ERROR, err->T("textures.ini filenames may not be cross-platform (banned characters)"), 6.0f);
+		g_OSD.Show(OSDType::MESSAGE_WARNING, err->T("textures.ini filenames may not be cross - platform(banned characters)"), badFilenames, 6.0f);
+		WARN_LOG(G3D, "Potentially bad filenames: %s", badFilenames.c_str());
 	}
 
 	if (ini.HasSection("hashranges")) {

assets/compat.ini

@@ -265,11 +265,16 @@ ULJM06365 = true
 # This hacks separates each mipmap to independent textures to display wrong-size mipmaps.
 # For example this requires games like Tactics Ogre(Japanese) to display multi bytes fonts stored in mipmaps.
 # See issue #5350.
-# Tactics Ogre(Japanese)
+# Tactics Ogre (Japanese)
 ULJM05753 = true
 NPJH50348 = true
 ULJM06009 = true
 
+# Tactics Ogre (US, EU). See #17491 / #17980
+# Required for texture replacement to work correctly, though unlike JP, only one layer is actually used (two duplicates, really)
+ULUS10565 = true
+ULES01500 = true
+
 [RequireBufferedRendering]
 # Warn the user that the game will not work and have issue, if buffered rendering is not enabled.
 # Midnight Club: LA Remix