Skip to content

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.

Sign up for GitHub

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

Open
yeelam-gordon wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix WPF inference UI thread blocking and add busy indicator #619

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1c4ec6e
Move WPF inference to background thread via Task.Run
yeelam-gordon Apr 13, 2026
b4dbf24
Add indeterminate ProgressBar busy indicator during inference and mod…
Copilot Apr 14, 2026
181695a
Address Copilot review: disable all controls during busy, fix enable …
Copilot Apr 14, 2026
2d48d1f
Address Copilot round 2: conditional status text, disable perf radios…
Copilot Apr 14, 2026
1d931fa
Address Copilot round 3: save/restore combo states in SetBusy, preven…
Copilot Apr 14, 2026
40da524
Address Copilot round 4: dispose VideoFrame, decouple device selectio…
Copilot Apr 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Samples/WindowsML/cs-wpf/MainWindow.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@
</Grid>
</Grid>

<!-- Busy indicator (indeterminate progress bar, hidden by default) -->
<ProgressBar x:Name="BusyIndicator" Grid.Row="0" IsIndeterminate="True" Visibility="Collapsed"
Height="3" VerticalAlignment="Bottom" Margin="0,0,0,-5"/>

<!-- Image Display Row -->
<GroupBox Header="Selected Image" Grid.Row="1" Margin="0,0,0,10" MinHeight="250">
<Border BorderBrush="Gray" BorderThickness="1" Background="LightGray">
Expand Down
42 changes: 35 additions & 7 deletions Samples/WindowsML/cs-wpf/MainWindow.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ private void SelectImageButton_Click(object sender, RoutedEventArgs e)
bitmap.EndInit();
SelectedImage.Source = bitmap;

RunInferenceButton.IsEnabled = true;
RunInferenceButton.IsEnabled = _session != null;
ResultsTextBox.Text = "Image selected. Click 'Run Inference' to classify the image.";
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

SelectImageButton_Click now disables the Run Inference button when _session is null, but the status text still says "Click 'Run Inference'" even when inference can’t be run yet. Update the message to reflect whether the model is loaded (e.g., prompt to load/reload the model first when _session is null).

Suggested change
ResultsTextBox.Text = "Image selected. Click 'Run Inference' to classify the image.";
ResultsTextBox.Text = _session != null
? "Image selected. Click 'Run Inference' to classify the image."
: "Image selected. Load or reload the model first to enable inference.";

Copilot uses AI. Check for mistakes.
}
catch (Exception ex)
Expand All @@ -182,6 +182,17 @@ private void SelectImageButton_Click(object sender, RoutedEventArgs e)
}
}

private void SetBusy(bool busy)
{
BusyIndicator.Visibility = busy ? Visibility.Visible : Visibility.Collapsed;
RunInferenceButton.IsEnabled = !busy && _session != null && !string.IsNullOrEmpty(_selectedImagePath);
ReloadSessionButton.IsEnabled = !busy;
SelectImageButton.IsEnabled = !busy;
Copy link

Copilot AI Apr 14, 2026

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). Because LoadModelAndLabelsAsync() reads these values after await points, 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.

Suggested change
SelectImageButton.IsEnabled = !busy;
SelectImageButton.IsEnabled = !busy;
EpCombo.IsEnabled = !busy;
DeviceCombo.IsEnabled = !busy;
AllowProviderDownloadCheckBox.IsEnabled = !busy;

Copilot uses AI. Check for mistakes.
Comment on lines +203 to +206
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

SetBusy() computes RunInferenceButton.IsEnabled based on _session and _selectedImagePath, but SelectImageButton_Click elsewhere still sets RunInferenceButton.IsEnabled = true unconditionally. That can re-enable the button when _session is null (model not loaded) and undermines the new centralized enable/disable logic. Consider updating the image-selection path to call a single helper (e.g., SetBusy(false) / UpdateUiState()) instead of directly toggling RunInferenceButton.IsEnabled so the enabled state is consistent everywhere.

Copilot uses AI. Check for mistakes.
EpCombo.IsEnabled = !busy;
DeviceCombo.IsEnabled = !busy;
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

SetBusy(bool) unconditionally sets DeviceCombo.IsEnabled and EpCombo.IsEnabled to !busy, which can override the intended enabled/disabled state managed by PopulateDeviceCombo (e.g., when only one device type exists or when there are no devices for the selected EP). After an inference/model load completes, this can leave DeviceCombo enabled with no valid selections and cause LoadModelAndLabelsAsync to prompt for a device type that cannot be selected. Consider restoring each control to its logical enabled state (e.g., recompute from current EP/devices, or disable a parent container while busy instead of forcing individual IsEnabled values).

