Skip to content

Improve SkiaSharp imaging provider and bump Magick.NET#25427

Merged
EngincanV merged 8 commits into
rel-10.4from
feat/use-skiasharp-as-default-imaging
May 14, 2026
Merged

Improve SkiaSharp imaging provider and bump Magick.NET#25427
EngincanV merged 8 commits into
rel-10.4from
feat/use-skiasharp-as-default-imaging

Conversation

@maliming

@maliming maliming commented May 14, 2026

Copy link
Copy Markdown
Member

Brings the SkiaSharp imaging provider to parity with ImageSharp and Magick.NET so Account.Pro can default to it (volosoft/volo#22327).

The resizer used to ignore ImageResizeArgs.Mode and always stretch to (W, H). It now implements every mode - Stretch, Max, Min, Crop, Pad, BoxPad - and normalizes None/Default to Crop to match the ImageSharp contributor. BoxPad max-fits the source into the box when it's larger than the target.

New SkiaSharpImageCompressorContributor + SkiaSharpCompressOptions on top of the existing resizer.

A few bugs fixed along the way: resized stream position reset to 0 before return; the compressor caches the bitmap-stream length before SKBitmap.Decode disposes it (also makes it tolerate non-seekable inputs reaching the contributor directly); animated GIFs return Unsupported instead of throwing NullReferenceException.

Magick.NET-Q16-AnyCPU 14.9.1 -> 14.13.0 clears NU1901/NU1902/NU1903 advisories.

Docs updated in image-manipulation.md.

- Add SkiaSharpImageCompressorContributor and SkiaSharpCompressOptions so
  Volo.Abp.Imaging.SkiaSharp ships a compressor alongside its existing
  resizer.
- Bump Magick.NET-Q16-AnyCPU 14.9.1 -> 14.13.0 to clear the
  NU1901/NU1902/NU1903 advisories.
Copilot AI review requested due to automatic review settings May 14, 2026 05:57
@maliming maliming added the dependency-change Indicates a version change of a dependency (typically, upgrade) label May 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a SkiaSharp-based image compressor contributor (paired options class) so the SkiaSharp imaging package now ships both a resizer and a compressor, and bumps Magick.NET to clear NuGet vulnerability advisories.

Changes:

  • New SkiaSharpImageCompressorContributor + SkiaSharpCompressOptions (auto-registered via ITransientDependency) supporting JPEG/PNG/WebP.
  • xUnit tests covering JPEG/PNG/WebP compression and stream/byte[] equivalence with Quality = 50.
  • Bump Magick.NET-Q16-AnyCPU 14.9.1 → 14.13.0 in Directory.Packages.props and document it in package-version-changes.md.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
framework/src/Volo.Abp.Imaging.SkiaSharp/Volo/Abp/Imaging/SkiaSharpImageCompressorContributor.cs New compressor contributor mirroring the existing resizer/MagickNet pattern.
framework/src/Volo.Abp.Imaging.SkiaSharp/Volo/Abp/Imaging/SkiaSharpCompressOptions.cs New options class (Quality, default 75).
framework/test/Volo.Abp.Imaging.SkiaSharp.Tests/Volo/Abp/Imaging/SkiaSharpImageCompressorTests.cs Unit tests for jpg/png/webp and stream vs byte[] parity.
Directory.Packages.props Magick.NET version bump for security advisories.
docs/en/package-version-changes.md Documents the Magick.NET version bump.

@maliming maliming requested a review from EngincanV May 14, 2026 06:05
@maliming maliming added this to the 10.4-final milestone May 14, 2026
maliming added 4 commits May 14, 2026 14:20
Update the image-manipulation reference to list SkiaSharp alongside
ImageSharp and Magick.NET, and add a SkiaSharp Provider section with
installation steps and the SkiaSharpResizerOptions /
SkiaSharpCompressOptions configuration knobs.
Match the ImageSharp and Magick.NET resizer contributors which return
the result stream positioned at the start, so downstream consumers
read the bytes instead of getting an empty read at end-of-stream.
Existing tests only checked Result.Length which doesn't depend on
Position, so a contributor that forgot to seek the result stream back
to 0 would still pass. Add an explicit Position == 0 assertion plus a
CopyToAsync round-trip that compares the copied length against the
result stream length, so the same regression cannot slip in again.
@maliming maliming marked this pull request as draft May 14, 2026 06:45
@codecov

codecov Bot commented May 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.66255% with 64 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.34%. Comparing base (b982334) to head (e9318ed).
⚠️ Report is 12 commits behind head on rel-10.4.

Files with missing lines Patch % Lines
...lo/Abp/Imaging/SkiaSharpImageResizerContributor.cs 71.52% 32 Missing and 9 partials ⚠️
...Abp/Imaging/SkiaSharpImageCompressorContributor.cs 74.72% 18 Missing and 5 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           rel-10.4   #25427      +/-   ##
============================================
+ Coverage     49.30%   49.34%   +0.04%     
============================================
  Files          3668     3670       +2     
  Lines        123235   123461     +226     
  Branches       9412     9434      +22     
============================================
+ Hits          60757    60922     +165     
- Misses        60666    60724      +58     
- Partials       1812     1815       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- SkiaSharp resizer now honors ImageResizeArgs.Mode (Stretch, Max, Min,
  Crop, Pad, BoxPad) instead of stretching to the target dimensions
  regardless of mode. None and Default normalize to Crop to match the
  ImageSharp contributor.
- BoxPad now Max-fits the source into the target box first when the
  source exceeds it, instead of cropping with negative offsets.
- SkiaSharp compressor caches the bitmap stream length before
  SKBitmap.Decode takes ownership of it, so the post-encode size check
  no longer accesses a disposed stream and tolerates non-seekable
  inputs reaching the contributor directly.
- Add resizer tests for every mode (exact target size for fixed modes,
  bounded size for Max/Min, pixel-level transparency check for BoxPad
  with a source larger than the target), and a compressor test that
  feeds a non-seekable stream directly to the contributor.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

@maliming maliming changed the title Add SkiaSharp image compressor and bump Magick.NET Improve SkiaSharp imaging provider and bump Magick.NET May 14, 2026
Previously the SkiaSharp resizer ignored ImageResizeArgs.Mode entirely
and always stretched, so callers that never specified a mode were
implicitly relying on stretch output. Map None and Default to Stretch
to preserve that observable behavior while still honoring explicit
Max/Min/Crop/Pad/BoxPad values.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

@maliming maliming marked this pull request as ready for review May 14, 2026 07:31
@EngincanV EngincanV merged commit 6ad28e7 into rel-10.4 May 14, 2026
13 checks passed
@EngincanV EngincanV deleted the feat/use-skiasharp-as-default-imaging branch May 14, 2026 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

abp-framework dependency-change Indicates a version change of a dependency (typically, upgrade)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants