Skip to content

Fix non-finite bounds producing UB via static_cast in XAML/WinUI3 elements#7

Merged
asklar merged 20 commits intomainfrom
copilot/fix-element-bounds-issues
Mar 2, 2026
Merged

Fix non-finite bounds producing UB via static_cast in XAML/WinUI3 elements#7
asklar merged 20 commits intomainfrom
copilot/fix-element-bounds-issues

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 28, 2026

XAML diagnostics can return NaN/Infinity property values for element bounds. static_cast<int>(NaN) is undefined behavior, typically producing INT_MIN on MSVC x64 — the "huge negative numbers" seen in WinUI3 output. These corrupt bounds then cause screenshot annotations to skip visible elements (they appear off-screen).

TAP DLL (lvt_tap.cpp)

  • Validate _wtof/ParseOffset results with std::isfinite() before setting node bounds
  • Only mark hasBounds = true when width/height are finite; only set offsets when both are finite

Bounds conversion (xaml_diag_common.cpp, plugin_loader.cpp, wpf_inject.cpp)

  • Add shared safe_double_to_int() in bounds_util.h — returns std::optional<int>, std::nullopt for non-finite values, clamps to INT_MIN..INT_MAX for out-of-range
  • Guard all graft_json_node bounds paths with the optional check pattern
  • Non-finite offsets skip without poisoning the accumulated parent offset
// bounds_util.h
inline std::optional<int> safe_double_to_int(double v) {
    if (!std::isfinite(v)) return std::nullopt;
    if (v >= static_cast<double>(INT_MAX)) return INT_MAX;
    if (v <= static_cast<double>(INT_MIN)) return INT_MIN;
    return static_cast<int>(v);
}

Screenshot annotation (screenshot.cpp)

  • Use long long intermediate arithmetic for bounds subtraction to prevent signed int overflow when element coords are extreme
  • Add collect_annotations() function and AnnotationInfo struct for structured annotation output (DEBUG builds only)
  • Add --annotations-json <file> CLI flag as a debug-only test hook to output annotation rectangles as JSON, guarded with #ifndef NDEBUG

Unit tests

  • Zero-size elements get no bounds
  • Very large offsets are clamped (not UB)
  • Normal values unchanged (regression)

Integration tests — Notepad-based

  • Win32 bounds validation: all element bounds within reasonable screen range
  • Root bounds non-zero: root window has positive dimensions
  • Annotations JSON output (debug only): --annotations-json produces valid structured data
  • Annotations match tree (debug only): every annotated element ID exists in the tree dump
  • WinUI3 bounds (framework-conditional): if WinUI3 detected, all WinUI3 elements have sane bounds
  • XAML/UWP bounds (framework-conditional): if XAML detected, all XAML elements have sane bounds

Integration tests — Controlled Win32 window (KnownWindowFixture)

Creates a Win32 window with Button, Edit, and Static controls at known client-area positions, then runs lvt against it:

  • TreeStructure: verifies root has correct className/type/text, exactly 3 children with expected class names, types, text, and win32 framework
  • ChildBoundsMatchWin32: verifies each control's JSON bounds match GetWindowRect of its HWND exactly
  • AnnotationsIncludeKnownControls (debug only): verifies all 4 controls (parent + 3 children) appear in annotations with positive dimensions
Original prompt

the bounds of elements especially in winui3 apps are wrong eg huge negative numbers. maybe uninitialized memory?
also not every button is being annotated in the screenshot


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Feb 28, 2026
Copilot AI and others added 4 commits February 28, 2026 22:51
- TAP DLL: validate _wtof/ParseOffset results are finite (reject NaN/Inf)
- graft_json_node (xaml_diag_common, plugin_loader, wpf_inject): add
  safe_double_to_int helper that clamps to INT_MIN..INT_MAX range
- screenshot.cpp: use long long arithmetic to prevent int overflow in
  bounds computations during annotation
- Add unit tests for bounds edge cases (zero size, large offsets, normal)

Co-authored-by: asklar <22989529+asklar@users.noreply.github.com>
… overflow

