TextureReplacer: Fix stale and shared lookup results

- Reloading the ini clears the per-key lookup caches, which could keep
  saying "no replacement" for textures the new ini replaces.
- With ignoreAddress, hash ranges were skipped when sizing the
  replacement, though ComputeHash applies them. cache_ is now keyed by
  the full key, so its lookups hit too.
- Textures sharing files but differing in size, hash range or filtering
  no longer share one ReplacedTexture (the first one's settings won).

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

GPU/Common/ReplacedTexture.h

@@ -119,7 +119,7 @@ class ReplacedTexture;
 // replacement (texture == nullptr).
 struct ReplacedTextureRef {
 	ReplacedTexture *texture;  // shortcut
-	std::string hashfiles;  // key into the cache
+	std::string hashfiles;  // key into levelCache_
 };
 
 // Metadata about a given texture level.

GPU/Common/TextureReplacer.cpp

@@ -147,6 +147,10 @@ bool TextureReplacer::LoadIni(std::string *error, bool notify) {
 	hashranges_.clear();
 	filtering_.clear();
 	reducehashranges_.clear();
+	// These hold what the old ini said about each texture, including "no replacement" markers.
+	// Only references go, the textures stay in levelCache_.
+	cache_.clear();
+	savedCache_.clear();
 
 	ignoreAddress_ = false;
 	reduceHash_ = false;
@@ -670,15 +674,18 @@ ReplacedTexture *TextureReplacer::FindReplacement(ReplacementCacheKey replacemen
 	desc.cacheKey = replacementKey;
 	desc.forceFiltering = (TextureFiltering)0;  // invalid value
 
+	// Hash ranges are per address even with ignoreAddress, since ComputeHash applies them.
+	LookupHashRange(replacementKey.Address(), w, h, &desc.newW, &desc.newH);
+
+	// cache_ stays keyed by the full key. ignoreAddress only affects finding the files.
+	ReplacementCacheKey lookupKey = replacementKey;
 	if (ignoreAddress_) {
-		replacementKey.ZeroAddress();
-	} else {
-		LookupHashRange(replacementKey.Address(), w, h, &desc.newW, &desc.newH);
+		lookupKey.ZeroAddress();
 	}
 
 	bool foundAlias = false;
 	bool ignored = false;
-	std::string hashfiles = LookupHashFile(replacementKey, &foundAlias, &ignored);
+	std::string hashfiles = LookupHashFile(lookupKey, &foundAlias, &ignored);
 
 	// Early-out for ignored textures, let's not bother even starting a thread task.
 	if (ignored) {
@@ -689,7 +696,7 @@ ReplacedTexture *TextureReplacer::FindReplacement(ReplacementCacheKey replacemen
 		return nullptr;
 	}
 
-	FindFiltering(replacementKey, &desc.forceFiltering);
+	FindFiltering(lookupKey, &desc.forceFiltering);
 
 	if (foundAlias) {
 		desc.logId = hashfiles;
@@ -707,12 +714,14 @@ ReplacedTexture *TextureReplacer::FindReplacement(ReplacementCacheKey replacemen
 	}
 
 	_dbg_assert_(!hashfiles.empty());
-	// OK, we might already have a matching texture, we use hashfiles as a key. Look it up in the level cache.
-	auto iter = levelCache_.find(hashfiles);
+	// OK, we might already have a matching texture. Textures sharing files can still differ in how
+	// they're scaled and filtered, so those go in the level cache key too.
+	std::string levelKey = StringFromFormat("%s#%dx%d>%dx%d#%d", hashfiles.c_str(), desc.w, desc.h, desc.newW, desc.newH, (int)desc.forceFiltering);
+	auto iter = levelCache_.find(levelKey);
 	if (iter != levelCache_.end()) {
 		// Insert an entry into the cache for faster lookup next time.
 		ReplacedTextureRef ref;
-		ref.hashfiles = hashfiles;
+		ref.hashfiles = levelKey;
 		ref.texture = iter->second;
 		cache_.emplace(std::make_pair(replacementKey, ref));
 		return iter->second;
@@ -725,12 +734,11 @@ ReplacedTexture *TextureReplacer::FindReplacement(ReplacementCacheKey replacemen
 	ReplacedTexture *texture = new ReplacedTexture(vfs_, desc);
 
 	ReplacedTextureRef ref;
-	ref.hashfiles = hashfiles;
+	ref.hashfiles = levelKey;
 	ref.texture = texture;
 	cache_.emplace(std::make_pair(replacementKey, ref));
 
-	// Also, insert the level in the level cache so we can look up by desc_->hashfiles again.
-	levelCache_.emplace(std::make_pair(hashfiles, texture));
+	levelCache_.emplace(std::make_pair(levelKey, texture));
 	return texture;
 }
 

unittest/TestTextureReplacer.cpp

@@ -22,6 +22,10 @@ static const u64 KEY_D = 0xA0000004C0000004ULL;  // ignored (empty filename)
 static const u32 HASH_D = 0x10000004;
 static const u64 KEY_E = 0xA0000005C0000005ULL;  // file missing
 static const u32 HASH_E = 0x10000005;
+static const u64 KEY_F = 0xA0000006C0000006ULL;  // same file as A, own hashrange
+static const u32 HASH_F = 0x10000006;
+static const u64 KEY_G = 0xA0000007C0000007ULL;  // ignoreAddress pack, hashrange
+static const u32 HASH_G = 0x10000007;
 
 static bool CreateTestPNG(const Path &filename, int w, int h, u32 color) {
 	std::vector<u8> buf((size_t)w * h * 4);
@@ -34,6 +38,16 @@ static bool CreateTestPNG(const Path &filename, int w, int h, u32 color) {
 	return pngSave(filename, buf.data(), w, h, 4);
 }
 
+static bool WriteIni(const Path &packDir, const char *iniContent) {
+	FILE *f = File::OpenCFile(packDir / "textures.ini", "w");
+	if (!f) {
+		return false;
+	}
+	fwrite(iniContent, 1, strlen(iniContent), f);
+	fclose(f);
+	return true;
+}
+
 static bool CreateTestPack(const Path &packDir) {
 	File::DeleteDirRecursively(packDir);
 	if (!File::CreateDir(packDir)) {
@@ -52,19 +66,18 @@ static bool CreateTestPack(const Path &packDir) {
 		"A0000003C000000310000003 = tex_c.png\n"
 		"A0000004C000000410000004 =\n"
 		"A0000005C000000510000005 = missing.png\n"
+		"A0000006C000000610000006 = tex_a.png\n"
 		"\n"
 		"[hashranges]\n"
 		"A0000003,512,512 = 256,256\n"
+		"A0000006,64,64 = 32,32\n"
 		"\n"
 		"[filtering]\n"
 		"A0000001C000000110000001 = nearest\n";
 
-	FILE *f = File::OpenCFile(packDir / "textures.ini", "w");
-	if (!f) {
+	if (!WriteIni(packDir, iniContent)) {
 		return false;
 	}
-	fwrite(iniContent, 1, strlen(iniContent), f);
-	fclose(f);
 
 	if (!CreateTestPNG(packDir / "tex_a.png", 64, 64, 0xFF0000FF)) return false;
 	if (!CreateTestPNG(packDir / "tex_b0.png", 64, 64, 0x00FF00FF)) return false;
@@ -94,11 +107,16 @@ static bool TestLookups(TextureReplacer *replacer) {
 	ReplacedTexture *texE = replacer->FindReplacement(ReplacementCacheKey(KEY_E, HASH_E), 16, 16);
 	EXPECT_TRUE(texE != nullptr);
 
+	// Key F: the same file as A, but its own hashrange and no filtering, so it can't share A's texture.
+	ReplacedTexture *texF = replacer->FindReplacement(ReplacementCacheKey(KEY_F, HASH_F), 64, 64);
+	EXPECT_TRUE(texF != nullptr);
+	EXPECT_TRUE(texF != texA);
+
 	// Unknown key: not in the ini at all.
 	ReplacementCacheKey unknownKey(0x2000000020000000ULL, 0x20000000);
 	EXPECT_TRUE(replacer->FindReplacement(unknownKey, 16, 16) == nullptr);
 
-	if (!texA || !texB || !texC || !texE) {
+	if (!texA || !texB || !texC || !texE || !texF) {
 		return false;
 	}
 
@@ -133,6 +151,13 @@ static bool TestLookups(TextureReplacer *replacer) {
 	EXPECT_EQ_INT(w, 512);
 	EXPECT_EQ_INT(h, 512);
 
+	// Key F: hashrange maps 64x64 -> 32x32, so the 64x64 image covers 128x128.
+	EXPECT_TRUE(texF->Poll(1.0));
+	texF->GetSize(0, &w, &h);
+	EXPECT_EQ_INT(w, 128);
+	EXPECT_EQ_INT(h, 128);
+	EXPECT_FALSE(texF->ForceFiltering(&filtering));
+
 	// Key E: missing file should end up NOT_FOUND.
 	EXPECT_TRUE(texE->Poll(1.0));
 	EXPECT_TRUE(texE->State() == ReplacementState::NOT_FOUND);
@@ -141,6 +166,47 @@ static bool TestLookups(TextureReplacer *replacer) {
 	return true;
 }
 
+// ignoreAddress matches files without the address, but hash ranges still apply per address.
+static bool TestIgnoreAddress(const Path &packDir) {
+	static const char *iniContent =
+		"[options]\n"
+		"hash = xxh32\n"
+		"ignoreAddress = true\n"
+		"version = 1\n"
+		"\n"
+		"[hashes]\n"
+		"00000000C000000710000007 = tex_c.png\n"
+		"\n"
+		"[hashranges]\n"
+		"A0000007,512,512 = 256,256\n";
+	if (!WriteIni(packDir, iniContent)) {
+		return false;
+	}
+
+	TextureReplacer replacer(nullptr);
+	std::string error;
+	if (!replacer.LoadPackForTesting(packDir, &error)) {
+		return false;
+	}
+
+	ReplacementCacheKey key(KEY_G, HASH_G);
+	ReplacedTexture *texG = replacer.FindReplacement(key, 512, 512);
+	EXPECT_TRUE(texG != nullptr);
+	if (!texG) {
+		return false;
+	}
+	EXPECT_TRUE(replacer.FindReplacement(key, 512, 512) == texG);
+
+	g_threadManager.Init(1, 1);
+	EXPECT_TRUE(texG->Poll(1.0));
+	int w = 0, h = 0;
+	texG->GetSize(0, &w, &h);
+	EXPECT_EQ_INT(w, 512);
+	EXPECT_EQ_INT(h, 512);
+	g_threadManager.Teardown();
+	return true;
+}
+
 bool TestTextureReplacer() {
 	Path packDir = Path("unittest_texture_pack");
 	if (!CreateTestPack(packDir)) {
@@ -160,6 +226,16 @@ bool TestTextureReplacer() {
 		return false;
 	}
 
+	// Reloading the ini forgets what the old one said about each texture.
+	EXPECT_TRUE(replacer.GetNumTrackedTextures() != 0);
+	EXPECT_TRUE(replacer.LoadPackForTesting(packDir, &error));
+	EXPECT_EQ_INT(replacer.GetNumTrackedTextures(), 0);
+
+	if (!TestIgnoreAddress(packDir)) {
+		File::DeleteDirRecursively(packDir);
+		return false;
+	}
+
 	File::DeleteDirRecursively(packDir);
 	return true;
 }