VideoCommon: migrate texture packs to use the asset loader system

[iwubcode]

This moves high resolution textures to be loaded as assets by the asset loader system. This makes the high resolution textures load in an asynchronous manner and also allows for them to be reloaded automatically:

High resolution textures did have some particular behaviors that didn't fit very well in the asset loader system, I've listed them below.

Change: The OSD messages about all textures being loaded was removed. Response: I know that this message is useful for debugging. This doesn't make sense for the asset loader though because an asset can be loaded at any time (not just on start up). My idea is to add an imgui window that lists a count of every asset type loaded. I haven't done this yet. If this is a blocker, I can implement it in a separate review before merging this.

Change: High resolution textures no longer block the game when they need to load. This eliminates stutter at the cost of textures "popping in". Response: On a higher fidelity machine, this doesn't seem distracting and all textures seem to be loaded just as quickly as on master. At least one or two users I've spoken with said they'd prefer pop-in over stutter if given the choice. And at least one other emulator offers this option (while also mentioning this is the preferred approach) so moving to this wouldn't surprise users.

Change: Previously, if the high resolution textures exceeded the memory limit Dolphin calculated, high resolution textures would be turned off completely and aborted. Now the system will just reject future assets, logging that the memory cap has been reached. Response: I think the previous solution was a poor decision and think that users which are memory capped would rather see some of the textures then none at all. In the future a smarter memory management system would possibly alleviate this.

Change: Previously, mip textures were more flexible and could be anywhere in the texture pack. Response: This made sense given the solution but is now difficult to do. I could introduce a separate library system just for high-resolution textures but in practice most packs I've seen keep their mip textures next to the main textures or are using DDS textures with the mips included. Most packs seem to use the same file type for the mip as they do the main texture too. The odd packs that don't should be updated to one of the aforementioned solutions.

---

TODO:

• [x] Support the "_mip" textures
• [x] Cleanup DynamicInputTexture logic to no longer force the texture cache to reload when input changes occur

[MayImilae]

High resolution textures no longer block the game when they need to load. This eliminates stutter at the cost of textures "popping in". On a higher fidelity machine, this doesn't seem distracting and all textures seem to be loaded just as quickly as on master

That is a significant behavior change. I assume this can still be avoided by preloading?

[iwubcode]

That is a significant behavior change. I assume this can still be avoided by preloading?

Yes, preloading is still supported.

[iwubcode]

Converted to draft until the other one is merged (confirmed behavior is still working with those changes in)

[AdmiralCurtiss]

Okay, this is a pretty unfamiliar corner of the code for me, so let me know if I got the logic right.

• On game boot (and when a high-res texture config option is changed), HiresTexture::Update() is called.
  • This enumerates all textures that exist in the ./Load/Textures subfolders matching the currently running game and 'registers' them in the s_file_library and in s_hires_texture_id_to_arbmipmap,
  • and possibly preloads it (by creating a HiresTexture and placing it in s_hires_texture_cache).
• When a game then encounters a texture, HiresTexture::Search() is called for each texture.
  • This first checks the s_hires_texture_cache for a matching texture and if yes returns it,
  • otherwise it checks the s_hires_texture_id_to_arbmipmap for if there is a registered non-loaded texture with that name and if yes loads it (by creating a HiresTexture and placing it in s_hires_texture_cache).

Both of these cases don't actually load the texture into memory yet, but only create an unfilled Asset object that will then be asynchonously filled by a background thread.

Okay, I think that's right so far. But then I'm not entirely sure what happens next. Whenever a texture is then rendered by the game, TextureCacheBase::Load() is called and that does... what, exactly? I'm not really sure how the dynamic reloading works correctly at all. ~~The m_last_write_time of the linked asset is never set to anything, so the last_asset_write_time > entry.linked_asset.m_last_write_time check always returns true, so the asset is reloaded every single time it's rendered? Surely that's not the intent, right?~~ Wait, it is set with that entry->linked_asset = std::move(cached_texture_asset), missed that one. But even still, this check looks pretty race-y -- the m_last_loaded_time on the CustomAsset is set from a the background loading thread and is not protected by anything. Something is definitely wrong here.

~~Also where exactly does it detect when a file on disk changes? I feel like I'm missing that part completely.~~ Ah I think I found it, it's in CustomAssetLoader::Init().

---

But anyway, that aside, assuming the reloading did work correctly, that still means:

