Skip to content
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

Made GetTile Async for fixing Wasm #169

Merged
merged 11 commits into from
Mar 9, 2022

Conversation

inforithmics
Copy link
Contributor

No description provided.

@inforithmics inforithmics changed the title (WIP) Made GetTile Async for fixing Wasm Made GetTile Async for fixing Wasm Mar 4, 2022
@inforithmics
Copy link
Contributor Author

To fix The appveyor Build the appveyor.yml needs to be included in the repository so I could fix it.

@inforithmics
Copy link
Contributor Author

Changes:

  1. Changed GetTile -> GetTileAsync
  2. Added Uno.Wasm and Uno.UWP Sample for Debugging Wasm.
  3. Change the Fetcher to be able to run on Wasm. (Replaced AutoManualResetEvent -> SemaphoreSlim)

Copy link
Contributor

@pauldendulk pauldendulk left a comment

Choose a reason for hiding this comment

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

In general great. Please take a look at my comments. And also, I really really want to keep the amount of code we need to maintain small.

Perhaps we could reference the Mapsui nuget (or perhaps we already have a reference).

@@ -27,6 +27,7 @@
<s:Boolean x:Key="/Default/Environment/InjectedLayers/FileInjectedLayer/=712F7012C17E2549846B478E6E138577/@KeyIndexDefined">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/InjectedLayers/InjectedLayerCustomization/=File712F7012C17E2549846B478E6E138577/@KeyIndexDefined">True</s:Boolean>
<s:Double x:Key="/Default/Environment/InjectedLayers/InjectedLayerCustomization/=File712F7012C17E2549846B478E6E138577/RelativePriority/@EntryValue">2</s:Double>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EdotCover_002EIde_002ECore_002EFilterManagement_002EModel_002ESolutionFilterSettingsManagerMigrateSettings/@EntryIndexedValue">True</s:Boolean>
Copy link
Contributor

Choose a reason for hiding this comment

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

I did not know you used ReSharper?

NuGet.config Outdated
@@ -6,8 +6,8 @@
<packageSources>
<clear />
<add key="nuget.org" value="https://api.nuget.org/v3/index.json" />
<add key="dotnet-core" value="https://dotnetfeed.blob.core.windows.net/dotnet-core/index.json" />
<!--<add key="dotnet-core" value="https://dotnetfeed.blob.core.windows.net/dotnet-core/index.json" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Did this cause a problem for you? Should it be removed completely?

@@ -0,0 +1,192 @@
using System;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is so much code. This needs to be maintained. Could the Mapsui package itself be used to present these samples? (needs to be released first).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I think this would be better, and making it a simpler example. I will slim down this pull request, for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also not sure if we need a sample. For your development it was good to see wasm now actually worked, but now it does do we still need it? Mapsui already has a dependency on Uno, so you could show how it works in there. Is there another benefit of the sample that could not be shown in the other samples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then I remove the Uno Sample, because it works better in Mapsui (probably not tested yet)

@inforithmics inforithmics changed the title Made GetTile Async for fixing Wasm (WIP) Made GetTile Async for fixing Wasm Mar 6, 2022
@inforithmics inforithmics changed the title (WIP) Made GetTile Async for fixing Wasm Made GetTile Async for fixing Wasm Mar 8, 2022
Copy link
Contributor

@pauldendulk pauldendulk left a comment

Choose a reason for hiding this comment

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

Thanks a lot. This is an important improvement.

@@ -18,12 +17,11 @@ public class Fetcher<T>
private double _unitsPerPixel;
Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about the BlockingCollection for fetching (mentioned in the Mapsui 'about events' developers post). Perhaps this sample is a good place to try that out.

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.

None yet

2 participants