Skip to content
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

Add support for multiple top-level windows on Windows #56090

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

hbatagelo
Copy link
Contributor

@hbatagelo hbatagelo commented Oct 24, 2024

Design doc: https://docs.google.com/document/d/1eQG-IS7r4_S9_h50MY_hSGwUVtgSTRSLzu7MLzPNd2Y/edit?tab=t.0

This PR is a part of a group of PRs. Going from first to last, these must be merged in the following order:

See also the corresponding framework PRs for multi-win support, starting from: flutter/flutter#157515

What's New

This pull request enables support for multiple top-level windows in Flutter applications on Windows. Specifically, this PR:

  • Adds the --enable-multi-window command-line option, allowing applications to opt in to multi-window support (disabled by default).

  • Introduces the flutter/windowing method channel, allowing windows to be created and destroyed from the framework.
    The following methods are available:

    • createWindow(Map): Creates a top-level window.
      Map must contain a <String, List<int>> key-value pair, where the key is "size", and the value is a list containing the requested width and height for the window's client rectangle, in logical coordinates.
      On success, a Map is returned with the following key-value pairs:

      Key (String) Value
      "viewId" The view ID of the window (int)
      "archetype" WindowArchetype::regular
      "size" The current size of the window (width, height), in logical coordinates (List<int>)
      "parentViewId" null

      If the method fails, a PlatformException is thrown.

    • destroyWindow(Map): Destroys a window.
      Map must contain a <String, int> key-value pair, where the key is "viewId", and the value is the view ID of the window to be destroyed. The method returns void on success. Otherwise, a PlatformException is thrown.

    The following callback methods can be listened to:

    • onWindowCreated(Map): Called when a window is created.
      Map contains the following key-value pairs:
      Key (String) Value
      "viewId" The view ID (int)
      "parentViewId" The parent view ID (int), or null if the window has no parent
    • onWindowDestroyed(Map): Called when a window is destroyed.
      Map contains a single <String, int> key-value pair, where the key is "viewId", and the value is the view ID.
    • onWindowChanged(Map): Called when a window property is changed (e.g., on resizing).
      Map contains the following key-value pairs:
      Key (String) Value
      "viewId" The view ID (int)
      "size" The current size of the window in logical coordinates (List<int>), or null if the window size hasn't changed

How It Works

With multi-window support enabled, FlutterWindowsEngine creates a uniquely owned FlutterHostWindowController that is responsible for managing the lifecycle of windows associated with the engine. The engine registers the platform channel using WindowingHandler.

When the Dart app requests a new window using the createX methods, WindowingHandler calls FlutterHostWindowController::CreateHostWindow with the decoded arguments. The controller creates a uniquely owned FlutterHostWindow object which manages a native Win32 window with a child Flutter view in its client area. Each view, represented by FlutterWindowsViewController, is owned by its corresponding FlutterHostWindow. When the Dart app requests the destruction of a window (given a view ID) or when the native window is closed from the system, the controller destroys the corresponding FlutterHostWindow, which, in turn, cleans up the associated view controller.

Window messages are first handled by a static FlutterHostWindow::WndProc function that initializes the native Win32 window upon receiving WM_NCCREATE. Other messages are forwarded to FlutterHostWindowController::HandleMessage to manage global behaviors (e.g., hiding satellites of inactive top-level windows). Finally, messages are passed back to FlutterHostWindow::HandleMessage for window-specific behavior.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

FLUTTER_EXPORT FlutterDesktopViewControllerRef
FlutterDesktopEngineCreateViewController(
FlutterDesktopEngineRef engine,
const FlutterDesktopViewControllerProperties* properties);
Copy link
Member

Choose a reason for hiding this comment

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

FYI, we were holding off on making the multi-window APIs public until the Windows embedder properly supports multi-window.

Known problems with Window's multi-window support: https://flutter.dev/go/multi-window-status#heading=h.i5rwjhitgcmu

  1. Cursor support
  2. Keyboard support
  3. Text input support
  4. App lifecycle across multiple windows
  5. Accessibility support
  6. Plugins updated

Could we keep all these APIs non-public until we're sure these are production ready?

Copy link

Choose a reason for hiding this comment

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

I hope it can be public before the above 6 items are ready, so that more people can prototype this function;
it is enough for flutter to just warn that it is not production ready;

Copy link
Member

@loic-sharma loic-sharma Oct 26, 2024

Choose a reason for hiding this comment

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

