[Engine] Consistently handle Display IDs above INT64_MAX by sero583 · Pull Request #189570 · flutter/flutter

sero583 · 2026-07-16T10:05:02Z

Dart ints are signed 64-bit, so a display ID above INT64_MAX can't be represented in Dart. What actually happened with such an ID depended on the toolchain.

tonic's DartConverter<unsigned long long> asserted the range, while every other integer type has no check. So it came down to what size_t is: on MSVC x64 debug builds aborted, on LP64 the same ID silently wrapped negative. This removes the specialization, as suggested in review, so all platforms behave the same now: values above INT64_MAX become the corresponding negative Dart integer, distinct values remain distinct. The dropped constants have no other users and nothing relied on the assert.

DisplayId itself was size_t, but the ABI says uint64_t, so 32-bit builds truncated IDs at the embedder boundary. It is int64_t now (like FlutterViewId), with one explicit cast at the boundary, and ViewportMetrics::display_id got the same change. The tonic change alone fixes the crash, the typedef fixes the 32-bit narrowing.

This replaces the HMONITOR masking this PR originally proposed.

EmbedderTest.SetSingleDisplayConfigurationWithDisplayIdAboveInt64Max sends 0xFFFFFFFFE02E16A5, a real sign-extended HMONITOR, and checks it reaches Dart as the negative equivalent through both the display list and the window metrics. Without the fix it reproduces the crash from the issue on Windows:

[ERROR:flutter/shell/common/shell.cc(121)] assertion failed val <= 0x7fffffffffffffffLL
Frame 0: tonic::DartConverter<unsigned long long,void>::ToDart
Frame 1: tonic::DartConverter<std::vector<unsigned long long,...> >::ToDart
Frame 2: tonic::ToDart<std::vector<unsigned long long,...> >
Frame 3: flutter::PlatformConfiguration::UpdateDisplays
Frame 4: flutter::RuntimeController::SetDisplays

On LP64 the old behavior already wrapped to the same value, so there the test pins the behavior down rather than catching a regression.

Fixes #189564

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

gemini-code-assist

Code Review

This pull request introduces a helper method ToDisplayId in DisplayManagerWin32 to convert HMONITOR handles to FlutterEngineDisplayId by truncating them to 32-bit values, which strips sign extension and prevents representation issues in Dart. The display manager, window implementation, and unit tests have been updated to use this method. There are no review comments, and I have no feedback to provide.

sero583 · 2026-07-28T06:28:32Z

Marking this as a draft to stay within the two active PR limit while I prioritize another change. Reviews are still welcome, and I will set it back to Ready once the other one lands.

loic-sharma · 2026-07-28T21:49:21Z

@sero583 Sounds good! Just FYI, @mattkae is the primary reviewer for Windows changes, and he's out for the next two weeks on vacation. Please expect some delays for reviews :)

knopp

I don't know if this is the best fix. I'm looking at dart_converter.h from tonic

  // TODO(abarth): The Dart VM API doesn't yet have an entry-point for
  // an unsigned 64-bit type. We will need to add a Dart API for
  // constructing an integer from uint64_t.
  //
  // (In the meantime, we have asserts below to check that we're never
  // converting values that have the 64th bit set.)

  static Dart_Handle ToDart(unsigned long long val) {
    TONIC_DCHECK(val <= 0x7fffffffffffffffLL);
    return Dart_NewInteger(static_cast<int64_t>(val));
  }

But Dart also have Dart_NewIntegerFromUint64 and it's been there for a while - seems to me that we should maybe fix this in tonic?

knopp · 2026-07-28T22:35:03Z

Never mind, Dart_NewIntegerFromUint64 basically just does the same check. So I think we should consider casting DisplayId to ssize_t, or better yet typedef it as int64_t (or uint64_t and static_cast it to int64_t before converting to dart). Right now FlutterEngineDisplayId in embedder.h is uint64_t while flutter::DisplayId is size_t. I'm not sure if anyone builds engine on 32bit anymore, but it doesn't seem right..

knopp · 2026-07-28T22:36:55Z

This is from embedder.h

/// Display refers to a graphics hardware system consisting of a framebuffer,
/// typically a monitor or a screen. This ID is unique per display and is
/// stable until the Flutter application restarts.
typedef uint64_t FlutterEngineDisplayId;

It doesn't mention anything about allowed ranges, so this really seems like a bug in the engine itself rather than windows embedder.

sero583 · 2026-08-03T07:35:29Z

@knopp You're right, this is not a Windows problem, it is the type.

DisplayId is size_t in shell/common/display.h. On MSVC x64 that is unsigned long long, so it selects the tonic specialization with the DCHECK and aborts. On LP64 it is unsigned long, which goes through DartConverterInteger and Dart_NewInteger without any check, so the same id silently becomes negative. That is why only Windows sees it. On 32 bit it is truncated even earlier, where FlutterEngineDisplayId is assigned into size_t in MakeViewportMetricsFromWindowMetrics and FlutterEngineNotifyDisplayUpdate, so you were right about that as well.

