Ryan-rsm-McKenzie / bsa

C++ library for working with the Bethesda archive file format
MIT License
41 stars 6 forks source link

created fo4dds ba2s are invalid #8

Closed Guekka closed 2 years ago

Guekka commented 2 years ago

I am currently investigating, but haven't found anything yet

Reproducing:

Recap of what I found so far:

Guekka commented 2 years ago

I'm not sure it is enough, but on my small sample, the fix was to change BitsPerPixel. Dividing it by 4 fixed the issue

Guekka commented 2 years ago

I've also noticed bsarch does not have Flags and BitsPerPixel. Instead, it has cubemaps:

https://github.com/TES5Edit/TES5Edit/blob/dev/Tools/BSArchive/wbBSArchive.pas#L278

Guekka commented 2 years ago

Dividing BitsPerPixel per 4 is not a good solution. But I am still pretty confident the bug comes from this part

Guekka commented 2 years ago

The problem might be more important than expected. rsm-bsa does not produce the same header as archive2 with the linked file

bsarch gets it right, so looking at the code might be helpful

test.zip

Ryan-rsm-McKenzie commented 2 years ago

What are the permissions on this file?

Ryan-rsm-McKenzie commented 2 years ago

I see what the issue is, beth does a bunch of stupid stuff to calculate chunk size and bits per pixel

Ryan-rsm-McKenzie commented 2 years ago

Fixed by 84c18fc96c022c584194cb7a43b5f250b0cc5f0a, but it still needs tests to guard against regressions, such as textures with cubemaps. The texture provided here is also a good sample, since it found a bug in how textures were chunked, but it depends on the licensing.

Guekka commented 2 years ago

The licensing does not allow it, but the author is active. I've asked for permission and will keep you informed

Guekka commented 2 years ago

Can confirm the fix appears to be working. The archive produced by rsm-bsa are still different, but I suppose it's only the file order that changes

Guekka commented 2 years ago

Already 2 months? Time flies So, I've found another mod to reproduce the bug, and it has open permissions. https://www.nexusmods.com/skyrim/mods/95466? I tried with the whole mod, but mushroom01_e is marked as "XBOXDDS"

Ryan-rsm-McKenzie commented 2 years ago

These files don't manage to add test coverage for the newly discovered chunking strategy for dds files, but they did manage to uncover a new bug. I believe cubemaps are excluded from chunking, which is what the flag in the archive file header is for.

Ryan-rsm-McKenzie commented 2 years ago

Added more coverage and another fix in e96e4abdd6fc4eea07b333c5ae7fbe587d46ee31

Guekka commented 2 years ago

So, do you know the consequences of this bug? I didn't get any report, so I assume it doesn't cause CTD?

Guekka commented 2 years ago

texture.zip

So, here's the texture that originally found the issue. I manually blacked it out. Pretty sure we can't get permission issue after that

Ryan-rsm-McKenzie commented 2 years ago

Added coverage for chunking in 63c01e2c04252d3224b5bf1cbbe6dfcdf615acd2

Ryan-rsm-McKenzie commented 2 years ago

I don't know if there's actually an issue with chunking archives incorrectly, but it's best to conform to whatever Archive2.exe produces.