Conversation
--- updated-dependencies: - dependency-name: System.Configuration.ConfigurationManager dependency-version: 9.0.8 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…erToolsExamples/MarkupSimplifierApp/System.Configuration.ConfigurationManager-9.0.8 Bump System.Configuration.ConfigurationManager from 9.0.7 to 9.0.8
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 5. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v5) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
…ctions/checkout-5 Bump actions/checkout from 4 to 5
Migrate imaging from ImageSharp to SkiaSharp + add exhaustive Skia-based tests
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the ImageSharp library with SkiaSharp to address licensing restrictions. The migration switches from ImageSharp's restrictive licensing model to SkiaSharp's permissive MIT license while maintaining image processing capabilities.
Key changes include:
- Replacing ImageSharp dependencies with SkiaSharp package references
- Updating color parsing logic to use SkiaSharp's color types
- Implementing image handling with SkiaSharp's SKBitmap and SKCodec APIs
- Adding comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| OpenXmlPowerTools.csproj | Replaces ImageSharp package reference with SkiaSharp and native Linux assets |
| ImageHandler.cs | Migrates from ImageSharp's Image/IImageFormat to SkiaSharp's SKCodec for image processing |
| ColorParser.cs | Implements color parsing using System.Drawing.ColorTranslator instead of ImageSharp |
| HtmlToWmlCssParser.cs | Updates color handling to use SKColor instead of ImageSharp Color types |
| HtmlToWmlConverterCore.cs | Replaces ImageSharp image loading with SkiaSharp's SKBitmap decoding |
| OxPtHelpers.cs | Updates color hex formatting to work with SkiaSharp's color component access |
| README.md | Documents the licensing rationale for the ImageSharp to SkiaSharp migration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| using (var s = newPart.GetStream(FileMode.Create, FileAccess.ReadWrite)) | ||
| { | ||
| s.Write(ba, 0, ba.GetUpperBound(0) + 1); | ||
| s.Write(ba, 0, ba.Length); |
There was a problem hiding this comment.
The code writes ba.Length bytes, but the original code wrote ba.GetUpperBound(0) + 1 bytes. While ba.Length is correct and the original was also correct, this change could be problematic if ba is a multi-dimensional array (though unlikely in this context). Consider verifying that ba is always a single-dimensional byte array.
| var s = bmp.Size(); | ||
| Emu cx = (long)(s.Width / hres * Emu.s_EmusPerInch); | ||
| Emu cy = (long)(s.Height / vres * Emu.s_EmusPerInch); | ||
| const float dpi = 96f; |
There was a problem hiding this comment.
The hardcoded DPI value of 96f replaces the original image metadata resolution. This assumes all images have 96 DPI, which may not be accurate for all images and could affect sizing calculations. Consider making this configurable or finding a way to preserve the original image DPI information.
| using SkiaSharp; | ||
| using System; | ||
| using System.Drawing; | ||
|
|
There was a problem hiding this comment.
Adding a dependency on System.Drawing may introduce platform compatibility issues, as System.Drawing is Windows-specific and not recommended for cross-platform applications. Consider using a more portable color parsing solution or implementing custom color name parsing.
No description provided.