Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support BC5 DDS encoding #5499

Open
pepperoni505 opened this issue May 15, 2021 · 21 comments
Open

Support BC5 DDS encoding #5499

pepperoni505 opened this issue May 15, 2021 · 21 comments

Comments

@pepperoni505
Copy link

pepperoni505 commented May 15, 2021

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.

@radarhere
Copy link
Member

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.

@pepperoni505
Copy link
Author

pepperoni505 commented May 15, 2021

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
Copy link
Author

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.

@radarhere
Copy link
Member

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?

@pepperoni505
Copy link
Author

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

@radarhere
Copy link
Member

128px by 128px?

@radarhere
Copy link
Member

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.

@pepperoni505
Copy link
Author

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?

@radarhere
Copy link
Member

radarhere commented May 17, 2021

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?

@pepperoni505
Copy link
Author

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

@radarhere
Copy link
Member

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

@pepperoni505
Copy link
Author

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

@radarhere
Copy link
Member

Thanks. I've added SNORM to the PR.

@pepperoni505
Copy link
Author

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

@radarhere
Copy link
Member

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)?

@pepperoni505
Copy link
Author

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

@radarhere
Copy link
Member

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.

@pepperoni505
Copy link
Author

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

@radarhere
Copy link
Member

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

@radarhere
Copy link
Member

radarhere commented Jun 11, 2021

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

@pepperoni505
Copy link
Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants