Esri / arcgis-maps-sdk-dotnet-samples

Sample code for ArcGIS Maps SDK for .NET – WPF, WinUI, .NET MAUI
Apache License 2.0
410 stars 518 forks source link

WPF and WinUI: Improve mouse-move behavior in OfflineRouting sample #1490

Closed mstefarov closed 1 month ago

mstefarov commented 1 month ago

Description

This sample calls an async UpdateRoute method on MouseMoved event, but does not wait for it to finish. That causes multiple parallel calls to UpdateRoute to execute at the same time -- I've seen as many as 10 in parallel. The implications of that are:

1) The sample burns system resources and can appear unresponsive 2) The shared _offlineRouteParameters are used in a thread-unsafe way 3) There is no guarantee that most-recent call to UpdateRoute is the one that completes last -- instead, whichever of the parallel calls take the longest get to update the graphics last.

To improve on this, I added some rudimentary guards against parallel execution to the samples. The sample feels much more responsive with these changes, and does not lock up any more:

https://github.com/user-attachments/assets/05a21202-1845-41f2-8e10-0496f8d1f2e0

MAUI sample did not need any changes, because it does not update route on mouse-move.

Note that most changes in the PR are due to indentation, and ignoring whitespace changes makes for a more readable diff: https://github.com/Esri/arcgis-maps-sdk-dotnet-samples/pull/1490/files?w=1

Type of change

Platforms tested on

Checklist

duffh commented 1 month ago

Definitely an improvement, I noticed a bit of an issue where when moving the mouse around and recalculating the route it occasionally throws an exception as no solution is found. This updates the error message to display an error which remains until the map is tapped.

JNhiATFb2R

It doesn't prevent using the sample, I don't think it's a major issue but we may need to revisit the error messaging.

duffh commented 1 month ago

I've removed the error messaging while the cursor is moving + the route is being calculated as I don't think it makes sense to show an error in the UI before the map has been tapped.

We still alert the user in the event a stop cannot be added so I think this change is fine to make.