Skip to content

#946: AoT compiler fixes and cleaned up AoTCompilerTools - #949

Merged
antonfirsov merged 4 commits into
SixLabors:masterfrom
dmanning23:AoT_iOS_compiler
Aug 6, 2019
Merged

antonfirsov merged 4 commits into
SixLabors:masterfrom
dmanning23:AoT_iOS_compiler

Conversation

@dmanning23

@dmanning23 dmanning23 commented Jul 19, 2019 •

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

Description

These changes update the AoTCompilerTools to fix a few issues with the iOS build.
I've verified that jpeg, gif, and resizing work on iOS with these changes.

  • Added several new virtual generic methods to the AoT compiler tools.
  • Hid the Seed methods. They never actually need to be called, the code just needs to present at compile time. This will be a breaking change for anyone currently using ImageSharp on iOS, but it will be a much better experience in the long run. This is the change proposed in Refactor AOTCompilerTools to hide implementation #800
  • Removed AoT unit tests. This code should never be called!

@antonfirsov antonfirsov left a comment

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.

You are a true hero, thanks a lot! 🎉

A few minor changes needed to make StyleCop happy though. (Normally, your local environment should report these warnings as errors on release builds.)

private static void AotCompileResizeOperations<TPixel>()
where TPixel : struct, IPixel<TPixel>
{
var resizeProcessor = new ResizeProcessor(new ResizeOptions(), new Size());

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.

new ResizeProcessor(new ResizeOptions(), default) should do the job.

@codecov

codecov Bot commented Jul 30, 2019 •

Copy link
Copy Markdown

Codecov Report

Merging #949 into master will decrease coverage by 0.17%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #949      +/-   ##
=========================================
- Coverage   89.67%   89.5%   -0.18%     
=========================================
  Files        1086    1085       -1     
  Lines       47777   47808      +31     
  Branches     3362    3362              
=========================================
- Hits        42846   42791      -55     
- Misses       4245    4325      +80     
- Partials      686     692       +6
Impacted Files Coverage Δ
...ssors/Transforms/Resize/ResizeProcessor{TPixel}.cs 98.86% <0%> (-1.14%) ⬇️
src/ImageSharp/Advanced/AotCompilerTools.cs 0% <0%> (-95.35%) ⬇️
...geSharp/PixelFormats/PixelImplementations/Bgr24.cs 87.71% <0%> (-8.78%) ⬇️
src/ImageSharp/Advanced/AdvancedImageExtensions.cs 76.47% <0%> (-5.89%) ⬇️
src/ImageSharp/Formats/Png/PngEncoderCore.cs 93.26% <0%> (-0.81%) ⬇️
...ssors/Quantization/OctreeFrameQuantizer{TPixel}.cs 98.07% <0%> (-0.65%) ⬇️
...rocessors/Quantization/WuFrameQuantizer{TPixel}.cs 91.93% <0%> (-0.47%) ⬇️
src/ImageSharp/Formats/Gif/GifDecoderCore.cs 79.82% <0%> (-0.43%) ⬇️

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 7196b43...f532347. Read the comment docs.

@antonfirsov

Copy link
Copy Markdown
Member

I'm merging this, we can do the rest of #946 in follow-up PR-s.

@antonfirsov
antonfirsov merged commit 3f3b7ab into SixLabors:master Aug 6, 2019
@JimBobSquarePants

Copy link
Copy Markdown
Member

Thanks so much for your help here @dmanning23 !!

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.

3 participants