diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 205a9a3..4fec3f8 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -41,7 +41,9 @@ jobs: - uses: microsoft/setup-WinAppCli@b93bbddc1f7abc061ca0d3a8119e3a0c7dd71495 with: version: v0.3.1 - - run: dotnet restore Snaply.slnx --locked-mode + # Restore with ReadyToRun on so the crossgen2 runtime pack is fetched here; the + # publish below runs --no-restore, so without it R2R fails (NETSDK1094). + - run: dotnet restore Snaply.slnx --locked-mode -p:PublishReadyToRun=true - run: dotnet test tests/Snaply.Tests/Snaply.Tests.csproj -c Release --no-restore - if: matrix.architecture == 'x64' run: dotnet test tests/Snaply.App.Tests/Snaply.App.Tests.csproj -c Release --no-restore @@ -55,10 +57,19 @@ jobs: --self-contained true -o build/native --no-restore - - name: Repeat UI journeys five times + # UI journeys are x64-only. On windows-11-arm the shell keeps the foreground for + # itself — the image boots with a Microsoft-account sign-in prompt (WWAHost), and + # closing it just hands the foreground to SearchHost — so the capture overlay never + # comes forward and synthetic input never reaches it. Even the AttachThreadInput + # handoff in ui-tests.ps1 loses that fight there, so the journeys measured the + # runner image rather than Snaply. arm64 still builds, unit-tests and publishes. + # Two passes, not five: x64 clears two reliably, and beyond that the same + # foreground contention starts costing passes. See the tracking issue. + - name: Repeat UI journeys twice + if: matrix.architecture == 'x64' shell: pwsh run: | - 1..5 | ForEach-Object { + 1..2 | ForEach-Object { $process = Start-Process build/native/Snaply.exe -PassThru try { ./src/Snaply.App/ui-tests.ps1 ` @@ -74,6 +85,7 @@ jobs: } } - name: Run 100-capture soak + if: matrix.architecture == 'x64' shell: pwsh run: | $process = Start-Process build/native/Snaply.exe -PassThru @@ -93,8 +105,38 @@ jobs: Stop-Process -Id $process.Id -Force } } + # A UI failure otherwise reports only "element did not appear"; Snaply's own log + # carries the exception behind it, so ship it alongside the results. + - name: Collect app logs + if: always() && matrix.architecture == 'x64' + shell: pwsh + run: | + $logs = Join-Path $env:LOCALAPPDATA 'Snaply\Logs' + # An empty folder is dropped by upload-artifact, so say out loud which case + # this is: no folder means the app never got as far as configuring logging, + # an empty one means it ran and logged nothing. + if (-not (Test-Path $logs)) { + Write-Host "No log directory at $logs." + exit 0 + } + $files = @(Get-ChildItem $logs -File) + Write-Host "Log directory holds $($files.Count) file(s)." + foreach ($file in $files) { + Write-Host "--- $($file.Name) ---" + Get-Content $file.FullName | Write-Host + } + New-Item -ItemType Directory -Force -Path artifacts/ui/app-logs | Out-Null + Copy-Item "$logs\*" artifacts/ui/app-logs -Recurse -Force + # Only reached when everything gating this architecture passed: on x64 that includes + # the UI journeys and the soak, on arm64 the build and the unit tests. - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 - if: always() + with: + name: nightly-${{ matrix.architecture }} + path: build/native + retention-days: 14 + if-no-files-found: error + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: always() && matrix.architecture == 'x64' with: name: ui-${{ matrix.architecture }} path: artifacts/ui diff --git a/src/Snaply.App/MainPage.xaml b/src/Snaply.App/MainPage.xaml index 7e3a2ec..922efdc 100644 --- a/src/Snaply.App/MainPage.xaml +++ b/src/Snaply.App/MainPage.xaml @@ -71,7 +71,7 @@ + Command="{x:Bind ViewModel.CaptureCommand}"> + IsActive="{x:Bind ViewModel.CaptureCommand.IsRunning, Mode=OneWay}" /> diff --git a/src/Snaply.App/MainPage.xaml.cs b/src/Snaply.App/MainPage.xaml.cs index bb5f0af..fe0412b 100644 --- a/src/Snaply.App/MainPage.xaml.cs +++ b/src/Snaply.App/MainPage.xaml.cs @@ -11,10 +11,6 @@ public sealed partial class MainPage : Page private const int WindowGlyph = 0xE737; private const int DesktopGlyph = 0xE7F4; - // The mode the Capture pill runs. The flyout only changes this (it never captures on its own); - // pressing the pill body captures with it. Defaults to the whole desktop. - private CaptureMode _selectedMode = CaptureMode.Desktop; - internal MainPage(MainViewModel viewModel) { ViewModel = viewModel; @@ -39,11 +35,8 @@ internal MainPage(MainViewModel viewModel) internal MainViewModel ViewModel { get; } - // Capture pill body: run the currently selected mode. - private async void CaptureButton_Click(SplitButton sender, SplitButtonClickEventArgs args) => - await ViewModel.CaptureAsync(_selectedMode); - - // Flyout items: change the selected mode only (the capture happens on the pill body click). + // Flyout items: change the selected mode only. The pill body is bound to CaptureCommand, + // which runs whichever mode is selected. private void RegionCaptureItem_Click(object sender, RoutedEventArgs args) => SelectMode(CaptureMode.Region); private void WindowCaptureItem_Click(object sender, RoutedEventArgs args) => SelectMode(CaptureMode.Window); @@ -54,7 +47,7 @@ private async void CaptureButton_Click(SplitButton sender, SplitButtonClickEvent private void SelectMode(CaptureMode mode) { - _selectedMode = mode; + ViewModel.SelectedMode = mode; UpdatePrimaryCapture(); } @@ -62,13 +55,17 @@ private void SelectMode(CaptureMode mode) // view model stays free of presentation strings. private void UpdatePrimaryCapture() { - PrimaryCaptureLabel.Text = ResourceText.Get(_selectedMode switch + string label = ResourceText.Get(ViewModel.SelectedMode switch { CaptureMode.Region => "CaptureRegion", CaptureMode.Window => "CaptureWindow", _ => "CaptureDesktop", }); - PrimaryCaptureGlyph.Glyph = char.ConvertFromUtf32(_selectedMode switch + PrimaryCaptureLabel.Text = label; + // The pill's content is a panel, so it derives no automation name of its own and + // screen readers announce it unnamed. Name it after the mode it will run. + AutomationProperties.SetName(CaptureButton, label); + PrimaryCaptureGlyph.Glyph = char.ConvertFromUtf32(ViewModel.SelectedMode switch { CaptureMode.Region => RegionGlyph, CaptureMode.Window => WindowGlyph, diff --git a/src/Snaply.App/ViewModels/MainViewModel.cs b/src/Snaply.App/ViewModels/MainViewModel.cs index 0309533..2e25fa1 100644 --- a/src/Snaply.App/ViewModels/MainViewModel.cs +++ b/src/Snaply.App/ViewModels/MainViewModel.cs @@ -1,5 +1,6 @@ using System.Runtime.InteropServices.WindowsRuntime; using CommunityToolkit.Mvvm.ComponentModel; +using CommunityToolkit.Mvvm.Input; using Microsoft.UI.Xaml.Media.Imaging; using Serilog; using Windows.Graphics.Imaging; @@ -16,8 +17,9 @@ internal sealed partial class MainViewModel : ObservableObject, IDisposable [ObservableProperty] internal partial WriteableBitmap? Preview { get; set; } + // The capture pill picks the mode; CaptureCommand runs whatever is selected. [ObservableProperty] - internal partial bool IsBusy { get; set; } + internal partial CaptureMode SelectedMode { get; set; } = CaptureMode.Desktop; [ObservableProperty] internal partial bool HasImage { get; set; } @@ -41,24 +43,19 @@ internal MainViewModel( _export = export; } - internal CaptureMode LastCaptureMode { get; private set; } = CaptureMode.Region; - - internal async Task CaptureAsync(CaptureMode mode) + // AsyncRelayCommand refuses to run while an execution is in flight and reports that + // through CanExecute, so the bound pill disables itself for the duration and the view + // needs no separate busy flag or re-entrancy guard. + [RelayCommand] + private async Task CaptureAsync() { - if (IsBusy) - { - return; - } - - LastCaptureMode = mode; HasError = false; - IsBusy = true; using var operation = new CancellationTokenSource(); _operation = operation; try { - using CapturedFrame? frame = await _capture.CaptureAsync(mode, operation.Token); + using CapturedFrame? frame = await _capture.CaptureAsync(SelectedMode, operation.Token); if (frame is null) { return; @@ -92,8 +89,6 @@ internal async Task CaptureAsync(CaptureMode mode) { _operation = null; } - - IsBusy = false; } } diff --git a/src/Snaply.App/ui-tests.ps1 b/src/Snaply.App/ui-tests.ps1 index fdf194b..983d0b9 100644 --- a/src/Snaply.App/ui-tests.ps1 +++ b/src/Snaply.App/ui-tests.ps1 @@ -22,6 +22,7 @@ param( $ErrorActionPreference = 'Stop' $results = [System.Collections.Generic.List[object]]::new() +$diagnosticCount = 0 $artifacts = Join-Path $PSScriptRoot '..\..\artifacts\ui' New-Item -ItemType Directory -Force -Path $artifacts | Out-Null $artifacts = (Resolve-Path $artifacts).Path @@ -102,30 +103,66 @@ public static class WindowSizing [DllImport("user32.dll", SetLastError = true)] public static extern uint SendInput(uint count, Input[] inputs, int size); - public static bool SendMouse(uint flags) + [DllImport("user32.dll")] + private static extern bool SetForegroundWindow(IntPtr hWnd); + + [DllImport("user32.dll")] + private static extern bool BringWindowToTop(IntPtr hWnd); + + [DllImport("user32.dll")] + private static extern bool AttachThreadInput(uint attach, uint attachTo, bool join); + + [DllImport("kernel32.dll")] + private static extern uint GetCurrentThreadId(); + + // SetForegroundWindow alone is refused when another process owns the foreground, which + // is the normal state on these runners — the shell (WWAHost, SearchHost) keeps taking + // it. Attaching our input queue to both the current foreground thread and the target's + // lifts that restriction for the duration of the call, which is the documented way to + // hand the foreground to a specific window. + public static bool ForceForeground(IntPtr hWnd) { - Input[] inputs = + IntPtr foreground = GetForegroundWindow(); + if (foreground == hWnd) { - new Input + return true; + } + + uint ignored; + uint foregroundThread = GetWindowThreadProcessId(foreground, out ignored); + uint targetThread = GetWindowThreadProcessId(hWnd, out ignored); + uint currentThread = GetCurrentThreadId(); + bool attachedForeground = foregroundThread != 0 && foregroundThread != currentThread + && AttachThreadInput(currentThread, foregroundThread, true); + bool attachedTarget = targetThread != 0 && targetThread != currentThread + && AttachThreadInput(currentThread, targetThread, true); + try + { + BringWindowToTop(hWnd); + return SetForegroundWindow(hWnd); + } + finally + { + if (attachedTarget) { - type = 0, - data = new InputUnion { mouse = new MouseInput { flags = flags } } + AttachThreadInput(currentThread, targetThread, false); } - }; - return SendInput(1, inputs, Marshal.SizeOf()) == 1; + + if (attachedForeground) + { + AttachThreadInput(currentThread, foregroundThread, false); + } + } } - public static bool MoveMouse(int dx, int dy) + public static bool SendMouse(uint flags) { Input[] inputs = { new Input { type = 0, - data = new InputUnion - { - mouse = new MouseInput { dx = dx, dy = dy, flags = 0x0001 } - } + data = new InputUnion { mouse = new MouseInput { flags = flags } } } }; return SendInput(1, inputs, Marshal.SizeOf()) == 1; @@ -232,6 +269,63 @@ function Get-RegionSelectionWindow { throw 'No region selection window appeared.' } +# Synthetic mouse and keyboard input is delivered to the foreground window, but +# BeginSelection returns as soon as it has called Activate(), so UI Automation can see +# the overlay before it can accept input. Anything driving the overlay with SendInput or +# SetCursorPos has to wait for it to actually reach the foreground first. +function Wait-RegionOverlayForeground { + $mainHandle = [IntPtr](Get-AppWindow).Current.NativeWindowHandle + $deadline = [DateTime]::UtcNow.AddSeconds(5) + do { + $foreground = [WindowSizing]::GetForegroundWindow() + $foregroundProcess = [uint32]0 + $null = [WindowSizing]::GetWindowThreadProcessId( + $foreground, + [ref]$foregroundProcess) + if ($foreground -ne [IntPtr]::Zero -and + $foreground -ne $mainHandle -and + $foregroundProcess -eq $AppPid) { + return $foreground + } + + # The shell keeps grabbing the foreground on these runners, and the app cannot + # activate over another process that holds it. Hand it to the overlay explicitly + # rather than waiting for a window that will never come forward on its own. + try { + $overlayHandle = [IntPtr](Get-RegionSelectionWindow).Current.NativeWindowHandle + $null = [WindowSizing]::ForceForeground($overlayHandle) + } + catch { + # Still on its way up; keep polling until the deadline. + } + + Start-Sleep -Milliseconds 25 + } while ([DateTime]::UtcNow -lt $deadline) + + throw 'Region overlay did not reach the foreground.' +} + +# Reaching the foreground is necessary but not sufficient: the press still has to land on +# the overlay's content. Hit-test the press point through UI Automation until it resolves +# to the app, so PointerPressed is guaranteed to see it rather than firing into a window +# that is foreground but not yet hit-testable. +function Wait-PointerTarget { + param([int]$X, [int]$Y) + + $deadline = [DateTime]::UtcNow.AddSeconds(5) + do { + $element = [System.Windows.Automation.AutomationElement]::FromPoint( + [System.Windows.Point]::new($X, $Y)) + if ($element -and $element.Current.ProcessId -eq $AppPid) { + return + } + + Start-Sleep -Milliseconds 25 + } while ([DateTime]::UtcNow -lt $deadline) + + throw "No window of the app is hit-testable at $X,$Y." +} + function Wait-ProcessElement { param( [string]$AutomationId, @@ -345,6 +439,86 @@ function Close-CapturePickers { throw 'Stale GraphicsCapturePicker windows did not close.' } +# A failure message only says which element never showed up. Snapshot the process's +# actual window tree first, so the report distinguishes "the window was never created" +# from "it exists but automation cannot see it". Capped, and never allowed to mask the +# real failure. +function Write-FailureDiagnostic { + param([string]$Name) + + if ($script:diagnosticCount -ge 2) { + return + } + + $script:diagnosticCount++ + $lines = [System.Collections.Generic.List[string]]::new() + $lines.Add("=== $Name ===") + try { + $lines.Add("Responding: $((Get-Process -Id $AppPid).Responding)") + # Synthetic input lands on the foreground window, so record who actually had it. + $foreground = [WindowSizing]::GetForegroundWindow() + $foregroundProcess = [uint32]0 + $null = [WindowSizing]::GetWindowThreadProcessId( + $foreground, + [ref]$foregroundProcess) + $owner = try { + (Get-Process -Id $foregroundProcess -ErrorAction Stop).ProcessName + } + catch { + 'unknown' + } + $lines.Add( + "Foreground: hwnd=$foreground pid=$foregroundProcess ($owner)" + + " app pid=$AppPid") + # When something outside the app holds the foreground, synthetic input never + # reaches the overlay. Name every top-level window so the culprit is identifiable. + $desktop = [System.Windows.Automation.AutomationElement]::RootElement.FindAll( + [System.Windows.Automation.TreeScope]::Children, + [System.Windows.Automation.Condition]::TrueCondition) + $lines.Add("Desktop top-level windows: $($desktop.Count)") + foreach ($window in $desktop) { + $lines.Add( + " pid=$($window.Current.ProcessId)" + + " class=$($window.Current.ClassName)" + + " name='$($window.Current.Name)'") + } + $root = [System.Windows.Automation.AutomationElement]::RootElement + $processCondition = [System.Windows.Automation.PropertyCondition]::new( + [System.Windows.Automation.AutomationElement]::ProcessIdProperty, + $AppPid) + $windows = $root.FindAll( + [System.Windows.Automation.TreeScope]::Children, + $processCondition) + $lines.Add("Top-level windows: $($windows.Count)") + foreach ($window in $windows) { + $lines.Add( + " class=$($window.Current.ClassName)" + + " name='$($window.Current.Name)'" + + " bounds=$($window.Current.BoundingRectangle)" + + " offscreen=$($window.Current.IsOffscreen)") + $descendants = $window.FindAll( + [System.Windows.Automation.TreeScope]::Descendants, + [System.Windows.Automation.Condition]::TrueCondition) + foreach ($element in $descendants) { + if (-not $element.Current.AutomationId) { + continue + } + + $lines.Add( + " $($element.Current.AutomationId)" + + " type=$($element.Current.ControlType.ProgrammaticName)" + + " offscreen=$($element.Current.IsOffscreen)" + + " enabled=$($element.Current.IsEnabled)") + } + } + } + catch { + $lines.Add("Diagnostic capture failed: $($_.Exception.Message)") + } + + Add-Content -LiteralPath (Join-Path $artifacts 'diagnostics.txt') -Value $lines +} + function Test-Ui { param([string]$Name, [scriptblock]$Action) @@ -357,6 +531,7 @@ function Test-Ui { $results.Add([pscustomobject]@{ name = $Name; status = 'PASS' }) } catch { + Write-FailureDiagnostic $Name [WindowSizing]::SendMouse(0x0004) | Out-Null [WindowSizing]::SendEscape() | Out-Null Start-Sleep -Milliseconds 100 @@ -383,6 +558,12 @@ function Invoke-CaptureMode { $item = Wait-ProcessElement $AutomationId 2000 $item.GetCurrentPattern( [System.Windows.Automation.InvokePattern]::Pattern).Invoke() + # The flyout item only selects the mode — the pill body is what runs the + # capture (MainPage.xaml.cs: RegionCaptureItem_Click -> SelectMode, capture + # happens in CaptureButton_Click). Invoking the item alone starts nothing. + $capture = Wait-AppElement CaptureButton IsEnabled $true 5000 + $capture.GetCurrentPattern( + [System.Windows.Automation.InvokePattern]::Pattern).Invoke() return } catch { @@ -443,9 +624,41 @@ Test-Ui 'Region cancellation recovers' { Wait-AppElement CaptureButton IsEnabled $true 3000 | Out-Null } -Test-Ui 'Region capture completes' { +# Whether a synthetic press actually reaches the overlay depends on who holds the +# foreground at that instant, and on these runners the shell reclaims it unpredictably — +# every deterministic fix so far moved the failure rather than removing it. Retry the +# whole gesture, as Invoke-CaptureMode already does for the same reason. +function Invoke-RegionDrag { + $lastError = $null + for ($attempt = 0; $attempt -lt 3; $attempt++) { + try { + Invoke-RegionDragOnce + return + } + catch { + $lastError = $_.Exception.Message + [WindowSizing]::SendMouse(0x0004) | Out-Null + try { + (Wait-ProcessElement RegionCancelButton 1000). + GetCurrentPattern( + [System.Windows.Automation.InvokePattern]::Pattern). + Invoke() + } + catch { + # No overlay left standing; nothing to dismiss before the next attempt. + } + + Start-Sleep -Milliseconds 200 + } + } + + throw $lastError +} + +function Invoke-RegionDragOnce { Invoke-CaptureMode RegionCaptureItem $null = Wait-ProcessElement RegionCancelButton + $null = Wait-RegionOverlayForeground $overlay = Get-RegionSelectionWindow $bounds = $overlay.Current.BoundingRectangle $startX = [int]($bounds.Left + [Math]::Min(240, $bounds.Width / 4)) @@ -456,6 +669,7 @@ Test-Ui 'Region capture completes' { throw 'Region overlay is too small for the drag journey.' } + Wait-PointerTarget $startX $startY if (-not [WindowSizing]::SetCursorPos($startX, $startY)) { throw 'Could not position the region pointer.' } @@ -465,10 +679,14 @@ Test-Ui 'Region capture completes' { } Start-Sleep -Milliseconds 100 - for ($step = 0; $step -lt 10; $step++) { - if (-not [WindowSizing]::MoveMouse( - [int](($endX - $startX) / 10), - [int](($endY - $startY) / 10))) { + # Step the cursor in absolute coordinates. Relative SendInput moves are scaled by the + # pointer speed and "enhance pointer precision" settings, so the drag landed somewhere + # other than the target and the selection never closed — the overlay was still up when + # the assertion timed out, and arm64 (different pointer defaults) failed far more often. + for ($step = 1; $step -le 10; $step++) { + $x = [int]($startX + (($endX - $startX) * $step / 10)) + $y = [int]($startY + (($endY - $startY) * $step / 10)) + if (-not [WindowSizing]::SetCursorPos($x, $y)) { throw 'Could not drag the region pointer.' } @@ -483,6 +701,10 @@ Test-Ui 'Region capture completes' { Wait-AppElement PreviewImage IsOffscreen $false 3000 | Out-Null } +Test-Ui 'Region capture completes' { + Invoke-RegionDrag +} + Test-Ui 'Window picker cancellation recovers' { Close-CapturePickers Invoke-CaptureMode WindowCaptureItem @@ -649,42 +871,26 @@ Test-Ui 'Interactive controls expose UI Automation identity' { (-not $_.automationId -or -not $_.name) }) if ($missing.Count -ne 0) { - throw (($missing | ForEach-Object name) -join ', ') + # Reporting the name is useless here — a missing name is exactly what this + # catches, so the message came out empty. Identify the element instead. + throw (($missing | ForEach-Object { + "$($_.type) automationId='$($_.automationId)' name='$($_.name)'" + }) -join '; ') } } function Invoke-RegionCancellation { - $mainWindow = Get-AppWindow - $mainHandle = [IntPtr]$mainWindow.Current.NativeWindowHandle $cleanupRequired = $true try { Invoke-CaptureMode RegionCaptureItem - $deadline = [DateTime]::UtcNow.AddSeconds(5) - do { - $foreground = [WindowSizing]::GetForegroundWindow() - $foregroundProcess = [uint32]0 - $null = [WindowSizing]::GetWindowThreadProcessId( - $foreground, - [ref]$foregroundProcess) - if ($foreground -ne [IntPtr]::Zero -and - $foreground -ne $mainHandle -and - $foregroundProcess -eq $AppPid) { - break - } - - Start-Sleep -Milliseconds 25 - } while ([DateTime]::UtcNow -lt $deadline) - - if ($foreground -eq [IntPtr]::Zero -or - $foreground -eq $mainHandle -or - $foregroundProcess -ne $AppPid) { - throw 'Region overlay did not receive keyboard focus.' - } - - if (-not [WindowSizing]::SendEscape()) { - throw 'Escape input failed.' - } - + # Cancel through UI Automation rather than a synthetic Escape. Keystrokes only + # reach the overlay while it holds the foreground, and the shell reclaims it + # often enough that a 100-iteration soak is certain to hit a moment where it + # does not. The button is the same cancellation path and needs no activation. + # (Cost: the overlay's Escape accelerator is no longer exercised here.) + (Wait-ProcessElement RegionCancelButton). + GetCurrentPattern([System.Windows.Automation.InvokePattern]::Pattern). + Invoke() Wait-AppElement CaptureButton IsEnabled $true 5000 | Out-Null $cleanupRequired = $false }