Skip to content

Conversation

@jleclanche
Copy link
Contributor

Only supports DXT1 and DXT5 for now.
The pixel formats ideally should be supported in decode.c instead,
but for now this is good enough.

Fixes #252

@hugovk
Copy link
Member

hugovk commented Jan 5, 2016

Thanks for the contribution!

Please could you include unit tests? (See here.)

That way we can know it works on all Python versions and other operating systems, and to make sure it doesn't get broken in the future.

@wiredfool
Copy link
Member

@jleclanche Thanks for pushing this in as a PR.

Quick set of reliability changes:

  • need to remove the main() and run the tests from the test suite
  • Can't have an assert in there, check and raise a SyntaxError or IOError
  • The unpack( ,fp.read(n)) should be checked for length and an IOError thrown if the length is not appropriate ((after _ensure_read in TiffImagePlugin.ImageFileDirectory_v2) (which should probably be pulled out into _binary or _util))
  • need to add some documentation at least in the image-file-formats.rst file.

If you don't get to these, It's something that I'll get to at some point.

Copy link
Member

Choose a reason for hiding this comment

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

Please name like DdsImageFile.

@jleclanche
Copy link
Contributor Author

Did the easy bits. I'll probably leave it up to you for the rest @wiredfool if you don't mind... feeling a bit nauseated right now ;)

Only supports DXT1 and DXT5 for now.
The pixel formats ideally should be supported in decode.c instead,
but for now this is good enough.

Fixes #252
@wiredfool
Copy link
Member

no worries

@hugovk
Copy link
Member

hugovk commented Jan 5, 2016

@jleclanche Do you have some test images we can distribute under the PIL licence?

@wiredfool
Copy link
Member

@hugovk -- there are the ones here: http://code.qt.io/cgit/qt/qtimageformats.git/tree/tests/shared/images/dds?id=a4670f609c48fed61cbacf1a0a780ea27483ce18

But I think they're some form of gpl, either gplv2, or 3, or lgpl 2.1 or 3, or fdl. Which one, and if that's technically compatible with what we've got is above my pay grade.

@hugovk
Copy link
Member

hugovk commented Jan 5, 2016

I've sent an email to ask if we can use them under the PIL licence.

@Metallicow
Copy link
Contributor

@wiredfool I got a almost complete set of test images you can use for this and for whatever for that matter. How would you like them. I have 50 saved in various dxt levels with a psd also used with the nvidia photoshop plugin to make them all.
Should I do a PullReqz where at??? or can a 7z be attached to a post like here?

@jleclanche Thanks for your work on this!

@wiredfool
Copy link
Member

@Metallicow You attach a zip like archive here.

@Metallicow
Copy link
Contributor

@wiredfool Here ya go.
These where made a few years back when I was tinkering with writing a python plugin but I don't think the photoshop plugin has changed any. They should all be labeled correctly.
License CC0/Public domain/PIL whatever.
ddstests.zip

hugovk added a commit to hugovk/Pillow that referenced this pull request Jan 6, 2016
hugovk added a commit to hugovk/Pillow that referenced this pull request Jan 6, 2016
@hugovk
Copy link
Member

hugovk commented Jan 6, 2016

See #1650 for this PR with added tests.

@jleclanche
Copy link
Contributor Author

Closing in favour of #1650 which contains my commit

@jleclanche jleclanche closed this Jan 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants