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

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

Merged
merged 3 commits into from
Jul 24, 2024

Conversation

mstefarov
Copy link
Contributor

@mstefarov mstefarov commented Jul 23, 2024

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:

2024-07-22_123513.ArcGIS.WinUI.Viewer.mp4

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

  • Bug fix

Platforms tested on

  • WPF .NET 8
  • WPF Framework
  • WinUI
  • MAUI WinUI
  • MAUI Android
  • MAUI iOS
  • MAUI MacCatalyst

Checklist

  • Self-review of changes
  • All changes work as expected on all affected platforms
  • There are no warnings related to changes
  • Code is commented and follows .NET conventions and standards
  • Codemaid and XAML styler extensions have been run on every changed file
  • No unrelated changes have been made to any other code or project files
  • Screenshots are correct size and display in description tab (800 x 600 on Windows, MAUI mobile platforms use the MAUI Windows screenshot)

@mstefarov mstefarov self-assigned this Jul 23, 2024
Copy link
Collaborator

@williambohrmann3 williambohrmann3 left a comment

Choose a reason for hiding this comment

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

This is a great change. On main branch right now, when you get an exception, you have to reset the sample to calculate routes. You've resolved this bug with your fix and made the sample more performant. 👍 Also thanks for adding a link to the no whitespace view, I did not realize that was a thing on GitHub!

@duffh
Copy link
Collaborator

duffh commented Jul 24, 2024

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
Copy link
Collaborator

duffh commented Jul 24, 2024

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.

@mstefarov mstefarov merged commit 39c8c4c into main Jul 24, 2024
6 checks passed
@mstefarov mstefarov deleted the matvei/offline-route-parallelism branch July 24, 2024 17:33
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.

3 participants