Pillow icon indicating copy to clipboard operation
Pillow copied to clipboard

Support BC5 DDS encoding

Open pepperoni505 opened this issue 4 years ago • 21 comments

BC5 is currently not supported by Pillow. There is already some code in the repo that can read BC5 files. I would have made a PR for this but I'm not familiar with how this project is laid out.

pepperoni505 avatar May 15 '21 04:05 pepperoni505

Hi. You may be interested to know that there is a PR in review that adds support for saving uncompressed DDS textures - #5402.

Where is the code that can read BC5 files? I look at https://github.com/python-pillow/Pillow/blob/master/src/PIL/DdsImagePlugin.py and I can only see code to read uncompressed textures, DXT1, DXT3, DXT5 and DX10? If you can't see it, attaching an BC5 file here that you can open with Pillow would be another way of answering my question.

radarhere avatar May 15 '21 05:05 radarhere

Hello. Sorry, I should have been more specific. I was talking about code in the following file: src/libImaging/BcnDecode.c. I don't know how easy it would be to add it into src/PIL/DdsImagePlugin.py.

pepperoni505 avatar May 15 '21 17:05 pepperoni505

I tried modifying src/PIL/DdsImagePlugin.py to accept BC5 files, but it doesn't fully work. Here is a diff:

diff --git a/src/PIL/DdsImagePlugin.py b/src/PIL/DdsImagePlugin.py
index 1a7fe003..83c6c16d 100644
--- a/src/PIL/DdsImagePlugin.py
+++ b/src/PIL/DdsImagePlugin.py
@@ -97,6 +97,9 @@ DXT5_FOURCC = 0x35545844
 DXGI_FORMAT_R8G8B8A8_TYPELESS = 27
 DXGI_FORMAT_R8G8B8A8_UNORM = 28
 DXGI_FORMAT_R8G8B8A8_UNORM_SRGB = 29
+DXGI_FORMAT_BC5_TYPELESS = 82
+DXGI_FORMAT_BC5_UNORM = 83
+DXGI_FORMAT_BC5_SNORM = 84
 DXGI_FORMAT_BC7_TYPELESS = 97
 DXGI_FORMAT_BC7_UNORM = 98
 DXGI_FORMAT_BC7_UNORM_SRGB = 99
@@ -150,12 +153,18 @@ class DdsImageFile(ImageFile.ImageFile):
             elif fourcc == b"DXT5":
                 self.pixel_format = "DXT5"
                 n = 3
+            elif fourcc == b"BC5S":
+                self.pixel_format = "BC5"
+                n = 5
             elif fourcc == b"DX10":
                 data_start += 20
                 # ignoring flags which pertain to volume textures and cubemaps
                 dxt10 = BytesIO(self.fp.read(20))
                 dxgi_format, dimension = struct.unpack("<II", dxt10.read(8))
-                if dxgi_format in (DXGI_FORMAT_BC7_TYPELESS, DXGI_FORMAT_BC7_UNORM):
+                if dxgi_format in (DXGI_FORMAT_BC5_TYPELESS, DXGI_FORMAT_BC5_UNORM):
+                    self.pixel_format = "BC5"   
+                    n = 5
+                elif dxgi_format in (DXGI_FORMAT_BC7_TYPELESS, DXGI_FORMAT_BC7_UNORM):
                     self.pixel_format = "BC7"
                     n = 7
                 elif dxgi_format == DXGI_FORMAT_BC7_UNORM_SRGB:

I have the following sample code used to convert this to a .PNG image:

from PIL import Image

image = Image.open("C:/Users/pepperoni505/Downloads/test.dds")
image.save("C:/Users/pepperoni505/Downloads/test.png", 'png')

That generates this PNG image. As you can see, it's just an empty image.

pepperoni505 avatar May 16 '21 02:05 pepperoni505

Just because I'm thinking ahead - if you're interested in Pillow because able to read these files before being able to write them, would you be able to generate a BC5 image that is smaller in file size, that you're happy for us to include in our test suite and distribute under the Pillow license?

radarhere avatar May 16 '21 03:05 radarhere

Yeah, I can send another image in a few hours. Any ideal size?

pepperoni505 avatar May 16 '21 03:05 pepperoni505

128px by 128px?

radarhere avatar May 16 '21 04:05 radarhere

Ok, I have a fix ready - it turns out the alpha channel was just set to zero. If it's not inconvenient to change my size request, 256 by 256.

radarhere avatar May 16 '21 05:05 radarhere

Here's a 256x256 BC5_UNORM image along with a PNG. Thanks for making the PR! Is it possible to support the other BC5 types?

pepperoni505 avatar May 16 '21 23:05 pepperoni505

Thanks. I've incorporated the DDS file into the PR. I've left your PNG alone - for some reason, it has slightly different colors?

The easiest way to add the other formats... are you able to generate sample TYPELESS and SNORM files, for inclusion in Pillow, same as before?

radarhere avatar May 17 '21 10:05 radarhere

I managed to get a SNORM file but was unable to get a TYPELESS file. BC5_SNORM.zip

pepperoni505 avatar May 18 '21 00:05 pepperoni505

In trying to get Pillow to read the SNORM file, I came up with this. bc5_snorm Is that how your image should look?

radarhere avatar May 18 '21 06:05 radarhere

Yeah, that looks right. When I generated the image there were some weird artifacts as well

pepperoni505 avatar May 18 '21 06:05 pepperoni505

Thanks. I've added SNORM to the PR.

radarhere avatar May 18 '21 07:05 radarhere

Thanks for your PR! In theory couldn't you also include TYPELESS importing? It's supported for BC7 yet there isn't a test for it

pepperoni505 avatar May 18 '21 15:05 pepperoni505

I'd like to try and find some documentation saying that TYPELESS should be treated as UNORM.

Let me ask a different question - you titled this "Support BC5 DDS encoding". Is this about reading that encoding (opening files), or encoding data into that format (saving files)?

radarhere avatar May 18 '21 22:05 radarhere

I guess both opening and saving - I only need to open BC5 files for now but saving as BC5 would be a huge plus

pepperoni505 avatar May 18 '21 23:05 pepperoni505

Ok, I found https://docs.microsoft.com/en-us/windows/win32/direct3d11/overviews-direct3d-11-resources-intro#strong-vs-weak-typing

In a type less resource, the data type is unknown when the resource is first created. The application must choose from the available type less formats

So I think that TYPELESS doesn't infer which type should be used. So I have updated the PR to go with our tradition of treating TYPELESS as UNORM. I've hexedited a copy of the UNORM file to be TYPELESS for a test.

radarhere avatar May 19 '21 11:05 radarhere

Out of curiosity, does your PR for DDS saving support BC5?

pepperoni505 avatar May 25 '21 03:05 pepperoni505

No. I'm working on DXT1 saving at the moment though, so maybe I'll get to BC5 eventually.

radarhere avatar May 25 '21 10:05 radarhere

#5501 has been merged, so when Pillow 8.3.0 is released on July 1, it will support reading BC5.

radarhere avatar Jun 11 '21 07:06 radarhere

Thank you!

pepperoni505 avatar Jun 13 '21 03:06 pepperoni505