-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable the FakeMipmapChange flag for US/EU Tactics Ogre, fixing replacement problem. #18001
Conversation
…cement problem. For correct lookups, without our texture replacement actually supporting volume textures, we need to use this mechanism here too. The game actually uses two mipmaps, but they're identical and point to the same memory, so we treat them as a regular 2D texture instead for purposes of both texturing and replacement. This is presumably legacy from the initial Japanese version that needs to use multiple texture layers. Similarly it does in in pairs. This does actually not fully fix texture replacement for the Japanese version, unfortunately. For that we need more proper support for these weird textures in the texture replacement code - when I refactored it before for more natural handling of regular mipmapping, this kinda got lost.
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()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't really an error, at least it used to be documented as an intentional way to disable saving a texture which you've decided it's not useful to upscale (example: solid color texture.) Seems annoying to logspam it.
-[Unknown]
|
||
if (ini.HasSection("hashes")) { | ||
auto hashes = ini.GetOrCreateSection("hashes")->ToMap(); | ||
auto hashes = ini.GetOrCreateSection("hashes")->ToVec(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When will it make sense to map the same hash to two different filenames? The level is still part of the key, so this would just overwrite those duplicates silently anyway?
Not sure I understand what the intent would be for:
1234567890abcdef_0 = image1.png
1234567890abcdef_0 = image2.png
1234567890abcdef_0 = image3.png
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The map was deduplicating the empty keys which were confusing my logging, which I added because of a theory that turned out to be wrong.
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obviously just seems like a flaw in the texture replacement engine now. But I'm sure you're going to find all the games that use cube mipmaps and add this flag to each one. Good luck.
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's not ideal. I will try to rectify this flaw, but I'm trying to get a release ready and the change I'm planning is a bit too big.
Though in reality we've only seen this in three or four games.
For correct lookups, without our texture replacement actually supporting volume textures, we need to use this mechanism even in the non-JP versions.
The game actually uses two 512x512 mipmaps, but they're identical and point to the same memory, so also add a tweak to treat them as a regular 2D texture instead for purposes of replacement (and also texturing). This is presumably legacy from the initial Japanese version that needs to use multiple texture layers, in pairs, presumably to avoid mip filtering (which is kinda silly, could've just turned that off).
This does actually not fully fix texture replacement for the Japanese version, unfortunately. For that we need more proper support for these weird textures in the texture replacement code - when I refactored it before for more natural and efficient handling of regular mipmapping, this kinda got lost. I will fix that separately.
Also better logging of invalid replacement lines in the [hashes] section of textures.ini.
Fixes #17980
Fixes #17491