• Regardless of if you prefetch or not,
  • Any new texture that did not exist when HiresTexture::Update() was called will not be detected and loaded until that function is called again.
    • Is this desired? Should there be an option for force-rescanning the texture directories for when you add a new texture while the game is running?
  • But, all textures that did already exist will be automatically updated when the file on disk changes.
  • You also incur a slight delay on boot even without prefetching as the files are enumerated. (Probably negligible, but worth noting.)

One other thing I noticed: Does the wildcard feature actually work properly when not prefetching here? I don't think it does; not only are cached wildcard textures preferred to uncached non-wildcard textures, the s_hires_texture_id_to_arbmipmap.find(full_name) check will never match any wildcarded texture name because full_name does not contain wildcards.

You also never clear s_hires_texture_id_to_arbmipmap anywhere.

[AdmiralCurtiss]

fwiw I think if you just make CustomAsset::m_last_loaded_time an std::atomic<TimeType> then the threading logic around that should be okay, assuming I understand the sequencing guarantees about that correctly.

[iwubcode]

First of all, thank you so much Admiral for reviewing this! Especially when it's not your usual stomping grounds :)

Okay, I think that's right so far

Yes, you are!

Whenever a texture is then rendered by the game, TextureCacheBase::Load() is called and that does... what, exactly?

If the texture doesn't exist in cache, it will create a texture. As part of this creation process, if high resolution textures are enabled, we try and load an asset as a "linked_asset", which is both the asset and a cached load time.

I'm not really sure how the dynamic reloading works correctly at all.

Next time the game tries to access that asset, we check to see if our cached time is older than our asset's load time and if so we reload the asset. There was a bug here...which I'll talk about after all this.

But even still, this check looks pretty race-y -- the m_last_loaded_time on the CustomAsset is set from a the background loading thread and is not protected by anything. Something is definitely wrong here.

Yes, this was a great catch. You're right I could have used a atomic. I opted for a lock instead but honestly never know when I should prefer one over the other. In looking over your statement and viewing the code, I found a few places that could explode in the right conditions. ~~I opened #11891 with a number of changes.~~

Any new texture that did not exist when HiresTexture::Update() was called will not be detected and loaded until that function is called again. Is this desired? Should there be an option for force-rescanning the texture directories for when you add a new texture while the game is running?

This is actually an "issue" in the current logic too. From my understanding, most people will just check and uncheck the "Load Textures" box to force a refresh. It is klunky but users are used to it. I was planning on reworking this with a mythical editor.

But, all textures that did already exist will be automatically updated when the file on disk changes.

Yes.

You also incur a slight delay on boot even without prefetching as the files are enumerated. (Probably negligible, but worth noting.)

That is a good point, hmm.

One other thing I noticed: Does the wildcard feature actually work properly when not prefetching here? I don't think it does; not only are cached wildcard textures preferred to uncached non-wildcard textures, the s_hires_texture_id_to_arbmipmap.find(full_name) check will never match any wildcarded texture name because full_name does not contain wildcards.

Wildcard worked with prefetch it was broken without it. I fixed that. Thank you for catching that!

You also never clear s_hires_texture_id_to_arbmipmap anywhere.

Thank you for that as well! Updated.

[iwubcode]

Ok, now to the bug. I noticed that the original code wrapped the cache calls with this:

if (g_ActiveConfig.bCacheHiresTextures)

I was only doing that in the initial load. This bCacheHiresTextures is a bit of a poorly named variable, it actually is whether you've turned on prefetching. Should we not be caching when prefetching is off? Seems odd to me but that's what the original code had so...

Regardless, once I put that in, I noticed none of my textures were loading with prefetch off. Why? Well first was that I was destroying the last reference to the asset (because the asset wasn't stored in the cache anymore!) before loading the new texture which caused things to reset. But even after fixing that, I still saw the issue. Turns out the game I was testing was leveraging the tmem caching!

I now provide a force_reload flag to force the reloading when the asset associated with a texture changes.

[AdmiralCurtiss]

Okay, tested this a bit. Seems to work fine but I found a crash when a texture is renamed/removed while the game is running -- you're using the throwing std::filesystem variants.

[AdmiralCurtiss]

Oh, that piece of code was in another PR... well whatever, I'll just merge this and then fixup in a different PR.

[AdmiralCurtiss]

Specifically it's the two calls in DirectFilesystemAssetLibrary to std::filesystem::last_write_time(), you need to pass the std::error_code parameter and then handle the case when that returns an error.

https://en.cppreference.com/w/cpp/filesystem/last_write_time

[iwubcode]

Thanks @AdmiralCurtiss ! I didn't even think of that. I can open a review in a few hours.