- Move safe_double_to_int to src/bounds_util.h to eliminate code duplication
- Cast winRect fields to long long before subtraction in screenshot annotation

Co-authored-by: asklar <22989529+asklar@users.noreply.github.com>
… overflow

Co-authored-by: asklar <22989529+asklar@users.noreply.github.com>
Co-authored-by: asklar <22989529+asklar@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix element bounds issues in WinUI3 apps Fix non-finite bounds producing UB via static_cast<int> in XAML/WinUI3 elements Feb 28, 2026
Copy link
Copy Markdown
Owner

@asklar asklar left a comment

Choose a reason for hiding this comment

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

@copilot add integration tests for win32, uwp xaml and winui3 where we a) check the element bounds returned in the dump and b) verify that we annotate the right elements when producing a screenshot (we will likely need a test hook to output the annotations in a structured way as a side channel to the image)

Comment thread src/bounds_util.h Outdated
…ok and integration tests

- bounds_util.h: safe_double_to_int now returns std::optional<int>, nullopt for non-finite
- Call sites updated to use if(sx && sy && sw && sh) pattern
- Added --annotations-json CLI flag for structured annotation output
- Added collect_annotations() for computing which elements get annotated
- Integration tests: Win32 bounds validation, annotation verification,
  framework-conditional WinUI3/XAML bounds checks

Co-authored-by: asklar <22989529+asklar@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 1, 2026

