Occasional wrong textures in objects

[Enet4]

In some mods with lots of textures (in this case Wazzal II), it may happen that some textures are instantly "swapped" for another irrelevant one just by moving around a bit. The two images below demonstrate this.

I recall experiencing something similar in 1.34.1, but it would be nice to correct this before v1.0.

[verhoevenv]

Whaaaat. This codebase just keeps on giving. Any idea what's wrong?

[Enet4]

Is it a glitch in the Matrix? :P

To be honest, I put little trust in our resource managers (mostly resource_handler.cpp) right now, as more than one other issue have emerged from there so far. Building these components from scratch could rid us from many troubles in the future, and might not be too extreme a thing to do.

[Enet4]

I don't know when I'm going to be able to pick this up, but here are my thoughts on how things in OpenNotrium are currently done. This should hopefully enable me or someone else attempt to solve the various issues around it.

There are multiple types of ID related with textures:

1. texture ID for the main resource handler entity (resource_handler)

   • type int
   • the value -1 is deliberately used for the "none" texture and is also returned when an error occurs (this should really change to something like -2)
   • used to identify a texture-type resource in the resource_handler
   • as an implementation detail, it is the index to a std::vector containing all loaded textures at some point in the game
   • this vector contains records (of type resource_handler::texture_handling_primitive_base) with the texture's name, the mod name, when it was last used, and the texture ID according to the graphics engine (a member called texture_handle_in_grim, because Grim was the name of the engine used in the original Notrium).
   • resource_handler has its own Texture_Set member function which maps the ID to the engine ID
   • the resource handler is free to unload textures. The game should be able to re-issue old textures at will, though.
   • to unload a texture, the corresponding texture is deleted at the engine and the texture_handle_in_grim member is set to -1.
   • the member function load_texture ensures that a unique ID for the texture is given, but does not ensure that the texture is loaded

2. Engine ID for the "Grim" engine

   • type int, negative values reserved for error codes (only non-negative values make valid texture IDs)
   • the graphics Engine implements the same (or similar) API of the Grim engine for rendering stuff.
   • a different type of ID is used here to identify the texture at the engine's own abstraction
   • as an implementation detail, it's the index to a fixed size array of EngineTexture records, specifying the texture's name, dimensions, whether it's to be flipped when drawn, whether it's deleted, and the OpenGL texture identifier.
   • to delete a texture, the deleted member is set to true. Textures created afterwards may then treat that position as a free slot and use it.

3. The OpenGL texture identifier

   • of type GLuint
   • an implementation detail of the graphics engine, basically

At some point in the game, if there are many textures in the mod, the loading process of a certain texture might fail or miss. Textures are unloaded without being re-issued, or in a way which prevents the resource handler from re-issuing it, which leads to the game holding an error code instead of a valid texture ID. The game may then try to Texture_Set at the resource handler with this ID (-1), which is invalid. Instead of assigning the right texture, some other previously set texture will be used in its stead. By the time resource_handler::Texture_Set is called with an error code, the real issue might have already happened. This needs to be traced back some time before, where the texture fails to load.

Another important hint: this can be related to missing texture files, or files with the wrong letter casing. On Linux, Wazzal II's "J.png" does not match with texture name "j.png". I've also noticed that some requested texture files in Wazzal II do not exist at all. Mods should indeed avoid mismatches such as these, but the game could also be a bit more robust.

[verhoevenv]

Interesting write up!

I'm still a bit unclear whether this is just bad API design or an internal implementation bug in resource_manager.cpp. The implementation of resource_handler::load_texture_in_grim doesn't seem to make a lot of sense to me.. We try two directories but we don't care if it fails after the second one? If all slots are full we attempt to release some textures but then don't even bother reloading the new texture? What is this mess?

Do you think the whole texture manager with unloading is worth it? On desktop computers, we surely have enough memory / graphics memory available these days, but I'm not sure about other devices. I've seen OpenNotrium being ported to handhelds so I think it's better to keep this behaviour? Then again, I'm unsure of the impact of unloading/reloading textures, I could imagine it actually leading to worse performance if the mod attempts to use many textures. The fixed and preallocated array of 1000 EngineTextures also seems very arbitrary...

[Enet4]

Nice follow-up questions! Let's see...

I'm still a bit unclear whether this is just bad API design or an internal implementation bug in resource_manager.cpp. The implementation of resource_handler::load_texture_in_grim doesn't seem to make a lot of sense to me.. We try two directories but we don't care if it fails after the second one? If all slots are full we attempt to release some textures but then don't even bother reloading the new texture? What is this mess?

I have been playing around with these portions of the code, trying to make it work better. It's a bit hard to evaluate whether this is a poor API design, but the implementation is certainly in need of improvements. I have pushed my current changes to a branch in case you want to see them, surely a WIP, but it seems that some of these changes also cover those concerns already: We should indeed check whether the texture was loaded successfully, and it should try to get the texture again after the clean-up.

Do you think the whole texture manager with unloading is worth it? On desktop computers, we surely have enough memory / graphics memory available these days, but I'm not sure about other devices. I've seen OpenNotrium being ported to handhelds so I think it's better to keep this behaviour? Then again, I'm unsure of the impact of unloading/reloading textures, I could imagine it actually leading to worse performance if the mod attempts to use many textures. The fixed and preallocated array of 1000 EngineTextures also seems very arbitrary...

I wouldn't mind that we keep the resource unloading mechanism, because we never know whether people will want to port it to smaller devices and expect it not to require too much memory. We've seen that Wazzal II can consume a lot of memory, and the difference between 300 as the number of active textures and the full 700 might make a difference in practice. We could probably use something a bit smarter though, even if just as an attempt to get rid of this maximum of pre-allocated textures. It was originally smaller than 1000, increased from 200 in #67 because Wazzal II managed to have that many textures active at the same time. The whole issue sounds too much like the ages-old question of garbage collection strategies, and there's probably one that works sufficiently well for Notrium without affecting performance significantly.

In the mean time, we could also make the game load a placeholder image (something with "?", a checkerboard or whatever) when it doesn't find the suggested texture anywhere. From my quick tests, this can resolve the main issues shown here. Another approach that I am not very fond of is to make file loading always case-insensitive in all platforms... it really doesn't solve the problem with mods actually missing the texture file, anyway.