-
Notifications
You must be signed in to change notification settings - Fork 319
Fix WPF inference UI thread blocking and add busy indicator #619
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
base: main
Are you sure you want to change the base?
Changes from all commits
1c4ec6e
b4dbf24
181695a
2d48d1f
1d931fa
40da524
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 | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -26,14 +26,27 @@ public partial class MainWindow : Window, IDisposable | |||||||||||||||||||||||||||||||
| private OrtEnv? _ortEnv; | ||||||||||||||||||||||||||||||||
| private List<string> _labels = new(); | ||||||||||||||||||||||||||||||||
| private bool _disposed; | ||||||||||||||||||||||||||||||||
| private bool _isBusy; | ||||||||||||||||||||||||||||||||
| private bool _deviceComboWasEnabled; | ||||||||||||||||||||||||||||||||
| private bool _epComboWasEnabled; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| public MainWindow() | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| InitializeComponent(); // Must match x:Class in XAML | ||||||||||||||||||||||||||||||||
| Loaded += MainWindow_Loaded; | ||||||||||||||||||||||||||||||||
| Closing += MainWindow_Closing; | ||||||||||||||||||||||||||||||||
| Closed += MainWindow_Closed; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private void MainWindow_Closing(object? sender, System.ComponentModel.CancelEventArgs e) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| if (_isBusy) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| e.Cancel = true; | ||||||||||||||||||||||||||||||||
| ResultsTextBox.Text = "Please wait for the current operation to complete before closing."; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private void MainWindow_Closed(object? sender, EventArgs e) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| Dispose(); | ||||||||||||||||||||||||||||||||
|
|
@@ -133,11 +146,11 @@ private async Task LoadModelAndLabelsAsync() | |||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| ModelPath = "SqueezeNet.onnx", | ||||||||||||||||||||||||||||||||
| EpName = EpCombo.SelectedItem?.ToString(), | ||||||||||||||||||||||||||||||||
| DeviceType = (DeviceCombo.IsEnabled ? DeviceCombo.SelectedItem?.ToString() : null), | ||||||||||||||||||||||||||||||||
| DeviceType = (DeviceCombo.Items.Count > 1 ? DeviceCombo.SelectedItem?.ToString() : null), | ||||||||||||||||||||||||||||||||
| PerfMode = GetSelectedPerformanceMode() | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| if (DeviceCombo.IsEnabled && DeviceCombo.SelectedItem == null) | ||||||||||||||||||||||||||||||||
| if (DeviceCombo.Items.Count > 1 && DeviceCombo.SelectedItem == null) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| ResultsTextBox.Text = "Select a device type for the selected execution provider."; | ||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||
|
|
@@ -172,8 +185,10 @@ private void SelectImageButton_Click(object sender, RoutedEventArgs e) | |||||||||||||||||||||||||||||||
| bitmap.EndInit(); | ||||||||||||||||||||||||||||||||
| SelectedImage.Source = bitmap; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| RunInferenceButton.IsEnabled = true; | ||||||||||||||||||||||||||||||||
| ResultsTextBox.Text = "Image selected. Click 'Run Inference' to classify the image."; | ||||||||||||||||||||||||||||||||
| RunInferenceButton.IsEnabled = _session != null; | ||||||||||||||||||||||||||||||||
| ResultsTextBox.Text = _session != null | ||||||||||||||||||||||||||||||||
| ? "Image selected. Click 'Run Inference' to classify the image." | ||||||||||||||||||||||||||||||||
| : "Image selected. Load or reload the model first to enable inference."; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| catch (Exception ex) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
|
|
@@ -182,6 +197,34 @@ private void SelectImageButton_Click(object sender, RoutedEventArgs e) | |||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private void SetBusy(bool busy) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| _isBusy = busy; | ||||||||||||||||||||||||||||||||
| BusyIndicator.Visibility = busy ? Visibility.Visible : Visibility.Collapsed; | ||||||||||||||||||||||||||||||||
| RunInferenceButton.IsEnabled = !busy && _session != null && !string.IsNullOrEmpty(_selectedImagePath); | ||||||||||||||||||||||||||||||||
| ReloadSessionButton.IsEnabled = !busy; | ||||||||||||||||||||||||||||||||
| SelectImageButton.IsEnabled = !busy; | ||||||||||||||||||||||||||||||||
|
Comment on lines
+203
to
+206
|
||||||||||||||||||||||||||||||||
| AllowProviderDownloadCheckBox.IsEnabled = !busy; | ||||||||||||||||||||||||||||||||
| PerfModeDefaultRadio.IsEnabled = !busy; | ||||||||||||||||||||||||||||||||
| PerfModeMaxPerfRadio.IsEnabled = !busy; | ||||||||||||||||||||||||||||||||
| PerfModeMaxEffRadio.IsEnabled = !busy; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| if (busy) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| // Save combo states before disabling so we can restore them later | ||||||||||||||||||||||||||||||||
| _epComboWasEnabled = EpCombo.IsEnabled; | ||||||||||||||||||||||||||||||||
| _deviceComboWasEnabled = DeviceCombo.IsEnabled; | ||||||||||||||||||||||||||||||||
| EpCombo.IsEnabled = false; | ||||||||||||||||||||||||||||||||
| DeviceCombo.IsEnabled = false; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| // Restore the combo enabled states that PopulateDeviceCombo computed | ||||||||||||||||||||||||||||||||
| EpCombo.IsEnabled = _epComboWasEnabled; | ||||||||||||||||||||||||||||||||
| DeviceCombo.IsEnabled = _deviceComboWasEnabled; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+200
to
+226
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private async void RunInferenceButton_Click(object sender, RoutedEventArgs e) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| if (string.IsNullOrEmpty(_selectedImagePath) || _session == null) | ||||||||||||||||||||||||||||||||
|
|
@@ -192,14 +235,15 @@ private async void RunInferenceButton_Click(object sender, RoutedEventArgs e) | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| try | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| SetBusy(true); | ||||||||||||||||||||||||||||||||
| ResultsTextBox.Text = "Running inference..."; | ||||||||||||||||||||||||||||||||
| Dispatcher.Invoke(() => { }, System.Windows.Threading.DispatcherPriority.Render); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| var videoFrame = await ImageProcessor.LoadImageFileAsync(_selectedImagePath); | ||||||||||||||||||||||||||||||||
| using var videoFrame = await ImageProcessor.LoadImageFileAsync(_selectedImagePath); | ||||||||||||||||||||||||||||||||
| var inputTensor = await ImageProcessor.PreprocessImageAsync(videoFrame); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| using var results = InferenceEngine.RunInference(_session, inputTensor); | ||||||||||||||||||||||||||||||||
| var resultTensor = InferenceEngine.ExtractResults(_session, results); | ||||||||||||||||||||||||||||||||
| var session = _session; | ||||||||||||||||||||||||||||||||
| using var results = await Task.Run(() => InferenceEngine.RunInference(session, inputTensor)); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| using var results = await Task.Run(() => InferenceEngine.RunInference(session, inputTensor)); | |
| using var results = InferenceEngine.RunInference(session, inputTensor); |
Copilot
AI
Apr 14, 2026
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.
RunInferenceButton_Click captures _session into a local session for Task.Run, but then calls ExtractResults(_session, results) using the field again. If _session changes (or is disposed) across the awaited calls, results extraction can use a different session’s metadata or hit a null/disposed session. Use the same captured non-null session for both RunInference and ExtractResults, and capture it before the first await in the method to keep it consistent for the whole inference operation.
Copilot
AI
Apr 14, 2026
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.
SetBusy(true) disables DeviceCombo, but LoadModelAndLabelsAsync uses DeviceCombo.IsEnabled to decide whether to read DeviceCombo.SelectedItem (and whether to require a device selection). When ReloadSessionButton_Click calls SetBusy(true) before LoadModelAndLabelsAsync, the chosen device type will always be treated as null, which can change EP/device selection behavior. Consider capturing the EP/device selection before calling SetBusy(true), or update LoadModelAndLabelsAsync to use a separate “device selection required” signal (e.g., based on available device types / item count) rather than IsEnabled, since IsEnabled is now also used for the busy UI state.
| { | |
| SetBusy(true); | |
| { | |
| var deviceComboWasEnabled = DeviceCombo.IsEnabled; | |
| var selectedDevice = DeviceCombo.SelectedItem; | |
| SetBusy(true); | |
| // Preserve the user's device-selection state for model loading. | |
| // LoadModelAndLabelsAsync uses DeviceCombo.IsEnabled/SelectedItem to decide | |
| // whether a device selection is required and which device to use. SetBusy(true) | |
| // temporarily disables the combo for UI purposes, so restore the pre-busy state | |
| // before loading to avoid changing EP/device selection behavior. | |
| DeviceCombo.IsEnabled = deviceComboWasEnabled; | |
| DeviceCombo.SelectedItem = selectedDevice; |
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.
SetBusy()disables the three buttons but leaves other interactive controls enabled (e.g.,EpCombo,DeviceCombo,AllowProviderDownloadCheckBox). BecauseLoadModelAndLabelsAsync()reads these values afterawaitpoints, the user can change selections mid-load and end up creating a session with a different EP/device than the one they clicked with. Consider disabling those inputs while busy (or snapshotting their values before any awaits) so model load is deterministic.