@copilot add integration tests for win32, uwp xaml and winui3 where we a) check the element bounds returned in the dump and b) verify that we annotate the right elements when producing a screenshot (w...

Added in a6725e3:

Test hook: --annotations-json <file> CLI flag writes structured JSON array of {id, x, y, width, height} for each element that would be annotated on a screenshot. Backed by new collect_annotations() in screenshot.cpp.

Integration tests added:

  • Win32BoundsReasonable — validates all element bounds are within -10k..50k range
  • RootBoundsNonZero — root window has positive dimensions
  • AnnotationsJsonOutput — annotations file is valid JSON with correct structure
  • AnnotationsMatchTreeElements — every annotated ID exists in the tree dump
  • WinUI3BoundsIfDetected — if winui3 detected, all WinUI3 elements have sane bounds (skips otherwise)
  • XamlBoundsIfDetected — same for UWP XAML elements

…ping in collect_annotations

Co-authored-by: asklar <22989529+asklar@users.noreply.github.com>
Copilot AI changed the title Fix non-finite bounds producing UB via static_cast<int> in XAML/WinUI3 elements Fix non-finite bounds producing UB via static_cast in XAML/WinUI3 elements Mar 1, 2026
Comment thread src/main.cpp
Co-authored-by: asklar <22989529+asklar@users.noreply.github.com>
@asklar
Copy link
Copy Markdown
Owner

asklar commented Mar 1, 2026

@copilot the integration test should create a win32 window with a few controls, at known locations, then verify that the tree dump and the annotations get the right data, including structure as well as bounds

…nds, and annotations

Co-authored-by: asklar <22989529+asklar@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 1, 2026

@copilot the integration test should create a win32 window with a few controls, at known locations, then verify that the tree dump and the annotations get the right data, including structure as well a...

Added KnownWindowFixture in ebbe001 — creates a Win32 window with Button, Edit, and Static controls at known positions (10,10 / 10,50 / 10,90), then verifies:

  • TreeStructure: root is LvtTestWindow with exactly 3 children matching expected className, type, text, and framework
  • ChildBoundsMatchWin32: each control's JSON bounds match GetWindowRect of its HWND exactly
  • AnnotationsIncludeKnownControls (DEBUG): all 4 controls appear in annotations with positive dimensions

asklar and others added 4 commits March 1, 2026 13:22
On Windows, fs::remove throws when the file is still open by an ifstream
in the same process. Close the ifstream explicitly before removing the
annotations JSON temp files in the three affected tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
WinUI3's XAML diagnostics API serializes Vector3 ActualOffset as '0'
instead of proper vector components, causing all XAML elements to stack
at the bridge origin with no position differentiation.

TAP DLL (lvt_tap.cpp):
- Collect Orientation, Spacing, Margin from property chain for layout
- ComputeLayoutOffsets: compute child offsets by accumulating sibling
  widths (horizontal) or heights (vertical) along StackPanel stacking axis
- Use first Orientation value in property chain (most specific override)

Bridge-to-XAML matching (xaml_diag_common.cpp):
- Replace order-based matching with size-based best-fit matching
- Recursively search XAML root descendants for content dimensions
- Skip contentless XAML roots (w=0, h=0) to prevent bridge consumption
- Match each XAML island to the bridge with closest width+height

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The ComputeLayoutOffsets heuristic produced wrong positions for elements
in Grid, Canvas, and other non-StackPanel containers, causing annotation
rectangles to be misaligned with actual UI elements.

Changes:
- Remove ComputeLayoutOffsets and layout field collection from TAP DLL
- Add ApplyDpiScaling: scale XAML logical pixel dimensions by the window
  DPI factor (dpi/96) so they match the physical pixel space used by
  GetWindowRect and the screenshot capture bitmap
- Remove unused IVisualTreeService2 member (GetProperty approach didn't
  work for Vector3 value types)
- Keep the improved size-based bridge-to-XAML matching

Without per-element offsets from ActualOffset (WinUI3 serializes Vector3
as '0'), elements within a XAML island share the bridge origin position.
This is a known WinUI3 diagnostics limitation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cript

Annotation bounds fix:
- TAP DLL: only include offsetX/offsetY in JSON when non-zero, since
  WinUI3 serializes ActualOffset (Vector3) as '0' making zero offsets
  indistinguishable from missing data
- graft_json_node: only set positioned screen bounds on XAML elements
  when explicit offset data is present in the JSON; elements without
  offsets still appear in the tree (type/name/framework/width/height)
  but won't produce garbage overlapping annotation rectangles
- Remove ApplyDpiScaling (no longer needed without positioned bounds)

Terminal self-match fix:
- Exclude windows belonging to our own process or console host from
  --name/--title enumeration, preventing the terminal window (whose
  title contains the lvt command line) from matching

Build script:
- Add build.cmd for building from a VS Developer Command Prompt
- Builds .NET dependencies (WPF TAP, Avalonia walker) before CMake
- Supports 'build.cmd clean' to reconfigure from scratch

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
asklar and others added 6 commits March 1, 2026 21:29
TAP DLL — get real element positions:
- Use IXamlDiagnostics::GetIInspectableFromHandle to get the IInspectable
  for each XAML element, QI for IUIElement (IID from WinUI3 winmd), and
  call TransformToVisual(nullptr).TransformPoint({0,0}) via raw vtable
  calls to get each element's position relative to the XAML island root
- This bypasses the broken ActualOffset property serialization (WinUI3
  serializes Vector3 as '0' for readonly computed properties)
- CollectPositionsViaTransformToVisual runs on the UI thread via
  SendMessage dispatch, collecting positions for 284/289 elements

TAP DLL — serialize offsets correctly:
- Only include offsetX/offsetY in JSON when non-zero, so graft_json_node
  can distinguish 'no offset data' from 'offset is (0,0)'

graft_json_node — use absolute offsets:
- TransformToVisual offsets are absolute within the XAML island (not
  cumulative parent-relative), so pass bridge base to all children
  instead of accumulating offsets
- Only set bounds when explicit offset data is present in JSON

screenshot.cpp — fix DPI scaling:
- Use GetWindowRect (logical pixels, same space as element bounds)
  instead of DwmGetWindowAttribute (physical pixels) for the annotation
  origin, then scale by bmpWidth/winW to convert to bitmap coordinates
- This fixes annotation alignment on high-DPI displays (e.g. 150%)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TAP DLL: capture key properties from GetPropertyValuesChain during
bounds collection. Properties extracted:
- Text content: Text, Content, Header, PlaceholderText, Description,
  Title, Glyph (filtered: skip numeric handle references >10 digits)
- Accessibility: AutomationProperties.Name, AutomationProperties.AutomationId,
  AutomationProperties.HelpText
- State: IsEnabled, Visibility, IsChecked, IsSelected, IsOn, Orientation
- Other: Source, Tag

Properties are serialized as a JSON object in the TAP pipe output and
copied into Element.properties by graft_json_node. Text/Content/Header
values also populate Element.text for display.

The Win32 provider already populates properties (hwnd, style, visible,
enabled). XAML elements now get similar treatment, enabling AI consumers
to identify controls by their accessible names (e.g. 'Bold (Ctrl+B)').

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace raw vtable calls with proper C++/WinRT projected types from
generated WinUI3 headers (Microsoft.UI.Xaml.winmd → cppwinrt.exe).

TAP DLL changes:
- #include winrt/Microsoft.UI.Xaml.h, .Media.h, .Controls.h for
  UIElement, GeneralTransform, TextBlock projected types
- CollectPositionsAndText: use inspectable.try_as<UIElement>() and
  call TransformToVisual(nullptr).TransformPoint({0,0}) for position
- Read TextBlock.Text() directly via C++/WinRT (no more numeric handle
  references from the property chain)
- Add WINRT_LEAN_AND_MEAN to avoid windowsapp.lib dependency; provide
  stub for WINRT_IMPL_RoOriginateLanguageException

graft_json_node (xaml_diag_common.cpp):
- Map XAML x:Name to properties['name'] instead of Element.text
- Set Element.text from Text > Content > Header > AutomationProperties.Name

Build:
- Generate C++/WinRT headers from WinUI3 winmd at build setup time
- Add src/tap/winui3/ to TAP DLL include path; gitignored (generated)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e duplicates

- Only set Element.text from actual text-content properties (Text,
  Content, Header) — not AutomationProperties.Name
- AutomationProperties.Name stays as a property attribute in the output
- When Text/Content/Header populates Element.text, remove it from
  properties to avoid outputting both text='File' and Text='File'

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e boxes

- Skip pure layout types (Grid, StackPanel, Border, ContentPresenter,
  ScrollViewer, Window, etc.) from screenshot annotations unless they
  have text content or an AutomationProperties.Name
- Place labels inside the bounding box when they fit, falling back to
  above or below the box when they don't
- Keeps interactive controls (Button, ToggleButton, TextBlock, etc.)
  and elements with meaningful text/a11y names

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The TAP DLL's WinUI3 C++/WinRT projected headers are generated from
Microsoft.UI.Xaml.winmd using cppwinrt.exe and are gitignored. CI needs
to generate them at build time.

CMakeLists.txt:
- Add configure-time step to find cppwinrt.exe (from Windows SDK),
  locate the Windows App SDK winmd files (NuGet global cache or local
  packages/ dir), and run cppwinrt to generate headers
- Search both NuGet global cache and CI packages/ directory
- Skip generation if headers already exist (incremental builds)

CI (ci.yml):
- Add step to install Windows App SDK NuGet package before configure

TAP DLL (lvt_tap.cpp):
- Make WinUI3 C++/WinRT include optional via __has_include guard
- All TransformToVisual/TextBlock.Text code is #if LVT_HAS_WINUI3_PROJECTION
- Without the headers, TAP DLL compiles and works for property chain
  bounds but without position resolution or text reading

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@asklar asklar marked this pull request as ready for review March 2, 2026 18:41
The TAP DLL now tries both WinUI3 (Microsoft.UI.Xaml) and system XAML
(Windows.UI.Xaml) interfaces when reading element positions and text.
This enables proper annotation of UWP apps like Clock, Calculator, etc.

- System XAML headers (Windows.UI.Xaml) are always available from the
  Windows SDK — no generation needed
- TransformToVisual: tries WinUI3 UIElement first, falls back to
  system XAML UIElement
- TextBlock.Text: tries WinUI3 TextBlock first, falls back to system
- Guard changed from LVT_HAS_WINUI3_PROJECTION to LVT_HAS_XAML_PROJECTION
  (always true since system headers are always present)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@asklar asklar merged commit 1f96838 into main Mar 2, 2026
1 check passed
@asklar asklar deleted the copilot/fix-element-bounds-issues branch March 2, 2026 19:25
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.

2 participants