Skip to content
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

Merged
merged 5 commits into from
Aug 29, 2023

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Aug 28, 2023

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

…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.
@hrydgard hrydgard added this to the v1.16.0 milestone Aug 28, 2023
@hrydgard hrydgard merged commit 64d0478 into master Aug 29, 2023
18 checks passed
@hrydgard hrydgard deleted the tactics-ogre-texture-replacement branch August 29, 2023 09:27
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());
Copy link
Collaborator

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();
Copy link
Collaborator

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]

Copy link
Owner Author

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)
Copy link
Collaborator

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]

Copy link
Owner Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants