KSPModdingLibs / KSPCommunityFixes

Community patches for bugs in the KSP codebase
49 stars 17 forks source link

Custom LRTR tech tree icons not showing #224

Closed gotmachine closed 4 months ago

gotmachine commented 4 months ago

Using the LRTR mod, forum user @Scottmm78 is reporting some icons of the custom tech tree not showing when KSPCF is installed.

frBjk9o P8lKkyO

User logs : LRTR_KSP.zip

This seems to happen only with multiple-of-4-width/height icons, meaning those converted to DXT5. The user is reporting the issue happening with both the texture cache enabled and disabled

I can't reproduce this locally, neither in isolation nor with the same mod set as him.

I've asked him to try a stripped down version of the mod in a minimum install : LRTR_KSPCF_Test.zip

Waiting for some feedback on this.

gotmachine commented 4 months ago

Okay, managed to reproduce this by running a minimal repro install from the steam directory, ie : C:\Program Files (x86)\Steam\steamapps\common\Kerbal Space Program In that case, the problematic textures indeed fail to load on the bool ImageConversion.LoadImage(Texture2D tex, byte[] data, bool markNonReadable) call with the following Player.log error (example shown for a 100x100 texture with mipmaps) :

d3d11: failed to create 2D texture id=1984 width=50 height=50 mips=6 dxgifmt=77 [D3D error was 80070057]
d3d11: failed to create 2D texture shader resource view id=1984 [D3D error was 80070057]

I've not managed to reproduce this by putting the game in various other paths, for example one with a Program Files (x86) subfolder.

I actually noticed such errors popping occasionally in the past, but always failed to identify and reproduce them consistently (the fact that there is no way to find back which texture this is about from the error message doesn't help...)

I was suspecting some screwup in the threaded file load or in how the buffers are read, but checking the buffer against a manually reloaded file with File.ReadAllBytes() show no difference, and the error reproduce just as well with the byte[] returned by it.

gotmachine commented 4 months ago

Okay, scrap all that, the issue is caused by having a lower than max global mipmap bias during loading.

On a side note, this already was somewhat of an issue, as if the user has lower than max mipmap level during loading, the DXT5 texture cache is created with that lower level and textures will consequently always be lower res if the user latter increase decrease the mipmap level (ie, increase texture quality in the settings).

But the current issue seems to be that Unity doesn't like calling LoadImage() to load a PNG/JPG into a Texture2D whose format is DXT5 with mipmaps, if at the same time QualitySettings.masterTextureLimit is set higher than 0.

Stock load such textures through UnityWebRequestTexture, which first return a RGBA32 texture, and in a second step, convert it to DXT5 by calling Texture2D.Compress(), then finally create the mipmaps and upload it to the GPU with Texture2D.Apply(updateMipmaps: true). I remember not doing that explicitly for performance concerns, but I don't remember the exact impact.

Anyway, the workaround is likely, when QualitySettings.masterTextureLimit is higher than 0, to always load textures into ARGB32, and then to convert them like stock does.

gotmachine commented 4 months ago

So this has turned into quite the rabbit hole...

After loosing many hours trying a bunch of stuff with what unity provides, implementing a custom PNG loader, messing around with pointers and blockcopy, I realized the issue : Unity simply doesn't support converting a NPOT image to DXT5 + mipmaps.

Then I asked myself, but how in hell does stock do ? And yeah. It doesn't. Turns out that when I wrote the custom PNG loader, I misunderstood in which cases stock is generating mipmaps, so all that time, KSPCF was generating them when it actually shouldn't have.

gotmachine commented 4 months ago

Should be fixed in 1.35.1