Skip to content

Refactor JpegDecoder to use Buffers. - #210

Merged
JimBobSquarePants merged 3 commits into
SixLabors:masterfrom
firenero:buffer-192
May 8, 2017
Merged

JimBobSquarePants merged 3 commits into
SixLabors:masterfrom
firenero:buffer-192

Conversation

@firenero

@firenero firenero commented May 6, 2017

Copy link
Copy Markdown
Contributor

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 matches the existing coding patterns and practise as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Remove DecodedBlockArray struct and replace it usages with new Buffer<T>. One of the cases in #192

@codecov-io

codecov-io commented May 6, 2017 •

Copy link
Copy Markdown

Codecov Report

Merging #210 into master will decrease coverage by 0.02%.
The diff coverage is 85.1%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #210      +/-   ##
==========================================
- Coverage   89.91%   89.89%   -0.03%     
==========================================
  Files         502      501       -1     
  Lines       22339    22324      -15     
  Branches     1563     1564       +1     
==========================================
- Hits        20087    20069      -18     
- Misses       1828     1830       +2     
- Partials      424      425       +1
Impacted Files Coverage Δ
...Formats/Jpeg/Components/Decoder/JpegScanDecoder.cs 73.14% <100%> (ø) ⬆️
...harp/Formats/Jpeg/Components/Decoder/YCbCrImage.cs 87.09% <100%> (ø) ⬆️
...mats/Jpeg/Components/Decoder/JpegBlockProcessor.cs 100% <100%> (ø) ⬆️
...ts/ImageSharp.Tests/Formats/Jpg/YCbCrImageTests.cs 88.23% <100%> (ø) ⬆️
src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs 62.02% <76.92%> (+0.15%) ⬆️
...p/Formats/Jpeg/Components/Decoder/JpegPixelArea.cs 94.11% <83.33%> (-1.72%) ⬇️
tests/ImageSharp.Tests/TestFileSystem.cs 63.63% <0%> (-13.64%) ⬇️

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 da8c233...c97cbb6. Read the comment docs.

byte[] pixels = BytePool.Rent(size);
return new JpegPixelArea(pixels, width, 0);
}
public static JpegPixelArea CreatePooled(int width, int height) => new JpegPixelArea(Buffer2D<byte>.CreateClean(width, height), width, 0);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should remove CreatePooled() / ReturnPooled() entirely, because this is an ownership/lifecycle concern. JpegDecoderCore and YCbCrImage should own (create + dispose) Buffer2D<byte> instead.
JpegPixelArea has to be a temporal struct to manage a (sub)area of pixels owned by Buffer2D<byte>.

@antonfirsov

Copy link
Copy Markdown
Member

Looks good now, thanks a lot @firenero!

Has anyone any concerns, or can I merge this now? @JimBobSquarePants?

@JimBobSquarePants

Copy link
Copy Markdown
Member

I'll merge it now. Thanks @firenero really appreciate your help! 👍

@JimBobSquarePants
JimBobSquarePants merged commit e9e37ec into SixLabors:master May 8, 2017
@firenero
firenero deleted the buffer-192 branch May 9, 2017 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants