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

[Feature] 🪟 Multi Window 🪟 .. redux! #1964

Merged
merged 75 commits into from
Dec 5, 2023

Conversation

bungoboingo
Copy link
Contributor

@bungoboingo bungoboingo commented Jul 21, 2023

This PR introduces support for multiple windows!

Branched off of @derezzedex's old PR (closes #1439), which implements RFC #8.

Note that much of the code in this PR is from his branch, updated to Iced master from 0.4, with some added bits on top and small tweaks/changes.

multi_window.mov

Summary 🪟

🆕 A new multi_window::Application trait

The goal is to be as non-invasive as possible when adding support for multiple windows for existing Iced users. Users can now implement the multi_window::Application trait for their applications if they want the capability of having more than one window.

This new trait adds a window::Id parameter to several methods of the familiar Application trait, e.g.

fn view(&self, window: window::Id) -> Element<Message>;

Here a user can choose which Element to return for each window, based on its window::Id. A window::Id is a unique identifier for a window, created when the window is first spawned. Users can store this ID and use it to reference specific windows. This is different from winit's WindowId. Internally, we reserve window::Id::MAIN to represent the first window spawned.

👐 Creating a window

Users can create a new window with the window::spawn command, which takes a window::Id and window::Settings.

window::spawn(window::Id::new(my_window), window::Settings::default());

🌂 Closing a window

A user can close a specific window with the window::close(id) command. I've utilized the existing exit_on_close_request field of iced::Settings to determine whether or not to close the entire application when the "main" or first window is closed. Not sure if this is wanted or needed behavior, but this exit_on_close_request doesn't make much sense in multi window otherwise. Could do a different set of settings, or just ignore it!

🤔 New Window Events

Users can choose to listen to new window events implemented for multi window, such as window::Event::Created which is emitted after the window is finished being created by winit, or window::Event::Destroyed which is emitted after the window is fully destroyed. There is also window::Event::CloseRequested, which can be subscribed to to close the window with the window::close command.

🚤 Known areas to improve performance:

  • Right now I am making one instance of a Renderer per-window, each with their own Backend. This is very bad! We should be sharing a single Backend across multiple Renderers. This will take some refactoring to our Compositor and our Backend to make sure that all the caching is refactored with multiple surfaces in mind.
  • There is currently no way to know what window needs to be updated based on the result of an application update. The plan is later to introduce some way to get a list of windows that need to be updated from multi_window::Application::update. This requires us to rebuild all user interfaces when processing any message, regardless of if that message affects all windows or just one
  • We currently have no way of knowing what window needs to be redrawn, so we redraw all windows every frame. With upcoming changes to widgets to manually request to be redrawn on state changes, this problem should be solved.

Other suggestions very welcome and appreciated!

👻 Commit history..
Yes it's a mess. I tried to rebase from the original PR, but that was done before the great Purification which changed nearly all of the codebase, and it was a real struggle. If we care enough I will get back in the trenches and try to sort out the commit history, but for now it's a bit of a frankenstein.

TODOs:

  • Test multi-window Program without a multi_window::Application trait. Not sure if we even want this behavior though.
  • Does it make sense to limit this to non-wasm targets..? I assume yes but not sure if there is a use case for it.

Tested on latest MacOS, PopOS & Windows 10.

@hecrj hecrj added feature New feature or request rendering shell labels Jul 26, 2023
@hecrj hecrj added this to the 0.11.0 milestone Jul 26, 2023
@hecrj hecrj added the addition label Sep 3, 2023
@hecrj hecrj force-pushed the feat/multi-window-support branch from 32dcf5f to 5c5e765 Compare December 2, 2023 21:29
@hecrj
Copy link
Member

hecrj commented Dec 2, 2023

Alright! Finally got this to the finish line! Great stuff, @derezzedex and @bungoboingo! 🥳

I have refactored a bunch of stuff here and there. Most important changes are:

  • The Event in the multi_window shell has been split into Event and Control in d34bc4e. We avoid the awkward unsafe impl Send and some unreachable!, as well as the weird nesting inside the Proxy type.
  • window::Id has been changed to use an opaque AtomicU64 in ea42af7. window::spawn initializes and returns a brand new unique window::Id alongside the Command. This way, we avoid weird scenarios where a user could spawn a window with the same identifier twice.
  • The Windows abstraction has been completely refactored into a WindowManager in 5c5e765. Instead of having the state distributed in multiple Vec, the new abstraction uses a single BTreeMap to store all the state for each Window and another one for aliasing winit identifiers.

And I think that's it! I have most definitely changed some of the behavior (i.e. Closed event returned right after a CloseRequested is handled), but I believe it's still somewhat intuitive and consistent.

Let me know what you think! And let's ship this! 🚢

@bungoboingo
Copy link
Contributor Author

LGTM, let's 🚢 it!!!

@hecrj hecrj merged commit fc285d3 into iced-rs:master Dec 5, 2023
13 checks passed
@varbhat
Copy link
Contributor

varbhat commented Dec 5, 2023

@hecrj @bungoboingo ,

We currently have no way of knowing what window needs to be redrawn, so we redraw all windows every frame. With upcoming changes to widgets to manually request to be redrawn on state changes, this problem should be solved.

Will all windows redraw on state change? I think this is one thing i dislike and which i think can be improved. I hoped for selective window redraw.

@bungoboingo
Copy link
Contributor Author

bungoboingo commented Dec 6, 2023

@hecrj @bungoboingo ,

We currently have no way of knowing what window needs to be redrawn, so we redraw all windows every frame. With upcoming changes to widgets to manually request to be redrawn on state changes, this problem should be solved.

Will all windows redraw on state change? I think this is one thing i dislike and which i think can be improved. I hoped for selective window redraw.

Currently all windows are being redrawn due to existing limitations; there are plans in the near-ish future to have widgets explicitly request to be redrawn on state changes, which will fix this behavior. Lots of iterations will be made for multi-window performance increases, like shared rendering resources, etc. They will all be added in time!

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

Successfully merging this pull request may close these issues.

6 participants