Support for creating child windows

It would be very useful to have support for creating child windows with a given parent handle.
I implemented support for child windows on Windows in my glutin fork like half a year ago:
https://github.com/Boscop/glutin/blob/master/src/window.rs#L205
https://github.com/Boscop/glutin/blob/master/src/api/win32/init.rs#L76-L84
https://github.com/Boscop/glutin/blob/master/src/api/win32/callback.rs#L23
https://github.com/Boscop/glutin/blob/master/src/api/win32/mod.rs#L471-L473
(Child windows have to run in the same thread as the parent window, so I'm using a HashMap with HWND as key for event dispatching of all the windows in the same thread.)

It would also be useful to have support for timers on the window.
Either via callback:
https://github.com/Boscop/glutin/blob/master/src/window.rs#L508-L516
Or events:
https://github.com/Boscop/glutin/blob/master/src/events.rs#L61
https://github.com/Boscop/glutin/blob/master/src/api/win32/callback.rs#L342-L346

I implemented this because it's necessary to create a child window for writing GUIs for VST plugins. The plugin host creates a window and passes it to the plugin, which has to create a child window on that for it's GUI and register a timer on the HWND so that it refreshes it's GUI every few milliseconds.
This is what I do in the plugin:

impl<'a> Editor for MyPlugin<'a> {
	fn open(&mut self, parent: *mut c_void) {
		let mut display = WindowBuilder::new().with_dimensions(WINDOW_WIDTH, WINDOW_HEIGHT).with_parent(WindowID::new(parent)).build_glium().unwrap();
		{
			let window = display.get_window().unwrap();
			unsafe { user32::SetWindowLongA(window.platform_window() as winapi::HWND, winapi::winuser::GWLP_USERDATA, self as *mut _ as winapi::LONG); }
			window.set_timer(WIN_GUI_FRAME_TIMER, 25 /* ms */, Some(handle_timer)).unwrap();
		}
// ...
const WIN_GUI_FRAME_TIMER: winapi::UINT_PTR = 4242;
unsafe extern "system" fn handle_timer(hwnd: winapi::HWND, _: winapi::UINT, timer_id: winapi::UINT_PTR, elapsed_ms: winapi::DWORD) {
	if timer_id != WIN_GUI_FRAME_TIMER {
		return;
	}
	let plugin = &mut *(user32::GetWindowLongA(hwnd, winapi::winuser::GWLP_USERDATA) as *mut MyPlugin);
	plugin.do_ui();
}

Unfortunately I only know how to do this on windows. .
It would be very useful to have this in winit, maybe even in a cross-platform way.
So that winit with conrod can be used to write GUIs for VST plugins, completely in Rust :)

Btw, at the time, conrod didn't have a winit (or glium) backend, so I also forked piston so I could pass the parent all the way down from my plugin to PistonWindow in conrod down to glutin.
Btw, here is a screenshot of an old version of one of my plugins: https://i.imgur.com/4BQ1tCQ.gifv
In the future I would like to use conrod with winit/glium backend for this.

@Boscop

@Boscop I can successfully do this on mac os, using cocoa-rs, but haven't started on winit yet, just making raw cocoa framework calls at the moment. Can we share knowledge? I've come to similar conclusions - PR on winit to accept child windows + conrod is the best way to get all of this going.

fwiw, this functionality isn't that niche:

Rust sciter has it too.
(Qt can also do it btw.)

Can winit give us some official stance on this? Many of us at the rust-dsp group have had hacked/forked versions of winit for a while now to get this going but it's become very tiresome keeping up with the changes.

If winit is willing to accept a "parent" functionality, could you provide us with some kind of api guideline (eg. what the calls should look like, an interface) and we will PR the feature.

I think the main point of confusion for us is not knowing if it's worth the trouble of PR-ing, if it is unlikely to be wanted by the winit project. I'll put the time into the mac side (I've already done it many times before for previous winit versions, though the latest makes it more complicated).

What do you guys think?

Yes, I would like to have this in winit itself..
I added this feature to my glutin fork before winit existed, then again to winit because it had changed so much, but now it has changed again..
Other people (who aren't using winit for VST plugins but still need child window functionality) would also benefit from this feature in winit :)
If tomaka agrees to merge a PR that implements this for mac and windows, I'll do the windows part.

@tomaka

@tomaka - myself and @Boscop would be willing to work together on a PR for this (we've done this before on earlier versions), but we'd need your input as to:

whether it's even wanted in winit
what the api / interface would look like

I don't see any reason why this wouldn't be accepted, so long as it doesn't cause issues with the existing functionality/ergonomics or have a prohibitive hit on complexity/maintainability. I'd even be willing to implement this on X11, time permitting.

As far as what the API should be, that's hard to determine without me knowing what's required. Since both of you understand what you need, that's a good starting point to work with. Once it's implemented, we'll know exactly how things differ between platforms, and what impact the implementation has on existing structure. From there, it would be much easier to develop an effective interface than it would be to try to develop one now. In the short term, WindowBuilderExt and WndowExt are always safe bets, since they temporarily absolve you from having to think about it.

I should mention there's a caveat here. We have a shortage of reviewers, so depending on how extensive/intrusive the changes would be, it could take a while to get them merged.

Given it's quite a bit of work, I'd really want someone to weigh in with merge permissions before we started this. If the project slips too far, particularly eyeing off PR #237 then the entire codebase changes significantly and we'd basically have to start again.

I think the safest way to do this is to bypass any kind of Builder pattern and just request a Window object directly from the Window class, supplying a handle as a c_void pointer.

The problem with supplying builder objects is that a lot of it becomes kind of redundant. Setup stuff isn't required as the window is already constructed and has attributes.

The other primary issue with winit is the way the mac codebase will implement Drop to free up the pointer to the window after the window closes. This is not desired at all and is too heavy handed for any applications wishing to just 'attach' to an unowned window. It will crash any host software you're trying to run under.

This may be a radical suggestion - but is winit actually becoming too complicated? Window creation and event loop management are so opinionated but separate that combining them in my opinion is creating a fairly bloated little environment. I know this might be the wrong place to raise this point, but to me, this whole issue being so complicated is a code smell of a god-problem with the library - it knows and controls a little too much and it doesn't really have to.

@RobSaunders

@RobSaunders well, I do have merge permissions. You can wait for someone else if you'd like, but I have the most free time.

I've been thinking more about this, and no real guarantees can be made about whether or not a PR that doesn't exist yet will be merged. You're giving me the impression that the changes required are quite substantial (the more you elaborate on this, the easier it is to for an assessment to be made). If it were merged, would you be around to fix any regressions reported, and advise other contributors about how to understand the new code if needed, including on how to rebase existing PRs?

Bypassing the builder makes sense, right. As far as Drop goes, I'm somewhat averse to making the lifetime management of windows more complex.

For your decoupling suggestion, I'd recommend creating a thorough proposal about 1) the intended API, 2) the implementation/architectural changes required, and 3) the pros and cons for both all users (but especially the average user) and for maintainers/contributors (basically, the impact it would have on Rust's graphical application story as a whole).

@francesca64

@francesca64 perfect! That's basically what I was asking, without trying to be rude :) thank you for putting the time into this issue.

The rust-dsp community (https://github.com/rust-dsp) reeeeally want this merged as it's kind of halting a lot of progress for us as we continually attempt to re-invent the wheel for a graphics solution, so I hope I'm portraying this correctly, but I want to be as explicit and honest as possible about what's required.

The scope of the issue is primarily hampered by the assumptions winit makes about windows you'll be managing, mainly on the MacOS side though. The Linux and Windows sides seem to be a much more straightforward implementation. This is because MacOS's event delegation system creates more choices for how to 'listen' to events, and because MacOS has an ARC for any created objects which must be catered for.

I don't foresee huge problems as long as we can bypass Builder and somehow remove the automatic ability to nuke the window (and call NSApp:terminate). A simple unowned_window parameter could alert winit to treat the contexts it does not own as read-only.

Fundamentally, I agree that we would not want to change winit in a way that any user not desiring these features would even notice they are there.

Just quickly on the last point about decoupling. I would foresee a library that concentrates purely on window generation, which winit could then depend upon. So users of winit would be completely unaware, but perhaps users like us would be able to interact with the smaller module directly.

This last point may actually solve both of our problems and also allow for new plugin/host architecture systems to use winit. For example, I could use a library that sits on top of this window library that wraps the window in a WebKit or WebView control (depending on OS) and rely on the smaller library purely for window generation, which as of now I have to roll my own and understand how the Rust/FFI/ObjC bridge works - I've gone this route and it is HEAVY learning, no normal user would want to do this.

Winit "takes over" so you can't do the above scenario at all, and then it provides an event loop that I have ZERO use for as these subsystems have their own eventing and callbacks.

Or, I could introduce a library that purely, for example, wraps some MacOS control such as a NSTimer or system control, and users could bundle together the support they needed to build a simple app without having to drag around the event management stuff of winit.

The winit system has chosen to abstract away events for a good reason - it becomes platform agnostic even to game systems like OpenGL, which is where it is most useful. But for regular desktop use or libraries that don't wish to go down this path, there's really no options in rust-land for them and it's holding them back in my opinion.

But again, even if the project were split out, I think users of winit should be largely unaffected as winit would just rely on this lib.

@RobSaunders

@RobSaunders alright, unowned_window sounds pretty reasonable. I don't have any objections at this point, and I'll have faith in your statement that it won't balloon complexity or anything.

There are some pending API changes / reworks to be mindful of:

API change proposal for window closing #434 will hopefully be done soon (though it looks like the scope could be expanding) and I imagine it will touch some of the same code as your changes.
You're already aware of Refactor MacOS and provide a coroutine-driven runloop #237. I don't think it's likely to merge any time soon, but ideally your changes won't be fundamentally obstructive to its design. I don't know if that needs to be said (since I don't actually know what saying that entails in this case), but I feel I'd be remiss if I didn't say it.
There's also Remove child thread from win32 event loop #445, relevant for @Boscop. It doesn't look like it will merge soon, since the unresolved issues are pretty significant and there's not any discussion happening.

On the decoupling issue, I can definitely see the merits in the idea. However, I have some reasons for why I don't think it can be a priority at present:

The average Rustacean already doesn't seem to know the difference between glutin and winit. With the addition of a 3rd layer, I can vividly visualize this scenario: report issue to glutin -> "sorry, this should actually be reported to winit" -> report issue to winit -> "actually, this is an issue with <insert name of our super cool window creation crate here>". I know this isn't a very satisfying reason for impeding progress, but I do want to emphasize that while we can abstract this complexity away in the API, it can still lead to confusion.
It would be very difficult to mobilize a change of this scope. Making changes across multiple platforms is always an undertaking, and the current situation with reviewers and dedicated contributors per platform is as much of a blocking issue as anything else.
winit still has a lot of unresolved issues, both in implementation and API. Many of these have a much more immediate impact on whether or not applications can effectively be developed on top of winit. My feeling is that it's best to focus most of our limited resources on these issues.

My current objective is to try to improve project health, and I think that when winit becomes healthier, a proposal like this is much more realistic.

On that note, how would you feel about being pinged to review macOS PRs? It would be a great help.

@francesca64

@francesca64 I can review MacOS PR's, sure. If you check my github you'll see I've done a lot of work in this area (Rust-MacOS FFI stuff). Would be happy to help out where I can, time permitting.

Decoupling, I did assume this was the reason so far. I think breaking off into libraries can simplify testing and merging PRs though. As libraries grow they often decouple to modularise teams and complexity. But I do see that it's no simple feat.

I do think that one approach would be for someone to actually write this smaller library, and perhaps once it reaches a level of stability and maturity, winit could consider consuming it in a future version and shedding its internal complexity on window generation and act as a wrapper for that plus eventing and the other benefits it provides.

I will need some time to fully go through those issues you linked, I am aware of them but I think I will chat with @Boscop and come up with a general game plan.

I'll give you some updates. CloseRequested/Destroyed is pretty much wrapped up, and it's very unintrusive, so you don't have to worry about it.

For the 2 PRs I linked, this is where discussion on the underlying issues is moving: #459 (comment)

@l0calh05t

@l0calh05t Ah, yep, indeed. Seems like the Unix and MacOS versions doesn't have with_parent yet.

@crsaracco

@crsaracco Does #1508 satisfy requirement 2 on Windows?

@Osspial

Hey @Osspial
Very cool to see some progress on this! A lot of people in RustAudio are very excited to start making some gui's for our vst's.

I'm trying to get a vst running using #1508, but I'm getting following error:

thread '' panicked at 'InvalidateRgn should be used instead of UpdateWindow', C:\Users\rasmu\Documents\RustProjects\winit\src\platform_impl\windows\event_loop.rs:880:17

I don't think this is my fault, but the code for the vst is here if that's useful:

Apologies if this isn't the right place for this

@Osspial

Hey @Osspial
Very cool to see some progress on this! A lot of people in RustAudio are very excited to start making some gui's for our vst's.

I'm trying to get a vst running using #1508, but I'm getting following error:

thread '' panicked at 'InvalidateRgn should be used instead of UpdateWindow', C:\Users\rasmu\Documents\RustProjects\winit\src\platform_impl\windows\event_loop.rs:880:17

I don't think this is my fault, but the code for the vst is here if that's useful:
https://gist.github.com/Fredemus/af69619c3fbc79b91ce7471d7372ed7e#file-winit-vst-rs-L72-L107

Apologies if this isn't the right place for this

Hmm, the link above seems dead.

Sorry, fixed it now

Good to know that this is a step in the right direction!

I'm trying to get a vst running using #1508, but I'm getting following error:

thread '' panicked at 'InvalidateRgn should be used instead of UpdateWindow', C:\Users\rasmu\Documents\RustProjects\winit\src\platform_impl\windows\event_loop.rs:880:17

I don't think this is my fault, but the code for the vst is here if that's useful:

Don't worry, that indeed is not your fault. There's actually some commented-out code right next to that panic that handles its branch properly - I think the assumption was that only Winit developers would ever see that panic - so I've uncommented said code and removed the panic in #1496. That should be fixed in #1508 once #1496 gets merged and I rebase #1508 onto master.

@crsaracco

@crsaracco Does #1508 satisfy requirement 2 on Windows?

Sorry Osspial for sleeping on this for a while, I sorta ragequit working on VST GUIs for a while 😅.

I feel my interest coming back around a little, so I might look into this again pretty soon.

Although I will say, I'm mainly interested in Linux right now, so we'll see I guess.

Just to (semi-)clarify the current state of this issue for those, who are watching for it:
the first part (#159 (comment)) seems to be possible on macOS too. WindowExtMacOS::ns_view (or maybe HasRawWindowHandle) allows you to make it a subview of another view using NSView::addSubview method from Cocoa. But I'm not sure what should be done and what happens with the window containing this view 😄

I made an example for the "child window" on macOS. It uses winit and Iced. There're some issues though. Contributions are very welcome. For VST you would use EventLoop::run_return inside the idle function.

In Linux X11, I could create a "child window" with egui for my VST3 plugin. It's used the fork repositories include changes equivalent to #2246. Using the result of that PR like this,

fix(macos): Fix incorrect Redo and CloseWindow accelerators (rust-win…

X11 support was added for that.

Is this not completely covered by WindowBuilder::with_parent(), which is only missing Wayland (is it possible there?).

If yes we could close this and open a new tracking issue where we clarify what's missing.
(apologies in advance, I didn't read through all that)

It all boils down to what kind of child windows we want here, it's either subviews (the ones constrained inside the parent) or when you just attach on-top of arbitrary window which may not even come from winit.

In general, both are not complete(subviews are not present at all). And both possible on Wayland, though child window is a bit limited there(not suvbiews).

Is this not completely covered by WindowBuilder::with_parent(), which is only missing Wayland (is it possible there?).

If yes we could close this and open a new tracking issue where we clarify what's missing. (apologies in advance, I didn't read through all that)

If you're on the same wl_display connection, you can establish the surface as a wl_subsurface. I actually need this functionality for writing an emulator, as the library I'm using (mupen64plus) seems to need its own surface.

Navigation Menu

Repository navigation

Support for creating child windows #159

20 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Support for creating child windows #159

Description

Activity

monomadic commented on May 27, 2017

conundrumer commented on Mar 18, 2018

Boscop commented on Mar 19, 2018

monomadic commented on Mar 25, 2018

Boscop commented on Mar 25, 2018

monomadic commented on Mar 26, 2018

francesca64 commented on Apr 8, 2018

monomadic commented on Apr 8, 2018

francesca64 commented on Apr 8, 2018

monomadic commented on Apr 9, 2018

francesca64 commented on Apr 11, 2018

monomadic commented on Apr 11, 2018

francesca64 commented on Apr 19, 2018

20 remaining items

crsaracco commented on Dec 12, 2019

Osspial commented on Mar 10, 2020

Fredemus commented on Mar 15, 2020

perlindgren commented on Mar 17, 2020

Fredemus commented on Mar 17, 2020

Osspial commented on Apr 19, 2020

crsaracco commented on May 15, 2020

ales-tsurko commented on May 28, 2020

ales-tsurko commented on Jun 3, 2020

t-sin commented on Apr 15, 2022

kchibisov commented on Oct 9, 2022

daxpedda commented on Dec 24, 2023

kchibisov commented on Dec 25, 2023

jgcodes2020 commented on Feb 2, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions