Respect byte order with 16 bit tiff images #1720
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1720 +/- ##
==========================================
+ Coverage 84.34% 84.40% +0.05%
==========================================
Files 818 822 +4
Lines 35968 36056 +88
Branches 4190 4203 +13
==========================================
+ Hits 30339 30434 +95
+ Misses 4808 4802 -6
+ Partials 821 820 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| { | ||
| for (int x = left; x < left + width; x++) | ||
| { | ||
| ushort intensity = TiffUtils.ConvertToShort(data.Slice(offset, 2), this.isBigEndian); |
There was a problem hiding this comment.
Great job!
If there is insufficient performance, you can make separate loops for isBigEndian and !isBigEndian
brianpopow
changed the title
|
|
||
| l16.PackedValue = intensity; | ||
| color.FromL16(l16); | ||
| pixels[x, y] = TiffUtils.ColorFromL16(l16, intensity, color); |
There was a problem hiding this comment.
Are we using the [x, y] indexer like this elsewhere? We should process per row span to avoid the additional per-pixel multiplication.
There was a problem hiding this comment.
yeah your right. There was other places used the indexer which i have changed, too. See 0ae95f1
There was a problem hiding this comment.
weird, now a test fails only on netcore2.1: TiffDecoder_CanDecode_48Bit
There was a problem hiding this comment.
it only fails for netcore2.1 and only in Release mode, not in Debug mode.
output looks like this:
There was a problem hiding this comment.
its again the bug with the defaults i reported recently: netcore2.1 default bug
I will try to work around it, but there are several places were defaults are used in the TiffColorDecoders
There was a problem hiding this comment.
I have changed the default usages introduced with this PR with values defined in TiffUtils now to workaround the issue with netcore2.1
There was a problem hiding this comment.
Man, that bug is annoying!
| Span<TPixel> pixelRow = pixels.GetRowSpan(y); | ||
| if (this.isBigEndian) | ||
| { | ||
| for (int x = left; x < left + width; x++) |
There was a problem hiding this comment.
You can go one better for performance now you have the span and avoid the bounds checks..
Span<TPixel> pixelRow = pixels.GetRowSpan(y).Slice(left, left + width);There was a problem hiding this comment.
Im sorry, but I dont understand. Can you explain further? How would the loop change then?
There was a problem hiding this comment.
Actually sorry, I misread.
The span declaration would be:
Span<TPixel> pixelRow = pixels.GetRowSpan(y).Slice(left, width);Then the inner loop would become:
for (x = 0; x < pixelRow.Length; x++)There was a problem hiding this comment.
yeah ok, makes sense now, changed with f1834c7
Prerequisites
Description
This PR changes the tiff decoder, so it will respect the byte order for 16 bit gray tiff images and 16 bit rgb planar images.