I used the int64_t typedef you suggested. uint64_t to int64_t is a bijection, so the uniqueness check in ValidDisplayConfiguration still holds, which my masking did not guarantee. ViewportMetrics::display_id needed the same change. The HMONITOR change is not needed any more, so I removed it and will retitle this PR instead of opening a new one.

One question before I push: embedder.h does not document the range, and Display.id says it is "not derived from any platform specific identifiers for displays", but Windows passes the raw HMONITOR and macOS the CGDirectDisplayID. Should I document the range, or normalize the ids in the shell so that it is true?

knopp · 2026-08-03T08:04:44Z

It seems to me that the existence of DartConverter<unsigned long long> specialization is the root of the problem and needs to be removed. On LP64 (Linux/macOS) platforms we do not do the range check and it makes no sense to only do it on LLP64 platform (Win32).

As for the Dart DisplayId comment - It is not entirely correct but we do not want users to make assumptions about the DisplayId in general.

Dart ints are signed 64-bit, so a display ID above INT64_MAX can't be represented in Dart. What actually happened with such an ID depended on the toolchain. tonic's DartConverter<unsigned long long> asserted the range, while every other integer type has no check. So it came down to what size_t is: on MSVC x64 debug builds aborted, on LP64 the same ID silently wrapped negative. This removes the specialization, as suggested by knopp, so all platforms behave the same now: values above INT64_MAX become the corresponding negative Dart integer, distinct values remain distinct. The dropped constants have no other users and nothing relied on the assert. DisplayId itself was size_t, but the ABI says uint64_t, so 32-bit builds truncated IDs at the embedder boundary. It is int64_t now (like FlutterViewId), with one explicit cast at the boundary, and ViewportMetrics::display_id got the same change. The tonic change alone fixes the crash, the typedef fixes the 32-bit narrowing. This replaces the HMONITOR masking this PR originally proposed. Test: EmbedderTest.SetSingleDisplayConfigurationWithDisplayIdAboveInt64Max sends 0xFFFFFFFFE02E16A5, a real sign-extended HMONITOR, and checks it reaches Dart as the negative equivalent through both the display list and the window metrics. Fixes flutter#189564

sero583 · 2026-08-17T08:20:15Z

Done. Removed the specialization, so unsigned long long converts like every other integer type now and the assert is gone on all platforms. Kept the int64_t typedef as well, 32-bit builds truncated the ID at the embedder boundary before the converter was even reached. The Display.id comment stays as it is. Retitled and pushed.

github-actions Bot added engine flutter/engine related. See also e: labels. platform-windows Building on or for Windows specifically a: desktop Running on desktop team-windows Owned by the Windows platform team labels Jul 16, 2026

mbcorona mentioned this pull request Jul 16, 2026

[windows] Debug assertion when Windows returns an HMONITOR with the high bit set (sign-extended display id) #189564

Open

This was referenced Jul 16, 2026

[Impeller] Add Vulkan rendering backend for Linux and Windows desktops #183382

Closed

[Linux] Add support for Vulkan #181711

Open

sero583 marked this pull request as ready for review July 17, 2026 05:22

sero583 requested a review from a team as a code owner July 17, 2026 05:22

gemini-code-assist Bot reviewed Jul 17, 2026

View reviewed changes

loic-sharma requested review from loic-sharma and mattkae July 21, 2026 17:11

sero583 marked this pull request as draft July 28, 2026 06:26

knopp requested changes Jul 28, 2026

View reviewed changes

sero583 force-pushed the fix-hmonitor-sign-extension branch from cf65c53 to 2125d14 Compare August 17, 2026 08:16

github-actions Bot added f: scrolling Viewports, list views, slivers, etc. and removed platform-windows Building on or for Windows specifically a: desktop Running on desktop team-windows Owned by the Windows platform team labels Aug 17, 2026

sero583 changed the title ~~[windows] Strip HMONITOR sign extension from display ids~~ [Engine] Consistently handle Display IDs above INT64_MAX Aug 17, 2026

sero583 requested a review from knopp August 17, 2026 08:21

Navigation Menu

Repository navigation

[Engine] Consistently handle Display IDs above INT64_MAX - #189570

[Engine] Consistently handle Display IDs above INT64_MAX#189570
sero583 wants to merge 1 commit into
flutter:masterfrom
sero583:fix-hmonitor-sign-extension

sero583 commented Jul 16, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

sero583 commented Jul 28, 2026

Uh oh!

loic-sharma commented Jul 28, 2026

Uh oh!

knopp left a comment

Uh oh!

knopp commented Jul 28, 2026 •

edited

Loading

Uh oh!

knopp commented Jul 28, 2026

Uh oh!

sero583 commented Aug 3, 2026

Uh oh!

knopp commented Aug 3, 2026

Uh oh!

sero583 commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sero583 commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

sero583 commented Jul 28, 2026

Uh oh!

loic-sharma commented Jul 28, 2026

Uh oh!

knopp left a comment

Choose a reason for hiding this comment

Uh oh!

knopp commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knopp commented Jul 28, 2026

Uh oh!

sero583 commented Aug 3, 2026

Uh oh!

knopp commented Aug 3, 2026

Uh oh!

sero583 commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sero583 commented Jul 16, 2026 •

edited

Loading

knopp commented Jul 28, 2026 •

edited

Loading