[Fix] Fixes compressed DDS files with sizes which aren't divisble by 4, non-square compressed texture mipmaps, and ATI2 textures saved by OpenIV

[hickorysb]

This PR would patch several additional DDS errors I found. I've done my best to comment with good explanations in the code why the changes I made were made. Prior to merge, if this does get merged I can go remove all those comments if that is desired. As for some samples of files that you can test to see before and after see the following (these errors were all found by going through the YTDs from PLOKS Car pack so all examples are from there).

• tltypes.ytd - specifically the texture generic_leather2_n, texture has an incorrect stride baked into the YTD and the BC5 decompression method had a bit of an apparent copy/paste mistake.
• q820.ytd - specifically the textures 1_leathern and airbag_opac1, both files had issues loading the final mipmap prior to these patches.
• r820.ytd - specifically r8_005_diff had a similar issue with the last mipmap level.

[hickorysb]

Whoops, this also contains the commits from the other patch branch. I can fix that later, if you intend to merge this in, for now though this is mainly to give some further idea of some possible DDS handling improvements.

[kirill-mapper]

Incorrect ATI2 texture data is created by OpenIV when saving, I reported this bug and it has been on their roadmap for a few years without any progress. All you need is to save a vanilla mapdetail.ytd in openiv to get a bunch of incorrect textures. If your patch fixes these textures, that's great

[hickorysb]

@kirill-mapper seemingly it does as the texture data in question looked wrong in OpenIV when opening it but correct in CodeWalker. However, this patch does not currently update the stride saved in the YTD it merely changes the stride used to calculate how much data to load from the YTD for any one particular texture. I haven't looked deep enough at code walker to see if it recalculates the stride used everytime it saves the ytd, if not maybe it should?

[kirill-mapper]

After saving in OIV, the texture data ati2/bc5 changes, stride, as you noted, maybe something else. For example, GTA does not react to this in any way and reads such textures without problems. But CW is sensitive to the texture data and will use incorrect data, which will lead to a viewer error and a visual display bug, when saving such a texture, it will be broken and will not be read anywhere. I think if the patch takes into account the calculated stride when opening the texture, there will be no display error, and saving will not break the texture, but fix it

[kirill-mapper]

@kirill-mapper seemingly it does as the texture data in question looked wrong in OpenIV when opening it but correct in CodeWalker. However, this patch does not currently update the stride saved in the YTD it merely changes the stride used to calculate how much data to load from the YTD for any one particular texture. I haven't looked deep enough at code walker to see if it recalculates the stride used everytime it saves the ytd, if not maybe it should?

It works! I just opened an incorrect texture saved in openIV with your patch, manually changed the stride and it fixed it after saving. So yes, when saving, you need to take this type of textures into account and calculate stride if it is incorrect

[hickorysb]

@kirill-mapper ATI2 isn't the only texture I've seen this issue on so likely stride should just be recalculated for all textures regardless of format. Also there's not much point in comparing it as computing the stride is more computationally intensive than placing the recalculated stride in the file. Comparing just wastes a extra "logical cycle".

Edit: I just got home from work, so I'll look into that.

[hickorysb]

This change actually makes #321 obsolete as I altered the way that that patch is done and gave an updated explanation. I still need to do a bit more research and get the 2x1 and 1x1 mipmaps figured out for DXT1, for some reason they seem to not be calculating color correctly.

[hickorysb]

I still need to do a bit more research and get the 2x1 and 1x1 mipmaps figured out for DXT1, for some reason they seem to not be calculating color correctly.

Actually apparently it's just an issue with the actual texture in question that I was messing with as exporting it to a DDS and resaving it with Paint.NET and then reimporting and saving and reopening the YTD makes the issue go away.

[kirill-mapper]

Vanilla textures don't have mipmaps smaller than 4x4, I think it's also important to take this into account and cut them off when saving

[dexyfex]

It bothers me that it adds extra overhead to recalculate the stride and total number of bytes when loading legacy textures, but I guess not much can be done about that?
Out of interest, have you run a test on all the vanilla ytd's to check the stride was recalculated correctly for those?

[dexyfex]

Also seems slightly wasteful that it calls DDSIO.GetDXGIFormat 3 times during loading

[dexyfex]

Does TextureBase.CalcDataSize method work correctly? I'm wondering if this could be used in Texture.Read instead of adding the extra code in TextureData.Read, so it then just works the same way as the gen9 code? (see gen9 code in TextureBase.Read)

[dexyfex]

So I've been looking more generally at the code surrounding this issue, and it seems clear that probably CalcDataSize isn't correct at the moment, which would cause similar/same problems for gen9... so I guess really the best approach will be to get all these additions/fixes into CalcDataSize, then the TextureData.Read code doesn't have to care if it's gen9 or not.
I'd like to get this all sorted out today if possible, so if I don't hear back from anyone in the next couple of hours I'll just go ahead and merge this PR to handle the other bugs, and then do the refactoring to CalcDataSize stuff.

[kirill-mapper]

It appears that Stride represents the width of one texture row, measured in 32-bit words (DWORD) rather than bytes.
This change in the CalculateStride() method converts the value to 32-bit words:
return (ushort)(rowPitch / 4); // Previously: return (ushort)rowPitch;
If we add calculation on read for Legacy this.Stride = CalculateStride();, this will automatically fix bugs in reading and writing buggy textures in CW

[hickorysb]

@kirill-mapper No, stride is the number of bytes in one row including padding.

https://learn.microsoft.com/en-us/windows/win32/medfound/image-stride

The stride is the number of bytes from one row of pixels in memory to the next row of pixels in memory.

[hickorysb]

It bothers me that it adds extra overhead to recalculate the stride and total number of bytes when loading legacy textures, but I guess not much can be done about that?
Out of interest, have you run a test on all the vanilla ytd's to check the stride was recalculated correctly for those?

I have not yet run a test on all the vanilla YTDs. I can do that though and report back later tonight.

Also seems slightly wasteful that it calls DDSIO.GetDXGIFormat 3 times during loading

You are very correct. I just didn't really think about it. I can correct that tonight.

Does TextureBase.CalcDataSize method work correctly? I'm wondering if this could be used in Texture.Read instead of adding the extra code in TextureData.Read, so it then just works the same way as the gen9 code? (see gen9 code in TextureBase.Read)

So I've been looking more generally at the code surrounding this issue, and it seems clear that probably CalcDataSize isn't correct at the moment, which would cause similar/same problems for gen9... so I guess really the best approach will be to get all these additions/fixes into CalcDataSize, then the TextureData.Read code doesn't have to care if it's gen9 or not.
I'd like to get this all sorted out today if possible, so if I don't hear back from anyone in the next couple of hours I'll just go ahead and merge this PR to handle the other bugs, and then do the refactoring to CalcDataSize stuff.

I'll have to look closer at this once I'm home, currently on break between routes, but if you haven't started on it then I'll look at it tonight.

@dexyfex

[dexyfex]

Thanks, and sorry I haven't gotten back to this yet.
That would be great if you could look into fixing/using CalcDataSize... I had another quick look at that and perhaps in most cases it actually does return the correct size? Except maybe for the minimumLengthPerMip stuff.
Also CalcDataSize doesn't seem particularly efficient either, but I don't think we should worry about that for now, but rather just focus on the possible refactoring.

[hickorysb]

Apologies that I've taken so long to get back to this going to open my IDE here soon and take a look. I've just been super busy since.

[hickorysb]

@dexyfex This should address the CalcDataSize concerns, it also applies the DXT size fixes to G9 textures when they are loaded.
Does not necessarily address all efficiency concerns.

[IMCraytex]

noticed one thing though with this patch while we can load those broken dds files upon saving a loaded ytd as a new ytd it makes the file bigger, by about 1.5mb's which can be bad

[hickorysb]

@IMCraytex Unknown why this might be. The commit doesn't modify anything related to exporting. Have you built a fresh copy of CodeWalker starting at the commit that this PR was originally based on to see if you have the same size difference on that build compared to the PR build?

[HyperExtendedReality]

I fixed it with my PR #348
It uses DirectXTexNet a .Net wrapper for DirectXTex dds file handling and now correctly and safely reads data from all dds formats including modern ones. Let me know if it works for your case just requires .net runtime now.

[dexyfex]

Obviously we don't just randomly add dependencies to this project.