-
-
Notifications
You must be signed in to change notification settings - Fork 94
Fix SkiaSharp obsolete warnings in generated output #526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7e4f2ed
cdfc0ed
496678a
568a108
7b53516
d529599
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,12 +30,12 @@ internal CanvasCommand DeepClone(CloneContext context) | |
| { | ||
| ClipPathCanvasCommand clipPathCanvasCommand => new ClipPathCanvasCommand(clipPathCanvasCommand.ClipPath?.DeepClone(context), clipPathCanvasCommand.Operation, clipPathCanvasCommand.Antialias), | ||
| ClipRectCanvasCommand clipRectCanvasCommand => new ClipRectCanvasCommand(clipRectCanvasCommand.Rect, clipRectCanvasCommand.Operation, clipRectCanvasCommand.Antialias), | ||
| DrawImageCanvasCommand drawImageCanvasCommand => new DrawImageCanvasCommand(drawImageCanvasCommand.Image?.DeepClone(context), drawImageCanvasCommand.Source, drawImageCanvasCommand.Dest, drawImageCanvasCommand.Paint?.DeepClone(context)), | ||
| DrawImageCanvasCommand drawImageCanvasCommand => new DrawImageCanvasCommand(drawImageCanvasCommand.Image?.DeepClone(context), drawImageCanvasCommand.Source, drawImageCanvasCommand.Dest, drawImageCanvasCommand.Paint?.DeepClone(context), drawImageCanvasCommand.Sampling), | ||
| DrawPictureCanvasCommand drawPictureCanvasCommand => new DrawPictureCanvasCommand(drawPictureCanvasCommand.Picture?.DeepClone(context)), | ||
| DrawPathCanvasCommand drawPathCanvasCommand => new DrawPathCanvasCommand(drawPathCanvasCommand.Path?.DeepClone(context), drawPathCanvasCommand.Paint?.DeepClone(context)), | ||
| DrawTextBlobCanvasCommand drawTextBlobCanvasCommand => new DrawTextBlobCanvasCommand(drawTextBlobCanvasCommand.TextBlob?.DeepClone(context), drawTextBlobCanvasCommand.X, drawTextBlobCanvasCommand.Y, drawTextBlobCanvasCommand.Paint?.DeepClone(context)), | ||
| DrawTextCanvasCommand drawTextCanvasCommand => new DrawTextCanvasCommand(drawTextCanvasCommand.Text, drawTextCanvasCommand.X, drawTextCanvasCommand.Y, drawTextCanvasCommand.Paint?.DeepClone(context)), | ||
| DrawTextOnPathCanvasCommand drawTextOnPathCanvasCommand => new DrawTextOnPathCanvasCommand(drawTextOnPathCanvasCommand.Text, drawTextOnPathCanvasCommand.Path?.DeepClone(context), drawTextOnPathCanvasCommand.HOffset, drawTextOnPathCanvasCommand.VOffset, drawTextOnPathCanvasCommand.Paint?.DeepClone(context)), | ||
| DrawTextCanvasCommand drawTextCanvasCommand => new DrawTextCanvasCommand(drawTextCanvasCommand.Text, drawTextCanvasCommand.X, drawTextCanvasCommand.Y, drawTextCanvasCommand.Paint?.DeepClone(context), drawTextCanvasCommand.TextAlign, drawTextCanvasCommand.Font?.DeepClone(context)), | ||
| DrawTextOnPathCanvasCommand drawTextOnPathCanvasCommand => new DrawTextOnPathCanvasCommand(drawTextOnPathCanvasCommand.Text, drawTextOnPathCanvasCommand.Path?.DeepClone(context), drawTextOnPathCanvasCommand.HOffset, drawTextOnPathCanvasCommand.VOffset, drawTextOnPathCanvasCommand.Paint?.DeepClone(context), drawTextOnPathCanvasCommand.TextAlign, drawTextOnPathCanvasCommand.Font?.DeepClone(context)), | ||
| RestoreCanvasCommand restoreCanvasCommand => new RestoreCanvasCommand(restoreCanvasCommand.Count), | ||
| SaveCanvasCommand saveCanvasCommand => new SaveCanvasCommand(saveCanvasCommand.Count), | ||
| SaveLayerCanvasCommand saveLayerCanvasCommand => new SaveLayerCanvasCommand(saveLayerCanvasCommand.Count, saveLayerCanvasCommand.Paint?.DeepClone(context)), | ||
|
|
@@ -65,17 +65,17 @@ public record ClipPathCanvasCommand(ClipPath? ClipPath, SKClipOperation Operatio | |
|
|
||
| public record ClipRectCanvasCommand(SKRect Rect, SKClipOperation Operation, bool Antialias) : CanvasCommand; | ||
|
|
||
| public record DrawImageCanvasCommand(SKImage? Image, SKRect Source, SKRect Dest, SKPaint? Paint = null) : CanvasCommand; | ||
| public record DrawImageCanvasCommand(SKImage? Image, SKRect Source, SKRect Dest, SKPaint? Paint = null, SKSamplingOptions? Sampling = null) : CanvasCommand; | ||
|
|
||
| public record DrawPictureCanvasCommand(SKPicture? Picture) : CanvasCommand; | ||
|
|
||
| public record DrawPathCanvasCommand(SKPath? Path, SKPaint? Paint) : CanvasCommand; | ||
|
|
||
| public record DrawTextBlobCanvasCommand(SKTextBlob? TextBlob, float X, float Y, SKPaint? Paint) : CanvasCommand; | ||
|
|
||
| public record DrawTextCanvasCommand(string Text, float X, float Y, SKPaint? Paint) : CanvasCommand; | ||
| public record DrawTextCanvasCommand(string Text, float X, float Y, SKPaint? Paint, SKTextAlign? TextAlign = null, SKFont? Font = null) : CanvasCommand; | ||
|
|
||
| public record DrawTextOnPathCanvasCommand(string Text, SKPath? Path, float HOffset, float VOffset, SKPaint? Paint) : CanvasCommand; | ||
| public record DrawTextOnPathCanvasCommand(string Text, SKPath? Path, float HOffset, float VOffset, SKPaint? Paint, SKTextAlign? TextAlign = null, SKFont? Font = null) : CanvasCommand; | ||
|
|
||
| public record RestoreCanvasCommand(int Count) : CanvasCommand; | ||
|
|
||
|
|
@@ -206,6 +206,11 @@ public void DrawImage(SKImage image, SKRect source, SKRect dest, SKPaint? paint | |
| AddCommand(new DrawImageCanvasCommand(image, source, dest, paint)); | ||
| } | ||
|
|
||
| public void DrawImage(SKImage image, SKRect source, SKRect dest, SKSamplingOptions sampling, SKPaint? paint = null) | ||
| { | ||
| AddCommand(new DrawImageCanvasCommand(image, source, dest, paint, sampling)); | ||
| } | ||
|
|
||
| public void DrawPicture(SKPicture picture) | ||
| { | ||
| AddCommand(new DrawPictureCanvasCommand(picture)); | ||
|
|
@@ -226,11 +231,21 @@ public void DrawText(string text, float x, float y, SKPaint paint) | |
| AddCommand(new DrawTextCanvasCommand(text, x, y, paint)); | ||
| } | ||
|
|
||
| public void DrawText(string text, float x, float y, SKTextAlign textAlign, SKFont font, SKPaint paint) | ||
| { | ||
| AddCommand(new DrawTextCanvasCommand(text, x, y, paint, textAlign, font)); | ||
|
Comment on lines
+234
to
+236
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new overload records Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in Retained text replay now consumes the recorded font and alignment state instead of relying only on the paint. The fix adds runtime For fallback/runtime APIs that still draw text through I added Validation run after the fix:
The full build passed with existing unrelated warnings only, and the full test run passed. |
||
| } | ||
|
|
||
| public void DrawTextOnPath(string text, SKPath path, float hOffset, float vOffset, SKPaint paint) | ||
| { | ||
| AddCommand(new DrawTextOnPathCanvasCommand(text, path, hOffset, vOffset, paint)); | ||
| } | ||
|
|
||
| public void DrawTextOnPath(string text, SKPath path, float hOffset, float vOffset, SKTextAlign textAlign, SKFont font, SKPaint paint) | ||
| { | ||
| AddCommand(new DrawTextOnPathCanvasCommand(text, path, hOffset, vOffset, paint, textAlign, font)); | ||
| } | ||
|
|
||
| public void SetMatrix(SKMatrix deltaMatrix) | ||
| { | ||
| TotalMatrix = TotalMatrix.PreConcat(deltaMatrix); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| // Copyright (c) Wiesław Šoltés. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for details. | ||
| using System; | ||
|
|
||
| namespace ShimSkiaSharp; | ||
|
|
||
| public enum SKFontEdging | ||
| { | ||
| Alias, | ||
| Antialias, | ||
| SubpixelAntialias | ||
| } | ||
|
|
||
| public sealed class SKFont : ICloneable, IDeepCloneable<SKFont> | ||
| { | ||
| private const float DefaultSize = 12f; | ||
| private const float DefaultScaleX = 1f; | ||
| private const float DefaultSkewX = 0f; | ||
|
|
||
| private SKTypeface? _typeface; | ||
| private float _size = DefaultSize; | ||
| private float _scaleX = DefaultScaleX; | ||
| private float _skewX = DefaultSkewX; | ||
| private bool _subpixel; | ||
| private bool _embolden; | ||
| private SKFontEdging _edging = SKFontEdging.Antialias; | ||
| private int _version; | ||
|
|
||
| public SKFont() | ||
| { | ||
| } | ||
|
|
||
| public SKFont(SKTypeface? typeface, float size = DefaultSize, float scaleX = DefaultScaleX, float skewX = DefaultSkewX) | ||
| { | ||
| _typeface = typeface; | ||
| _size = size; | ||
| _scaleX = scaleX; | ||
| _skewX = skewX; | ||
| } | ||
|
|
||
| internal int Version => _version; | ||
|
|
||
| public SKTypeface? Typeface | ||
| { | ||
| get => _typeface; | ||
| set | ||
| { | ||
| if (ReferenceEquals(_typeface, value)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _typeface = value; | ||
| _version++; | ||
| } | ||
| } | ||
|
|
||
| public float Size | ||
| { | ||
| get => _size; | ||
| set | ||
| { | ||
| if (_size.Equals(value)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _size = value; | ||
| _version++; | ||
| } | ||
| } | ||
|
|
||
| public float ScaleX | ||
| { | ||
| get => _scaleX; | ||
| set | ||
| { | ||
| if (_scaleX.Equals(value)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _scaleX = value; | ||
| _version++; | ||
| } | ||
| } | ||
|
|
||
| public float SkewX | ||
| { | ||
| get => _skewX; | ||
| set | ||
| { | ||
| if (_skewX.Equals(value)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _skewX = value; | ||
| _version++; | ||
| } | ||
| } | ||
|
|
||
| public bool Subpixel | ||
| { | ||
| get => _subpixel; | ||
| set | ||
| { | ||
| if (_subpixel == value) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _subpixel = value; | ||
| _version++; | ||
| } | ||
| } | ||
|
|
||
| public bool Embolden | ||
| { | ||
| get => _embolden; | ||
| set | ||
| { | ||
| if (_embolden == value) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _embolden = value; | ||
| _version++; | ||
| } | ||
| } | ||
|
|
||
| public SKFontEdging Edging | ||
| { | ||
| get => _edging; | ||
| set | ||
| { | ||
| if (_edging == value) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _edging = value; | ||
| _version++; | ||
| } | ||
| } | ||
|
|
||
| public SKFont Clone() => DeepClone(new CloneContext()); | ||
|
|
||
| public SKFont DeepClone() => Clone(); | ||
|
|
||
| object ICloneable.Clone() => Clone(); | ||
|
|
||
| internal SKFont DeepClone(CloneContext context) | ||
| { | ||
| if (context.TryGet(this, out SKFont existing)) | ||
| { | ||
| return existing; | ||
| } | ||
|
|
||
| var clone = new SKFont(); | ||
| context.Add(this, clone); | ||
|
|
||
| clone.Typeface = Typeface?.DeepClone(context); | ||
| clone.Size = Size; | ||
| clone.ScaleX = ScaleX; | ||
| clone.SkewX = SkewX; | ||
| clone.Subpixel = Subpixel; | ||
| clone.Embolden = Embolden; | ||
| clone.Edging = Edging; | ||
|
|
||
| return clone; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // Copyright (c) Wiesław Šoltés. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for details. | ||
|
|
||
| namespace ShimSkiaSharp; | ||
|
|
||
| public enum SKFilterMode | ||
| { | ||
| Nearest, | ||
| Linear | ||
| } | ||
|
|
||
| public enum SKMipmapMode | ||
| { | ||
| None, | ||
| Nearest, | ||
| Linear | ||
| } | ||
|
|
||
| public readonly struct SKCubicResampler | ||
| { | ||
| public static readonly SKCubicResampler Mitchell = new(1f / 3f, 1f / 3f); | ||
| public static readonly SKCubicResampler CatmullRom = new(0f, 1f / 2f); | ||
|
|
||
| public SKCubicResampler(float b, float c) | ||
| { | ||
| B = b; | ||
| C = c; | ||
| } | ||
|
|
||
| public float B { get; } | ||
|
|
||
| public float C { get; } | ||
| } | ||
|
|
||
| public readonly struct SKSamplingOptions | ||
| { | ||
| public static readonly SKSamplingOptions Default = new(); | ||
|
|
||
| public SKSamplingOptions(SKFilterMode filter, SKMipmapMode mipmap = SKMipmapMode.None) | ||
| { | ||
| Filter = filter; | ||
| Mipmap = mipmap; | ||
| Cubic = default; | ||
| UseCubic = false; | ||
| } | ||
|
|
||
| public SKSamplingOptions(SKCubicResampler cubic) | ||
| { | ||
| Filter = default; | ||
| Mipmap = default; | ||
| Cubic = cubic; | ||
| UseCubic = true; | ||
| } | ||
|
|
||
| public SKFilterMode Filter { get; } | ||
|
|
||
| public SKMipmapMode Mipmap { get; } | ||
|
|
||
| public SKCubicResampler Cubic { get; } | ||
|
|
||
| public bool UseCubic { get; } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a caller records an image with the new
DrawImage(..., SKSamplingOptions, ...)overload, the retained model storesSampling, butSkiaModel.Drawstill renders everyDrawImageCanvasCommandvia the oldskCanvas.DrawImage(image, source, dest, paint)path and never readsdrawImageCanvasCommand.Sampling(checkedsrc/Svg.Skia/SkiaModel.cslines 1845-1849). As a result, models that request cubic/linear sampling display with whatever the paint/default legacy path chooses, while codegen emits the requested sampling, so runtime rendering and generated output diverge.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in
568a1081a9bbc4720aecaa0b3507ef49330aa46f.SkiaModel.Drawnow readsDrawImageCanvasCommand.Samplingduring retained replay. BecauseSvg.Skiacurrently compiles against a SkiaSharp runtime surface whereSKCanvas.DrawImage(..., SKSamplingOptions, ...)is not available, the replay path translates the retained shimSKSamplingOptionsto the closest runtimeSKFilterQualityand applies it to a scoped clonedSKPaint. This keeps cached paints immutable and makes retained runtime rendering honor explicit recorded sampling instead of falling back to the legacy paint/default choice.I also added
SKSvgRebuildFromModelTests.ToSKPicture_UsesRecordedImageSampling, which renders a retained two-color image with explicit linear sampling and verifies blended pixels are produced.Validation run after the fix:
dotnet format Svg.Skia.slnx --no-restoredotnet build Svg.Skia.slnx -c Release --no-restore -v:minimal /nologodotnet test Svg.Skia.slnx -c Release --no-restore -v:minimal /nologoThe full build passed with existing unrelated warnings only, and the full test run passed.
Svg.Skia.UnitTestsnow reports 1848 passed and 533 skipped.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional refinement landed in
7b535163dcaa3bdb0a97d85bd62937798f1ba36a.The initial fix in
568a1081a9bbc4720aecaa0b3507ef49330aa46fmade retained replay consumeDrawImageCanvasCommand.Sampling. This follow-up removes the remaining lossy behavior for newer SkiaSharp runtimes: whenSKSamplingOptionsand the matchingSKCanvas.DrawImageoverload are present at runtime, replay now constructs the native sampling options and invokes that exact overload. TheSKFilterQualitymapping remains only as the SkiaSharp 2.88 fallback path.This was validated with the focused retained replay tests plus the full release build and full release test suite after the follow-up commit.