I hope it can be public before the above 6 items are ready, so that more people can prototype this function; it is enough for flutter to just warn that it is not production ready;

The API is "pubternal"; advanced users can opt-in to using it either by building a custom engine or by linking against the API directly. I think that's a good compromise as it prevents folks from unintentionally using an API that we know is broken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. I'll revert to the non-public C API.


namespace flutter {

// A singleton controller for Flutter windows.
Copy link
Member

Choose a reason for hiding this comment

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

Could you expand why this needs to be a singleton? What would I do if my app has two Flutter engines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This approach seemed reasonable to avoid injecting the controller into each window instance, but we weren't taking into account the use of multiple engines. I'll change it to a non-singleton class.


#include <dwmapi.h>

namespace {
Copy link
Member

Choose a reason for hiding this comment

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

This shell/platform/windows/client_wrapper layer is code that the Flutter tool copies into the end user's project. This code doesn't get compiled into flutter_windows.dll. This doesn't seem like the right place to implement the flutter/windowing platform channel.

Our other platform channels are registered in the FlutterWindowsEngine here. Should we move the platform channel handling logic down to there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood. I'll move the handler to FlutterWindowsEngine

@loic-sharma
Copy link
Member

@hbatagelo would you be able to join the next Google / Canonical sync meeting to discuss this PR?

@hbatagelo
Copy link
Contributor Author

@hbatagelo would you be able to join the next Google / Canonical sync meeting to discuss this PR?

Sure. I'll be there.

@wanjm
Copy link

wanjm commented Oct 30, 2024

when will be the next sync meeting.

auto const& data{data_opt.value()};
result.Success(EncodableValue(EncodableMap{
{EncodableValue("viewId"), EncodableValue(data.view_id)},
{EncodableValue("archetype"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Enumerated values in other channels are normally passed as strings, i.e. "Archetype.regular"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good to know, thanks. I'll change it.

return;
}

std::wstring const title{ArchetypeToWideString(archetype)};
Copy link
Contributor

Choose a reason for hiding this comment

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

Setting the title seems to the archetype seems like debugging code that shouldn't be here anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's just a placeholder. Setting the title is straightforward. I'll do it.

{EncodableValue("viewId"), EncodableValue(data.view_id)},
{EncodableValue("archetype"),
EncodableValue(static_cast<int>(data.archetype))},
{EncodableValue("size"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be easier to send "width" and "height" - less marshalling required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current approach aims to maintain symmetry between the Dart API and the serialized structure, although we haven't given it much thought yet. I agree that a flat structure would simplify the serialization, but the nested approach may be more readable and intuitive as we add more attributes.

Maybe we should decide whether to adopt a flat structure (easier to serialize, but possibly cluttering the payload with many keys) or a nested structure (mimicking the Dart syntax but a bit more complex to parse) and write this down in the design doc.

Nested structure (using createPopup as an example):

{
  "parentViewId": 42,
  "preferredSize": { "width": 800, "height": 600 },
  "anchorRect": { "left": 0, "top": 0, "width": 100, "height": 200 },
  "positionerParentAnchor": "topLeft",
  "positionerChildAnchor": "center",
  "positionerOffset": { "dx": 10, "dy": 20 },
  "positionerConstraintAdjustment": ["slideX", "slideY"]
}

Flat structure:

{
  "parentViewId": 42,
  "preferredWidth": 800,
  "preferredHeight": 600,
  "anchorRectLeft": 0,
  "anchorRectTop": 0,
  "anchorRectWidth": 100,
  "anchorRectHeight": 200,
  "positionerParentAnchor": "topLeft",
  "positionerChildAnchor": "topLeft",
  "positionerOffsetX": 10,
  "positionerOffsetY": 20,
  "positionerConstraintAdjustment": ["slideX", "slideY"]
}

@hbatagelo
Copy link
Contributor Author

Updates from canonical#1:

  • The changes from the multi-view patch on Windows have been reverted. The multi-window C API is now non-public again.
  • The window controller, previously a singleton, is now instantiated once per engine and uniquely owned by it.
  • Applications can opt in for multi-window support by using the --enable-multi-window command-line option.
  • The engine now directly registers the platform channel for windowing communication. This channel is only registered for applications that opt in for multi-window support.
  • Several fixes and improvements (prevented leakage of registered window classes, improved the behavior of modal dialogs by ensuring disabled windows do not appear active when selected via the task switcher, refactored tests, aligned the behavior of dialogs according to the spec, updated code style, improved comments).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants