Skip to content

Conversation

@brianpopow
Copy link
Collaborator

@brianpopow brianpopow commented Aug 4, 2021

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) note: Test only use DebugSave, because reference decoder does not work with 24/32 bit. output looks ok, tough

Description

This PR adds support for the following tiff images:

  • Add support for decoding gray 24 bit tiff's
  • Add support for decoding gray 32 bit tiff's
  • Add support for decoding 24bit per channel color tiff with contiguous pixel data
  • Add support for decoding 32bit per channel color tiff with contiguous pixel data
  • Add support for decoding 24bit per channel color tiff with planar pixel data
  • Add support for decoding 32bit per channel color tiff with planar pixel data

public void TiffDecoder_CanDecode_96Bit<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
// Note: because the MagickReferenceDecoder fails to load the image, we only debug save them.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Magick.Net can actually decode the images, but in MagickReferenceDecoder I am not sure how to correctly transfer the pixel byte data from Magick.Net to our format.

Copy link
Member

Choose a reason for hiding this comment

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

@dlemstra Would you be able to assist here?

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 will go ahead and merge this now. It would be still useful to know if we can use Magick.Net for images with color channel bit depth >= 24. We can add this in a future PR, if it is possible.

@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #1724 (51e1870) into master (566babf) will increase coverage by 0.02%.
The diff coverage is 88.49%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1724      +/-   ##
==========================================
+ Coverage   84.37%   84.40%   +0.02%     
==========================================
  Files         822      830       +8     
  Lines       36033    36262     +229     
  Branches     4200     4238      +38     
==========================================
+ Hits        30404    30607     +203     
- Misses       4814     4839      +25     
- Partials      815      816       +1     
Flag Coverage Δ
unittests 84.40% <88.49%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...icInterpretation/WhiteIsZero24TiffColor{TPixel}.cs 0.00% <0.00%> (ø)
...mageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs 64.88% <74.07%> (+3.41%) ⬆️
...cInterpretation/TiffColorDecoderFactory{TPixel}.cs 77.41% <88.88%> (+2.41%) ⬆️
...icInterpretation/BlackIsZero16TiffColor{TPixel}.cs 100.00% <100.00%> (ø)
...icInterpretation/BlackIsZero24TiffColor{TPixel}.cs 100.00% <100.00%> (ø)
...icInterpretation/BlackIsZero32TiffColor{TPixel}.cs 100.00% <100.00%> (ø)
...metricInterpretation/Rgb161616TiffColor{TPixel}.cs 100.00% <100.00%> (ø)
...tricInterpretation/Rgb16PlanarTiffColor{TPixel}.cs 100.00% <100.00%> (ø)
...metricInterpretation/Rgb242424TiffColor{TPixel}.cs 100.00% <100.00%> (ø)
...tricInterpretation/Rgb24PlanarTiffColor{TPixel}.cs 100.00% <100.00%> (ø)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 566babf...51e1870. Read the comment docs.

@brianpopow brianpopow changed the title WIP: Add support for decoding 24 and 32 bit tiff images Add support for decoding 24 and 32 bit tiff images Aug 4, 2021
{
for (int x = 0; x < pixelRow.Length; x++)
{
data.Slice(offset, 3).CopyTo(buffer.AsSpan(bufferStartIdx));
Copy link
Contributor

Choose a reason for hiding this comment

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

small improvement, you can save buffer.AsSpan(bufferStartIdx) to variable before loops

Copy link
Member

Choose a reason for hiding this comment

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

I think that would be a more significant speed up than you think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed with 4e5dec9

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.

LGTM Great work! 👍

public void TiffDecoder_CanDecode_96Bit<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
// Note: because the MagickReferenceDecoder fails to load the image, we only debug save them.
Copy link
Member

Choose a reason for hiding this comment

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

@dlemstra Would you be able to assist here?

# Conflicts:
#	src/ImageSharp/Formats/Tiff/PhotometricInterpretation/BlackIsZero16TiffColor{TPixel}.cs
#	src/ImageSharp/Formats/Tiff/PhotometricInterpretation/Rgb161616TiffColor{TPixel}.cs
@brianpopow brianpopow merged commit 6a388c8 into master Aug 6, 2021
@brianpopow brianpopow deleted the bp/tiff24bit branch August 6, 2021 17:30
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