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)

Description

Fixes a bug calculating the stripIndex for planar tiff images.

example strip offsets and byte counts from flower-rgb-planar-08-6strips.tiff (tiffinfo -s)

6 Strips:
            offsets, byte count
      0: [       8,     2190]
      1: [    2198,      949]
      2: [    3147,     2190]
      3: [    5337,      949]
      4: [    6286,     2190]
      5: [    8476,      949]

@IldarKhayrutdinov could you please review this change when you have time?

@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #1723 (4f19692) into master (83427f2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1723   +/-   ##
=======================================
  Coverage   84.36%   84.36%           
=======================================
  Files         822      822           
  Lines       36022    36023    +1     
  Branches     4204     4204           
=======================================
+ Hits        30391    30392    +1     
  Misses       4816     4816           
  Partials      815      815           
Flag Coverage Δ
unittests 84.36% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs 88.70% <100.00%> (+0.09%) ⬆️

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 83427f2...4f19692. Read the comment docs.

@IldarKhayrutdinov
Copy link
Contributor

@brianpopow yeah, it seems there was a bug, now looks good!

@brianpopow
Copy link
Collaborator Author

yeah, it seems there was a bug, now looks good!

@IldarKhayrutdinov ok, thanks for taking a look!

[Theory]
[WithFile(FlowerRgb888Planar6Strips, PixelTypes.Rgba32)]
[WithFile(FlowerRgb888Planar15Strips, PixelTypes.Rgba32)]
public void TiffDecoder_Planar<TPixel>(TestImageProvider<TPixel> provider)
Copy link
Contributor

Choose a reason for hiding this comment

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

new test files are good,
we needs to add more test files for tiff

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we had test files for planar, but only with 3 strips, so the bug was not triggered.

@brianpopow brianpopow merged commit 7da6c33 into master Aug 4, 2021
@brianpopow brianpopow deleted the bp/fixPlanarDecoding branch August 4, 2021 16:02
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.

3 participants