-
-
Notifications
You must be signed in to change notification settings - Fork 348
Update SkiaSharp to 3.119, Svg.Skia to 3.0.0, HarfBuzzSharp to 8.3.1.1, Uno to 6.0.130 and CommunityToolkit.Mvvm to 8.4.0 #3031
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
4f2f31f
8bee2b8
ca4384b
f4043cf
dc23ced
8fca0c1
87ab34f
02ae4fe
7d01c9d
d98b01e
445787b
46f0712
9c5dc99
d0c3751
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 |
|---|---|---|
|
|
@@ -41,8 +41,6 @@ | |
|
|
||
| public MapControl() | ||
| { | ||
| SharedConstructor(); | ||
|
|
||
| // The commented out code crashes the app when MouseWheelAnimation.Duration > 0. Could be a bug in SKXamlCanvas | ||
| //if (Dispatcher.HasThreadAccess) _canvas?.Invalidate(); | ||
| //else RunOnUIThread(() => _canvas?.Invalidate()); | ||
|
|
@@ -62,6 +60,9 @@ | |
| _canvas.PaintSurface += Canvas_PaintSurface; | ||
| } | ||
|
|
||
| // The Canvas needs to be first set before calling the Shared Constructor or else it crashes in the InvalidateCanvas | ||
| SharedConstructor(); | ||
|
Contributor
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. I had to move it here or it crashed on my computer with _canvas not initilized in InitCanvas(); I think this could be a problem on other platforms too. But I think this should be then a seperate pull request.
Member
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. Yes it is. I was just looking into this because of another bug report I got. |
||
|
|
||
| Children.Add(_selectRectangle); | ||
|
|
||
| Loaded += MapControlLoaded; | ||
|
|
@@ -304,7 +305,7 @@ | |
| GC.SuppressFinalize(this); | ||
| } | ||
| #elif HAS_UNO && __IOS__ // on ios don't dispose _canvas, _canvasGPU, _selectRectangle, base class | ||
| protected new virtual void Dispose(bool disposing) | ||
|
Check warning on line 308 in Mapsui.UI.WinUI/MapControl.cs
|
||
| { | ||
| SharedDispose(disposing); | ||
| } | ||
|
|
@@ -313,22 +314,14 @@ | |
| { | ||
| GC.SuppressFinalize(this); | ||
| } | ||
| #else | ||
| #if __ANDROID__ | ||
|
Member
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. Ghd #if ANDROID is not relevant anymore. Why?
Contributor
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. The dispose in Android is now too only Dispose() without (boolean) Parameter so making an #if Android compilation made no sense anymore because it is the same code as the #else case. |
||
| protected new virtual void Dispose(bool disposing) | ||
| { | ||
| CommonUnoDispose(disposing); | ||
| SharedDispose(disposing); | ||
| base.Dispose(disposing); | ||
| } | ||
| #else | ||
| protected virtual void Dispose(bool disposing) | ||
| { | ||
| CommonUnoDispose(disposing); | ||
| SharedDispose(disposing); | ||
| base.Dispose(); | ||
| } | ||
| #endif | ||
|
|
||
| public new void Dispose() | ||
| { | ||
| Dispose(true); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,6 @@ | |
| "rollForward": "disable" | ||
| }, | ||
| "msbuild-sdks": { | ||
| "Uno.Sdk": "5.5.56" | ||
| "Uno.Sdk": "6.0.130" | ||
| } | ||
| } | ||
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.
What does the UnoFeatures SkiaRenderer do? It uses SkiaRendering instead of something else? Which platforms does it affect?
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.
SkiaRenderer is now the default on Uno 6.0. I'll investigate if this is needed in the Control. My guess is that this setting only works on an WinUI Application, because only in the WinUI Sample the references changed when I enabled this. But nothing changed in the MapControl.
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.
This is needed in the Control too, It adds some more dependencies (But only in the Wasm Targets)
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.
I don't think you should be using SkiaRenderer here - it will make it stop working on native renderers. The correct thing to do is leave it off unless you only want to support skia.
Uh oh!
There was an error while loading. Please reload this page.
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.
Note that
#if ANDROIDblocks won't have any effect when a non-skia library is used on skia renderer app though.LiveCharts2 renders using skia and it works on both native and skia renderers, might be worth having a look at what they did?
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.
@mikernet There
#if ANDROIDwas removed, so that is a good thing?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.
To 'support skia' means that the entire interface is rendered to a skia canvas, right? And from your url I understand that this is the default for Android and iOS fro Uno 6, but apparently it is just an option. So, the question is what Mapsui should support, or what it needs to do to support both.
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.
@pauldendulk Sorry, that was slightly confusing phrasing from me. For a library to support both uno native and uno skia renderers (where the whole app is rendered to a skia canvas), the library itself should not use the
SkiaRendererfeature. The controls in the library itself can render to a skia surface I believe, as apparently evidenced by LiveCharts2 working fine on both, but not sure what caveats that entails.