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.

Sign up for GitHub

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

Jump to bottom

Microsoft Direct DrawSurface (DDS) support. #1021

Merged
merged 23 commits into from
Sep 25, 2024
Merged

Microsoft Direct DrawSurface (DDS) support. #1021

merged 23 commits into from
Sep 25, 2024

Conversation

p4paul
Copy link
Contributor

@p4paul p4paul commented Sep 20, 2024

Initial implementation of Microsoft Direct DrawSurface (DDS) support.
DDS reader is based on https://github.com/npedotnet/DDSReader

@p4paul
Basic wrapper around DDSReader.
82bf485 
Nasty hack to get initial plugin working...
Mark and Reset the imageinput stream when reading the header.  Then read the whole buffer into DDSReader and generate a fixed BufferedImage ignoring the getDestination() BufferedImage.
@p4paul
Read header (DDSHeader) and pass into DDSReader
ec7635d 
Remove header methods using buffer offset addressing.
Switch endian for DX1-DX5 types
@p4paul
Fix pixel order to ARGB
120d6eb
@p4paul
Push ImageInputStream into DDSReader
0cd056a 
Unable to determine buffer length (so as a hack I over allocate buffer and read)

```
byte[] buffer = new byte[width * height * 4];
int len = imageInput.read(buffer);
```

Added test files for all supported formats.
@p4paul
Added processImageStarted and stubbed out processImageProgress and pr…
7082a53 
…ocessReadAborted

Added all DDS format test cases to getTestData
@p4paul
Remove offset and use imageInput.readFully
79aef6e 
Reads next image calculating/updating width/height from mipmapLevel.
Probably will never get used as we only want the largest image returned.
@p4paul
Code cleanup and added exception handling.
7eb45b1
@p4paul
Use Enum DDSType instead of int values.
dede5ea 
Pass imageIndex into mipmap.
@p4paul p4paul mentioned this pull request Sep 20, 2024
Closed
haraldk
haraldk reviewed Sep 23, 2024
View reviewed changes
Copy link
Owner

@haraldk haraldk left a comment

Choose a reason for hiding this comment

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

First of all! Great work, this is a very nice addition! 👍

I have made some comments, inline in the code.

In addition, the code base uses 4 spaces indents, but your code seems to use tabs. I will have to ask you to change this. 😀

.run/DDSImageReader.run.xml Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/plugins/dds/DDSHeader.java Outdated Show resolved Hide resolved
imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/plugins/dds/DDSHeader.java Outdated Show resolved Hide resolved
imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/plugins/dds/DDSImageReader.java Outdated Show resolved Hide resolved
imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/plugins/dds/DDSReader.java Outdated Show resolved Hide resolved
imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/plugins/dds/DDSHeader.java Outdated Show resolved Hide resolved
imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/plugins/dds/DDSType.java Outdated Show resolved Hide resolved
imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/plugins/dds/DDSHeader.java Outdated Show resolved Hide resolved
imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/plugins/dds/DDSHeader.java Outdated Show resolved Hide resolved
p4paul and others added 8 commits September 24, 2024 09:55
@p4paul @haraldk
Update imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/pl…
be2d6d6 
…ugins/dds/DDSHeader.java


Improve IIOException "Invalid DDS header size"

Co-authored-by: Harald Kuhr <harald.kuhr@gmail.com>
@p4paul @haraldk
Update imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/pl…
771e486 
…ugins/dds/DDSHeader.java


Improve Magic IIOException

Co-authored-by: Harald Kuhr <harald.kuhr@gmail.com>
@p4paul
Delete .run directory
53356e7
@p4paul
Format tabs -> 4 spaces
bcfab45
@p4paul
Convert 4byte array into %08x string format.
a648d1a
@p4paul
Don't need to expose classes outside the package
e95d403 
DDSHeader, DDSReader, DDSType
@p4paul
(fix) wrong public...
789cbe8
@p4paul
Move setByteOrder to DDSImageReader.readHeader
edf132b
haraldk
haraldk reviewed Sep 24, 2024
View reviewed changes
imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/plugins/dds/DDSHeader.java Outdated
byte[] magic = new byte[DDS.MAGIC.length];
imageInput.readFully(magic);
if (!Arrays.equals(DDS.MAGIC, magic)) {
throw new IIOException(String.format("Not a DDS file. Expected DDS magic %s, read %s", String.format("%08x", new BigInteger(1, DDS.MAGIC)), String.format("%08x", new BigInteger(1, magic))));
Copy link
Owner

Choose a reason for hiding this comment

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

Sorry, I forgot that the magic was a byte array, not an int...

I think, however, this should work:

Suggested change
throw new IIOException(String.format("Not a DDS file. Expected DDS magic %s, read %s", String.format("%08x", new BigInteger(1, DDS.MAGIC)), String.format("%08x", new BigInteger(1, magic))));
throw new IIOException(String.format("Not a DDS file. Expected DDS magic %08x, read %08x", new BigInteger(1, DDS.MAGIC), new BigInteger(1, magic)));

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 did something similar...
String.format("%08x", new BigInteger(1, magic))

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, but that is 3 String.formats instead of 1...

@p4paul
Copy link
Contributor Author

p4paul commented Sep 24, 2024

I think that addresses all of your review items; do let me know if I missed anything.

@p4paul p4paul requested a review from haraldk September 24, 2024 17:03
haraldk
haraldk requested changes Sep 24, 2024
View reviewed changes
Copy link
Owner

@haraldk haraldk left a comment

Choose a reason for hiding this comment

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

Excellent.

Almost there! Promise! 😀

If you can just revert the .gitignore file to what it was before, add some @Ingores to all the failing tests, and clean up some String.formats, we're good to go!

Awsome work!

.gitignore Outdated Show resolved Hide resolved
imageio/imageio-dds/src/main/java/com/twelvemonkeys/imageio/plugins/dds/DDSHeader.java Outdated
byte[] magic = new byte[DDS.MAGIC.length];
imageInput.readFully(magic);
if (!Arrays.equals(DDS.MAGIC, magic)) {
throw new IIOException(String.format("Not a DDS file. Expected DDS magic %s, read %s", String.format("%08x", new BigInteger(1, DDS.MAGIC)), String.format("%08x", new BigInteger(1, magic))));
Copy link
Owner

Choose a reason for hiding this comment

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

Yes, but that is 3 String.formats instead of 1...

imageio/imageio-dds/src/test/java/DDSImageTeaderTest.java Outdated Show resolved Hide resolved
@haraldk
Copy link
Owner

haraldk commented Sep 25, 2024

Thank you! Great work! I'll merge later today, when all the checks have passed and I have some spare time! 😀

@haraldk haraldk merged commit 3c01071 into haraldk:master Sep 25, 2024
18 checks passed
@p4paul
Copy link
Contributor Author

p4paul commented Sep 25, 2024

Thank you for reviewing this change and merging to main. I look forward to using it when you have time to update/release. If you need any additional help please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haraldk haraldk haraldk approved these changes

Assignees
No one assigned
Labels
New feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@p4paul @haraldk