niftools / pyffi

PyFFI is a Python library for processing block structured files.
http://www.niftools.org/pyffi
Other
47 stars 26 forks source link

Correct save_as_dds method (textures embedded in NIF) #68

Closed compomega closed 1 year ago

compomega commented 4 years ago

@niftools/pyffi-reviewer

Overview

Corrects the export of DDS textures from a NIF file using the save_as_dds method (part of ATextureRenderData). This is the first step in fixing embedded texture support in the blender plugin.

Fixes Known Issues

Fixes #3

Documentation

[Overview of updates to documentation]

Testing

[Overview of testing required to ensure functionality is correctly implemented]

Manual

[Order steps to manually verify updates are working correctly]

Automated

[List of tests run, updated or added to avoid future regressions]

Additional Information

[Anything else you deem relevant]

compomega commented 4 years ago

What would you do for testing? Should some contrived test NIF files be generated that don't contain data from a particular game? So far I've only tested one path through a NIF file from a game I have so it doesn't cover other versions of the format or other pixel formats.

@neomonkeus I noticed you are working on a big refactor of the blender plugin. I don't want to create too big of a merge nightmare - is it getting close?

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-0.01%) to 57.677% when pulling 9c19b856e96680fedad059a86d03cbbf03811e5a on compomega:fix-save_as_dds into 7f4404dbb8cf832dadd4b3150819340b8764f9b0 on niftools:develop.

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-0.01%) to 57.677% when pulling 9c19b856e96680fedad059a86d03cbbf03811e5a on compomega:fix-save_as_dds into 7f4404dbb8cf832dadd4b3150819340b8764f9b0 on niftools:develop.

compomega commented 4 years ago

I have been trying to create an embedded texture from scratch with NIfSkope to create a test case. It's been a bit tricky but I finally have one for the first case (PX_FMT_RGB8). The texture was generated with GIMP: image

I think this format is supposed to be uncompressed? There is DTX1, DXT3, DXT5, etc. formats as well which are actually compressed. When saving the DDS out of the NIF it was writing a linear size (just the size of the pixel data) instead of a pitch. From my understanding looking at the DDS documentation it should be using a pitch calculation (which is what GIMP is doing in the uncompressed case). Making that change produces the same output DDS file:

#header.flags.linear_size = 1
header.flags.pitch = 1
#header.linear_size = len(self.pixel_data)
header.linear_size = ( header.width * self.bits_per_pixel + 7 ) // 8

Perhaps GIMP is just unforgiving when it comes to the DDS format. What is unfortunate is that the texture must have mipmaps. I don't see any other way to specify the size of the texture in the NIF file. I'll have to see if I can dig up some real world NIF files that have this format.

The usage of pixel_data_matrix was actually removed back in Aug 31, 2008 less than a year after it was added. This looks to be dead code so I'll remove it.

Watch this space :smile:

neomonkeus commented 4 years ago

Firstly, thanks for the PR it looks very clean :+1: The blender plugin pulls the release from pypi, the interface isn't being changed so no worries there. The code refactor is unaffected even if i merge and publish a new build.

As for test nif there are some test folder which get run as part of the build so feel free to expand on them as much as you think.

Regarding removing pixel_data_matrix we should probably check to see if there are nifs that actually using that feature. Even though a feature may be removed from a specific version of the engine, a game which uses it might still be use the feature, eg Morrowind was released in 2002.