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 reading BC4U and DX10 BC1 images #6486

Merged
merged 68 commits into from Dec 6, 2023

Conversation

REDxEYE
Copy link
Contributor

@REDxEYE REDxEYE commented Aug 6, 2022

Changes proposed in this pull request:

  • Refactored DDS reader/writter
  • Add Tile namedtuple for code readability
  • Add support for reading/writing single channel DDS textures

@REDxEYE REDxEYE marked this pull request as draft August 6, 2022 21:05
@REDxEYE
Copy link
Contributor Author

REDxEYE commented Aug 6, 2022

Is it possible to make pre-commit.ci not to reformat some parts of the code?

@radarhere
Copy link
Member

@REDxEYE REDxEYE marked this pull request as ready for review August 25, 2022 13:15
src/PIL/Image.py Outdated
("offset", int),
("tile_args", Tuple),
],
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you say a few words about why this is helpful?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added it to understand what tile structure is. I didn't found any doc for it, so i scouted sources to understand what everything mean

@REDxEYE REDxEYE requested a review from radarhere August 26, 2022 20:53
src/PIL/DdsImagePlugin.py Outdated Show resolved Hide resolved
src/PIL/DdsImagePlugin.py Outdated Show resolved Hide resolved
NV12 = 103
P010 = 104
P016 = 105
_420_OPAQUE = 106
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note - this should be 420_OPAQUE, but you've prefixed it with an underscore because otherwise Python will throw a SyntaxError.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any better way around this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think so, not while using an enum.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about OPAQUE_420? Some might think it's private with a leading underscore.

@radarhere
Copy link
Member

I extracted the support for single channel images, and it has been merged as PR #6820.

@REDxEYE
Copy link
Contributor Author

REDxEYE commented Dec 26, 2022

Awesome!

@REDxEYE
Copy link
Contributor Author

REDxEYE commented Dec 3, 2023

A bit offtop. I may work on adding support for PVR textures in some time. I already implemented ETC decoder(one of they compression formats) https://github.com/REDxEYE/TextureDecoder/blob/master/library/src/decoders/etc/etc1.cpp

@radarhere radarhere changed the title Refactor DDS plugin Added enums to DdsImagePlugin Dec 3, 2023
Tests/test_file_dds.py Outdated Show resolved Hide resolved
Tests/test_file_dds.py Outdated Show resolved Hide resolved
Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Let's also add release notes for this (can be in a followup PR) and retitle the PR for the most important thing ("Add support for ..."?).

NV12 = 103
P010 = 104
P016 = 105
_420_OPAQUE = 106
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about OPAQUE_420? Some might think it's private with a leading underscore.

src/PIL/DdsImagePlugin.py Outdated Show resolved Hide resolved
@radarhere radarhere changed the title Added enums to DdsImagePlugin Support reading BC4U and DX10 BC1 images Dec 5, 2023
@radarhere
Copy link
Member

I've retitled the PR, and added release notes. The release notes also cover #7603

Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
^^^^^^^^^^^^^^^^^^^^^^^^^^

``DDSD``, ``DDSCAPS``, ``DDSCAPS2``, ``DDPF``, ``DXGI_FORMAT`` and ``D3DFMT``
enums have been added to DdsImagePlugin.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
enums have been added to DdsImagePlugin.
enums have been added to py:class:`PIL.DdsImagePlugin`.

@radarhere radarhere force-pushed the improved_dds branch 2 times, most recently from b8aa548 to 306b58a Compare December 5, 2023 21:13
docs/releasenotes/10.2.0.rst Outdated Show resolved Hide resolved
docs/releasenotes/10.2.0.rst Outdated Show resolved Hide resolved
docs/releasenotes/10.2.0.rst Outdated Show resolved Hide resolved
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@hugovk hugovk merged commit 7cc0482 into python-pillow:main Dec 6, 2023
57 checks passed
@hugovk
Copy link
Member

hugovk commented Dec 6, 2023

Thank you for your patience! This will be in the next release, set for 2nd January 2024.

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

Successfully merging this pull request may close these issues.

None yet

4 participants