Suggested change
private void SetBusy(bool busy)
{
BusyIndicator.Visibility = busy ? Visibility.Visible : Visibility.Collapsed;
RunInferenceButton.IsEnabled = !busy && _session != null && !string.IsNullOrEmpty(_selectedImagePath);
ReloadSessionButton.IsEnabled = !busy;
SelectImageButton.IsEnabled = !busy;
EpCombo.IsEnabled = !busy;
DeviceCombo.IsEnabled = !busy;
private bool _epComboEnabledBeforeBusy;
private bool _deviceComboEnabledBeforeBusy;
private void SetBusy(bool busy)
{
if (busy)
{
_epComboEnabledBeforeBusy = EpCombo.IsEnabled;
_deviceComboEnabledBeforeBusy = DeviceCombo.IsEnabled;
}
BusyIndicator.Visibility = busy ? Visibility.Visible : Visibility.Collapsed;
RunInferenceButton.IsEnabled = !busy && _session != null && !string.IsNullOrEmpty(_selectedImagePath);
ReloadSessionButton.IsEnabled = !busy;
SelectImageButton.IsEnabled = !busy;
EpCombo.IsEnabled = busy ? false : _epComboEnabledBeforeBusy;
DeviceCombo.IsEnabled = busy ? false : _deviceComboEnabledBeforeBusy;

Copilot uses AI. Check for mistakes.
AllowProviderDownloadCheckBox.IsEnabled = !busy;
}
Comment on lines +200 to +226
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

SetBusy disables EP/device selection and provider download, but leaves the Perf Mode radio buttons enabled. Since Perf Mode is read during model load and affects session creation, it should also be disabled while busy to avoid users changing configuration mid-load/inference and getting inconsistent behavior.

Copilot uses AI. Check for mistakes.

private async void RunInferenceButton_Click(object sender, RoutedEventArgs e)
{
if (string.IsNullOrEmpty(_selectedImagePath) || _session == null)
Expand All @@ -192,13 +203,14 @@ 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);
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

ImageProcessor.LoadImageFileAsync returns a VideoFrame (disposable). Unlike the WinUI/WinForms versions of this sample, this handler doesn’t dispose the videoFrame, which can leak native resources/file handles across repeated inferences. Use a using/Dispose pattern for the returned VideoFrame (and ensure any downstream objects that hold it are disposed as needed).

Suggested change
var videoFrame = await ImageProcessor.LoadImageFileAsync(_selectedImagePath);
using var videoFrame = await ImageProcessor.LoadImageFileAsync(_selectedImagePath);

Copilot uses AI. Check for mistakes.
var inputTensor = await ImageProcessor.PreprocessImageAsync(videoFrame);

using var results = InferenceEngine.RunInference(_session, inputTensor);
var session = _session;
using var results = await Task.Run(() => InferenceEngine.RunInference(session, inputTensor));
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

Running inference on a background thread introduces a new lifecycle/concurrency edge case: the window can be closed while the Task.Run inference is still executing, and MainWindow.Dispose() will dispose _session concurrently with session.Run. This can lead to ObjectDisposedException or native failures. Consider preventing window close while busy, or tracking the in-flight inference task and awaiting it (or otherwise coordinating) before disposing the session/environment.

Suggested change
using var results = await Task.Run(() => InferenceEngine.RunInference(session, inputTensor));
using var results = InferenceEngine.RunInference(session, inputTensor);

Copilot uses AI. Check for mistakes.
var resultTensor = InferenceEngine.ExtractResults(_session, results);

Comment on lines +244 to 247
Copy link

Copilot AI Apr 14, 2026

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 uses AI. Check for mistakes.
var topPredictions = ResultProcessor.GetTopPredictions(resultTensor, _labels, 5);
Expand All @@ -208,6 +220,10 @@ private async void RunInferenceButton_Click(object sender, RoutedEventArgs e)
{
ResultsTextBox.Text = $"Error during inference: {ex.Message}";
}
finally
{
SetBusy(false);
}
}

private void PopulateDeviceCombo(OrtEnv env)
Expand Down Expand Up @@ -258,11 +274,23 @@ private void PopulateDeviceCombo(OrtEnv env)

private async void ReloadSessionButton_Click(object sender, RoutedEventArgs e)
{
ResultsTextBox.Text = "Loading / reloading model...";
await LoadModelAndLabelsAsync();
if (_session != null)
try
{
SetBusy(true);
Comment on lines +310 to +311
Copy link

Copilot AI Apr 14, 2026

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.

Suggested change
{
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;

Copilot uses AI. Check for mistakes.
ResultsTextBox.Text = "Loading / reloading model...";
await LoadModelAndLabelsAsync();
if (_session != null)
{
ResultsTextBox.Text += "\nModel loaded. Select an image and click 'Run Inference'.";
}
}
catch (Exception ex)
{
ResultsTextBox.Text = $"Error loading model: {ex.Message}";
}
finally
{
ResultsTextBox.Text += "\nModel loaded. Select an image and click 'Run Inference'.";
SetBusy(false);
}
}

Expand Down
Loading