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

[Eframe] Allow eframe to run on an existing event loop #2875

Open
rockisch opened this issue Apr 4, 2023 · 9 comments · May be fixed by #3340
Open

[Eframe] Allow eframe to run on an existing event loop #2875

rockisch opened this issue Apr 4, 2023 · 9 comments · May be fixed by #3340
Labels
feature New feature or request

Comments

@rockisch
Copy link

rockisch commented Apr 4, 2023

Is your feature request related to a problem? Please describe.
I want my program to have 2 separate winit windows, one with an egui interface managed by eframe and another managed by myself. Currently, this is not possible with eframe because the only API to start a window is run_native, which internally creates an event loop, and only 1 event loop can exist on a thread afaik.

For now, I am managing the egui window by myself using egui_winit and egui_wgpu, but there are a lot of caveats I need to take care (paint intervals, custom handling of specific events, etc), and if I wanted to integrate with epi it would take even more work.

Describe the solution you'd like
It would be great if eframe could implement:

  1. A way to initialize eframe through a different run method, perhaps run_native_ex, run_native_custom, run_native_managed, etc, which returns some kind of structure.
  2. An on_event method on said structure that returns the currently internal Result<EventResult> result struct + enum, so users can manage control_flow on their own.
  3. A paint method on said structure that paints stuff to the screen.

Describe alternatives you've considered
Could also be solved if you could have 2 event loops on the same thread. I'm not that knowledgeable on winit's internals, so not sure if that's possible on their part (since it's currently not allowed, I'm guessing it's not).

@rockisch rockisch added the feature New feature or request label Apr 4, 2023
@emilk
Copy link
Owner

emilk commented May 22, 2023

I'd be happy to review a PR in this area!

@LoganDark
Copy link
Contributor

LoganDark commented Sep 2, 2023

sdhgggggdfjghldksfjghksdfjg I want to run a softbuffer graphics context on the same winit event loop as an egui window and it is SO COMPLICATED to use egui-glow and egui-winit and eframe already includes all the mile long code including platform specific quirks but it's ALL crate-private. fork time :(

  1. making all this stuff public

    use eframe::{App, Frame, NativeOptions, UserEvent};
    use eframe::native::run::glow_integration::GlowWinitApp;
  2. modifying GlowWinitApp to have a method new_proxy that takes the Arc<egui::mutex::Mutex<EventLoopProxy<UserEvent>>> directly so I can clone a new one at any time rather than having to create new window up-front

  3. copying eframe::native::run::run_and_return in my own event loop (way easier than reimplementing the entirety of eframe)

is my way of making it work in a hacky way because I don't care about making PR-ready code when asking for one is the only response from the maintainer here

@emilk
Copy link
Owner

emilk commented Sep 4, 2023

@LoganDark thanks for outlining the steps required to get this to work!

If you're gonna make a fork and do the changes, why not just open a PR while you're at it?

I don't care about making PR-ready code when asking for one is the only response from the maintainer here

I don't follow. What more do you want from me? This issue touches on a use case I have no experience in, and is a perfect place for others to help out.

I've spent thousands of hours of my own time building egui, answering question, reviewing PRs, etc. egui also have had hundreds of other contributors, giving away their time for free so that you can enjoy egui. Is your time too valuable to open a PR, yet you still have time to complain?

@LoganDark
Copy link
Contributor

LoganDark commented Sep 4, 2023

If you're gonna make a fork and do the changes, why not just open a PR while you're at it?

it exposes a ton of private implementation details to the public API and I assumed you wouldn't accept it since it's just adding pub to a bunch of stuff with ~no other changes.

What more do you want from me?

Sorry, I was just frustrated at the time and it's really upsetting to see the exact issue I'm trying to solve addressed with a simple "PRs welcome". I had bad experience with that specific type of response.

Is your time too valuable to open a PR, yet you still have time to complain?

Value does not play a part in my decision-making, ADHD does. If you know anything about ADHD - don't lecture me for making a choice that sounds like it would be harder, or sounds like it wouldn't make that much of a difference. Words like this won't in any way encourage me to help out here.

@LoganDark
Copy link
Contributor

LoganDark commented Sep 4, 2023

Recording.2023-09-04.102432.mp4

Eh it looks like eframe has some global state or something that gets really mad when you have multiple windows open. Only the most recently created GlowWinitApp renders properly. Closing that one doesn't get the others to work either, and trying to create others after the event loop has already started seems to just not work (actually fixed by manually sending the Resumed event to new windows to get them to start up, but that still causes all existing windows to turn glitchy).

this could be a decent architecture though, because it allows you to access the apps themselves (and their fields) from the event loop (those buttons work by setting a field that the event loop checks for)

@LoganDark
Copy link
Contributor

Recording.2023-09-04.110820.mp4

wait. that was an issue with the glow backend specifically. with wgpu it works perfectly what-

@LoganDark
Copy link
Contributor

whatever. I don't know anything about OpenGL or Glutin, don't know why it's corrupting other windows when they are open, but if wgpu works fine then I guess that's what I'll use.

for what it's worth, making a PR for this would probably be probably easier than I thought because eframe's existing event loop implementation can be reimplemented in terms of this. And also it would probably remove the shameless copy and pasting between run_and_return and run_and_exit because they can both just be implemented as wrappers around forwarding events to a single WinitAppRunner (or whatever the struct ends up being called).

I'd still need your help to fix the glow backend and preferably with a bit less of whatever it is that was in your last comment but I've already done most of the work so it's probably fine.

@LoganDark
Copy link
Contributor

@rockisch
Copy link
Author

Hey @emilk, I actually do have a finished working implementation. I never submitted it because there are some design choices that need to be made regarding the exposed API. I'll try to open a PR later today with what I choose but listing the alternatives, since I think it'll be easier to reason about it looking at the code.

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

Successfully merging a pull request may close this issue.

3 participants