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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for decoding tiled tiff images #2290

Merged
merged 8 commits into from Nov 9, 2022
Merged

Conversation

brianpopow
Copy link
Collaborator

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 馃懏.
  • I have provided test coverage for my change (where applicable)

Description

This PR adds support for decoding tiff images which arrange the pixel data in tiles rather then strips.

Copy link
Member

@JimBobSquarePants JimBobSquarePants left a comment

Choose a reason for hiding this comment

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

This all looks great! Nice work!

IExifValue tilesByteCountsExifValue = tags.GetValueInternal(ExifTag.TileByteCounts);
if (tilesOffsetsExifValue is null)
{
// Note: This is against the spec, but libTiff seems to handle it this way.
Copy link
Member

Choose a reason for hiding this comment

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

I've come to believe there is not a single image spec over 10 years old that hasn't been blatantly trampled on


try
{
int bytesPerTileRow = ((tileWidth * bitsPerPixel) + 7) / 8;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
int bytesPerTileRow = ((tileWidth * bitsPerPixel) + 7) / 8;
int bytesPerTileRow = (int)((((uint)tileWidth * (uint)bitsPerPixel) + 7) / 8);

C#: uglier
Asm: nicer

Or work with uint generally here (instead of int), then the casts aren't needed.

Comment on lines 614 to 615
int bytesPerRow = ((width * bitsPerPixel) + 7) / 8;
int bytesPerTileRow = ((tileWidth * bitsPerPixel) + 7) / 8;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.


int tileBufferOffset = 0;
uncompressedPixelBufferOffset += bytesPerTileRow * tileX;
int bytesToCopy = isLastHorizontalTile ? ((bitsPerPixel * remainingPixelsInRow) + 7) / 8 : bytesPerTileRow;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

}
}

int bytesPerRow = ((width * bitsPerPixel) + 7) / 8;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also use a helper-method for this, which has all the ugly casts in it, so the call-sites remain clean and descriptive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have added a helper method for this: 8fe8948

@JimBobSquarePants JimBobSquarePants merged commit a49862a into main Nov 9, 2022
@JimBobSquarePants JimBobSquarePants deleted the bp/tiledtiffs branch November 9, 2022 05:01
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.

None yet

3 participants