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

Discussion: maintainer change #392

Closed
Gekkio opened this issue Nov 26, 2020 · 16 comments
Closed

Discussion: maintainer change #392

Gekkio opened this issue Nov 26, 2020 · 16 comments

Comments

@Gekkio
Copy link
Contributor

Gekkio commented Nov 26, 2020

I've decided to stop maintaining imgui-rs, because honestly it feels like 100% chore at this point without any fun. The ratio used to be a bit different earlier when there were interesting technical challenges 🙂 but now many of those are solved and I mostly face annoying technical challenges caused by choices I can't influence, and I keep having to spend time trying to defend the choices I have made.

Simply put, there's so many other projects I'd much rather spend time on...I've never been especially passionate about user interfaces since my own needs are very simple and I don't think Dear ImGui is that great from a Rust perspective, no matter if it's wrapped by current imgui-rs implementation, or some hypothetical better alternative wrapper.

From a practical point of view, I think the repo needs to be moved under some organization and I need to give crates.io release rights to somebody. What happens after that is 100% up to the next maintainer(s) 😄 personally I will be probably switching to egui in the few projects that currently use imgui-rs

Let's discuss here who wants to be the new maintainer(s)!

If you'd like to maintain imgui-rs, please answer the following three questions. This is not a "job interview" and there's no strict requirements, but I'd like to have some idea who I'm passing the torch to 🙂

  1. Why do you want to maintain imgui-rs?

Maybe you have some grand plans for imgui-rs? Or maybe you currently use it and just want it to be actively maintained? It would be nice to know why you're interested. I have never been very passionate about imgui-rs and have managed to maintain it for years, but some passion probably helps if you want to be a good maintainer.

  1. Do you currently use imgui-rs?

It's not a strict requirement, but I think it's so much better if you use the thing you maintain, so I'd like to know if you use imgui-rs and how you use it. One of my own problems has been that I don't actually use imgui-rs very much, so I don't get to experience all pain points from a user point of view.

  1. What kind of qualifications do you have for maintaining a crate like this?

No need to have a master's degree in user interfaces, games, or unsafe code 😄 But I'd like to know if you already maintain any crates related to game development or have some other relevant skills which would be useful with imgui-rs.

@Gekkio Gekkio pinned this issue Nov 26, 2020
@thomcc
Copy link
Member

thomcc commented Nov 27, 2020

Weakly throwing my hat in the ring. I'm probably not the best candidate since I'm no longer as active in the gamedev space, but I have had plans in the past for it and would be more than willing to take it up.

  1. Why do you want to maintain imgui-rs?

    I have a lot of experience with the original ImGUI and think it's notably easier to use (and more flexible) in C++ than in Rust. Many times I've wanted to write another version of it (and think I have an unfinished one in my projects folder), but certainly taking over the existing project is another option.

  2. Not at the moment, no, and that's partially because I , but mostly because I haven't been active in the Rust gamedev scene recently.

  3. I currently maintain rusqlite (another popular FFI bindings crate), I have significant experience with the original ImGUI, and some experience with imgui-rs.

@AldaronLau
Copy link

@Gekkio I might also be interested in maintaining this crate. I would also be 100% on board with co-maintaining with @thomcc (if they want), as this crate has a lot of open issues and PRs.

  1. Why do you want to maintain imgui-rs?

I've been wanting to work on a GUI toolkit in Rust, but most of existing toolkits have different goals than I. I think this code base could be a good start - immediate mode GUI is seems like a good idea (and I like that it doesn't depend on any specific graphics backend). As for grand plans, I would want to re-write the entire C++ code dependencies into pure rust (making this crate no longer bindings, but rather a re-write into Rust). Luckily, I enjoy converting C/C++ to Rust. This crate could have a new home in the libcala organization that I started (since you mentioned you'd like it to be part of an organization). I would also be interested in improving the documentation and adding tutorials, as a at quick glance I'm struggling to figure it out (I'll figure it out, though, I'm sure). Also, if it doesn't currently work on mobile, I would need to change that. I would also add an additional focus on accessibility.

  1. Do you currently use imgui-rs?

No, but if I started maintaining it I certainly would (it solves issues I need solved, although I also would like it solved in pure Rust).

  1. What kind of qualifications do you have for maintaining a crate like this?

I currently maintain a crate called cala. I think it could be possible for cala to be used with imgui-rs, although I haven't tried yet. Cala currently provides cross-platform hardware support for various things like audio, graphics, gamepads, input, etc., but it's missing GUI widgets (one difference between Cala and other crates that solve similar issues, is that it's asynchronous, but also more modular). Cala is a project that I started myself, that includes most of the crate's dependencies (minus chrono). I have a lot of experience writing FFI code and maintaining somewhat popular crates, and I'm also working on a video game in Rust using cala, as well as other applications that need both a GUI and features from cala.

@thomcc
Copy link
Member

thomcc commented Nov 27, 2020

I would also be 100% on board with co-maintaining with @thomcc (if they want), as this crate has a lot of open issues and PRs.
...
As for grand plans, I would want to re-write the entire C++ code dependencies into pure rust (making this crate no longer bindings, but rather a re-write into Rust).

I'm not really interested in working on a rewrite so much as bindings to the existing library. I think there's value in having bindings, since there's a lot of plugins to https://github.com/ocornut/imgui that it would be nice to support too. These wouldn't be compatible with a Rust rewrite.

@AldaronLau
Copy link

Maybe then it should stay bindings, and perhaps I'm not the person for the job. I don't really see any documentation on plugins after doing a quick search, or even if this crate supports them, but I'll take your word on it. After reading about some of the author's design decisions, I think it would be likely closer to how they wanted it to live on if you were the maintainer instead of me.

@thomcc
Copy link
Member

thomcc commented Nov 27, 2020

plugins after doing a quick search, or even if this crate supports them, but I'll take your word on it

They're mostly just C++ code that gets compiled in. They aren't supported by the current version of the crate. I'd like to do a survey of the more popular ones (I know bgfx used to have a few) and consider bundling them as features if they're worthwhile. The docking branch is also arguably plugin-esque, but it requires a big change to the internals IIRC. That said, it's not a big deal.

I agree that keeping it bindings rather than a rewrite feels more in spirit with what the crate is currently, although a rewrite would certainly be an interesting project.

@thomcc
Copy link
Member

thomcc commented Nov 27, 2020

One example of such a plugin is ImPlot, which is requested in #384. More generally: https://github.com/ocornut/imgui/wiki/Useful-Widgets has a lot of things, many of which are good, although some of which are likely impossible to adapt in a general way.

Another issue I've had with Rust ImGUI in the past is ImString/im_str being kind of awkward and painful. I have some plans for this, as I have similar issues in rusqlite. I'd like to make it so normal rust &str works. This isn't done in the current API, as it seems like it would incur an allocation, but that can be avoided in two cases:

  1. If the string is already nul-terminated, e.g. it's passed in like `"blah\0". that's a little awkward but not so bad.

  2. Using a small-string-optimization approach for smaller strings. In rusqlite we have an internal SmallCStr which is pretty janky but I have a half-finished better version that I've been meaning to finish up (but havent gotten around to it because for rusqlite's case this isn't a huge deal).

That said, it's been a while, I don't remember if Dear ImGUI cares about pointer identity of the strings for its internals, and if so the 2nd approach wouldn't work.

@benmkw
Copy link
Contributor

benmkw commented Nov 28, 2020

plugins

I recently wrote bindings for a node editor (https://github.com/benmkw/imnodes-rs) (not published) because I liked https://github.com/4bb4/implot-rs very much.

I don't think Dear ImGui is that great from a Rust perspective

I disagree because wrapping a c like interface in rust made it much easier to use in the case of imnodes and I also think your API is nice and enjoyable (of course imgui/ imnodes/ implot being c-like only made it possible to port them in the first place).

To reduce maintenance burden I would suggest to remove the backends from this repo and move them to their own repo such as https://github.com/Yatekii/imgui-wgpu-rs and maybe use the same org for all of them. Thus updates don't need to happen at the same time but its still somewhat connected.
Maybe remove compatibility with old winit versions more quickly

The goal would be to make it possible for the people maintaining imgui-rs to not worry about the backends so much (if a backend only works with an older version its not a big issue) and also push more work onto the users (don't provide backwards compatibility so much).

I would reconsider adding a docking feature branch as from my point of view thats the only/ the major next API consideration which needs the most attention, apart from that imgui is not moving super quickly. Of course this cuts into the maintenance budget which is why it was not done.

Web:
In 6-12months when wgpu implementations have converged a bit more it will probably be easy to run imgui with wgpu in browsers as well.

Utilities:
I made a small file browser in rust/ imgui-rs and collected some imgui styles from github and this could be added to imgui-rs to make it easier to use. The same can be done for some other imgui "plugins" which are more like small widgets and don't need full bindings. Adobe also has an open source version of imgui with styling which would be nice to support.

rewrite in rust

For me imgui-rs was really pleasant to use, its by no means perfect but its a great baseline and provides an easy to use gui for rust today. Thus I think its valuable to build on top of a much used gui lib at the moment although this will hopefully be superseded at some point.

I've never been especially passionate about user interfaces since my own needs are very simple

Thats basically true for me as well, I just want to get something to work and show friends ... a simple tool thats easier to use than a terminal and imgui does just that.

The following is wip but compiles today and I think thats pretty nice and basically replaces matplotlib for simple things for me:

fn main() {
    let plotcontext = implot::Context::create();

    imgui_wgpu::simple_api::run(Default::default(), plotcontext, |ui, plotcontext| {
        imgui::Window::new(im_str!("example"))
            .build(&ui, || {
                let plot_ui = &plotcontext.get_plot_ui();

                implot::Plot::new("Simple line plot")
                    .build(plot_ui, || {
                        let x_positions = vec![0.1, 0.9];
                        let y_positions = vec![0.1, 0.9];
                        implot::PlotLine::new("legend label").plot(&x_positions, &y_positions);
                    });
            });

    });
}

This is not really an application, just my thoughts because I got a little bit involved with the plugin and backend stuff and am also using the lib.

@Gekkio
Copy link
Contributor Author

Gekkio commented Nov 29, 2020

Thanks to everybody for the comments! 😄
I agree it's a good idea to keep imgui-rs as a bindings library, instead of trying to reimplement stuff in Rust. A reimplementation would be interesting, and I can reveal at one point I started an experiment doing that, because I got so fed up with trying to understand some semantics of the C++ library. But I realized how much effort it would take, especially since the Dear ImGui APIs probably won't always translate to safe Rust without some redesign work.

Let's keep this issue open for a couple of days to see if we get more comments and/or potential maintainers. Based on the comments so far, I think @thomcc seems like a promising candidate for the next maintainer.

@ocornut
Copy link
Contributor

ocornut commented Dec 1, 2020

Another issue I've had with Rust ImGUI in the past is ImString/im_str being kind of awkward and painful. [...]

On the topic of string view / non-null terminated strings, on Dear ImGui side we've been eying all year at making the switch toward using string-view like (aka a pair of pointers, not requiring null termination), based on existing work from @bitshifter which we updated and tweaked.
See: ocornut/imgui#3038 (comment)

For whoever ends up taking over imgui-rs this could be a good project to undergo together (it involves cimgui as well). From what I understand this would be notable improvement for imgui-rs users.

@erlend-sh
Copy link

erlend-sh commented Dec 1, 2020

From a practical point of view, I think the repo needs to be moved under some organization and I need to give crates.io release rights to somebody.

For the org part, the Amethyst org could act as the neutral host to help with coordination and contributor on-boarding. We’ve done the same thing for other projects like rlua, specs, atelier-assets etc.

Former and active maintainers would have Owner privileges on the repo; our role would merely be administrative. And @Gekkio you would always have the option to move the repo again should you wish to do so.

@thomcc
Copy link
Member

thomcc commented Dec 1, 2020

On the topic of string view / non-null terminated strings, on Dear ImGui side we've been eying all year at making the switch toward using string-view like (aka a pair of pointers, not requiring null termination)...

For whoever ends up taking over imgui-rs this could be a good project to undergo together (it involves cimgui as well). From what I understand this would be notable improvement for imgui-rs users.

This is great news! I would be very excited to help out here. I'm familiar enough with C++ (I mean, I use it more-or-less daily at work) that I can help on that side if needed too.

For the org part, the Amethyst org could act as the neutral host to help with coordination and contributor on-boarding. We’ve done the same thing for other projects like rlua, specs, atelier-assets etc.

Former and active maintainers would have Owner privileges on the repo; our role would merely be administrative. And @Gekkio you would always have the option to move the repo again should you wish to do so.

I wouldn't be opposed to this, but I do worry it would lead people to thinking it required Amethyst to use. Maybe not though, I haven't heard of issues there with the other projects.

The other risk is if Amethyst dies out, the projects are under an org that doesn't inspire confidence — You see this with projects under the PistonDevelopers org, even if they're maintained (https://github.com/PistonDevelopers/glfw-rs, for example).

When I took over rusqlite I just made a new org (which all the previous and other current maintainers have control over as well), which is what I planned to do here if I'm chosen as the maintainer.

That said it does come with other benefits, such as visibility and the like so I guess it's hard to say.

@thomcc
Copy link
Member

thomcc commented Dec 3, 2020

So, I spent some time and wrote up some further things I'd like to do if I end up maintaining this crate.

Not all of these are fully thought out — some are downright half-baked, but it should at least give you an idea of some of the stuff I'm thinking about and am interested in.

This is all in addition to the stuff I mentioned already, and isn't really in any order, although clearly some is.

Familiarizing

  1. Add more documentation, possibly some more examples. In practice a lot of this I'll end up doing just as a way to familiarize myself with the codebase.

  2. Dig into the build/bindgen code. It actually seemed decent from what I saw, but have a lot of thoughts about how to do that stuff best from my time working on libsqlite3-sys, libmimalloc-sys, nss-sys (from when I worked on firefox — this never was put on crates.io for various reasons), and other similar crates...

Making the core/platform/renderer relationship more explicit

As I see it, there's basically three pieces users need to wrangle to use imgui-rs:

  1. The "core" imgui crate.
  2. A platform implementation (imgui-winit-support in this repo, and externally imgui-sdl2, imgui-glfw-rs).
  3. A renderer implementation (imgu-gfx-renderer, imgui-glium-renderer in this repo, externally imgui-wgpu).

This is highly decoupled and clean, but can be a bit tricky to wrangle since no part knows about all the other parts. I'd like to make the platform/renderer responsibilities and api a bit more explicit (ideally as pub trait Platform/pub trait Renderer exposed from imgui).

After this, perhaps even provide a imgui-app crate (or maybe imgui-bundle? Naming is hard) that would intended to be for the final binary that uses imgui, and helps combine these separate parts in a nice way.

To a certain extent we have something like this in the imgui-examples support code, although it also demands a lot more control over the application than I think is good for a library like imgui, so would need adjusting.

More directly supported renderers / platform crates.

Some things that in the past have stopped me from using ImGUI in projects:

  1. No first party wgpu renderer. Now there's imgui-wgpu, which might be good enough? Worth investigating.

  2. No good non-glium GL renderer. Glium is a decent enough crate but it kind of demands full control over GL.

    • I have some bindings in an unpublished project using the old gl-rs crate, but I think people prefer glow now? Worth looking into.
    • imgui-opengl-renderer exists but hasn't been updated in over a year.
  3. No very solid and well-maintained SDL2 platform layer. imgui-sdl2 is only sorta maintained? The github repo isn't updated for all the updates... It's also not on the latest imgui... Unclear what the story is here...

  4. Ditto the previous, but for GLFW.

    • I have a soft spot for glfw since I used it at a previous job, and it always worked well. (I think SDL is more popular now, not that it matters that much)

I also will probably reach out to the maintainers of any of these that are active, and try to find out if there are any particular issues, if they want to keep maintaining it or would rather see it integrated, if they want to collaborate on it in other capacities, etc.

Wider target support (or better docs for it where it's already supported)

  1. This may require some sort of platform binding in addition (or perhaps it's well supported by winit, or by emscriptens reimplementations of glfw/sdl2), but it should be possible to use imgui with wasm, given the right renderer/platform.

    • EDIT: Actually, it looks like we already have wasm bindings? Imagine that!
  2. Mobile support too. At my last job I did lots of rust dev on mobile (firefox for android/ios) so I have a good amount of experience here and am not really worried. I suspect this already works if you know how to finagle it, but examples/docs would be cool.

Wild dreams

  1. Better thread safety. Dear Imgui isn't a thread-safe library, which makes it a bit painful to bind in Rust.

    • I think one option might be batching some changes in a window and synchronizing when the window drops? Is there a way that's possible without replicating a shitload of imgui internal state and logic? Hopefully! Short of that, perhaps just taking a mutex?

    • It seems like we use a recursive mutex for something already. I clearly need to dig deeper here!

  2. How far are we from no_std (+ alloc) support?

    • I think not very close, but would enable some nice use cases. My recollection Dear ImGUI doesn't require that much of a runtime.

Minor stuff

These are teeny, but I got a bit excited and started did these these in the tentative branch in my fork.

  1. Make imgui-winit-support's features behave more additively, and document them better.

    • This is mostly because it was causing rust-analyzer to complain in my editor and I was surprised how many versions were supported, so I did some digging.
    • I'm still not 100% sold that what I did there was actually an improvement beyond the docs, though — perhaps just failing to compile directly was better.
  2. Remove lazy_static and use the parking_lot::const_recursive_mutex function instead.

    • This is just because I knew it was no longer necessary (I have some background in this space, from before parking_lot supported this).

@ocornut
Copy link
Contributor

ocornut commented Dec 3, 2020

No first party wgpu renderer.

I'm not sure if you mean Rust or core imgui side, but a pull-request for a imgui_impl_wgpu renderer has been submitted this week in the main imgui repo. It's reasonably clean and I expect it shouldn't take too long to finish (see ocornut/imgui#3632)

Wild dreams: better thread safety

Library unfortunately isn't designed for that so currently only a mutex would work. I'm not aware of UI libraries that support parallel write/append. The only thing which is possible is having multiple imgui contexts (in situation where e.g. you want a "render thread" vs "update thread" imgui). For that specific use case I expect down the line to provide utilities to facilitate compositing of multiple contexts (input wise and maybe facilities such as cross-context drag and drop).

@thomcc
Copy link
Member

thomcc commented Dec 3, 2020

Ah, I'm not trying to change the C++ Dear ImGUI library here either, so much as thinking of improvements to the Rust bindings to it. Sorry that wasn't more clear.

I'm not sure if you mean Rust or core imgui side

Rust side, since in practice Rust code will end up wanting this. I think we don't currently use any of your imgui_impl_* code directly, since in practice it ends up being worth it to just rewrite it rather than have to go over the FFI.

Library unfortunately isn't designed for that

Yeah, I'm aware.

so currently only a mutex would work

I suspect in the no-contention case this would be fine, since parking_lot is very good when there's no contention (it's still typically better than system mutexes under contention too).

Taking a mutex around every call could easily result in mismatched begin/end — we'd have to hold it the whole time the window is built probably. I think this would work — I suspect in practice contention is low, Rust just requires there to be a mutex to avoid the chance of data races (which would be memory unsafe).

Note that some of this might already exist — I noticed there is a global mutex in the rust bindings, but I'm not exactly sure when/where it's taken (yet).

That said, this is not really ideal still. Plenty of Rust entity management libraries will run your code on whichever thread is convenient. It would be nice for imgui to work there, but yeah, gui libraries almost never do.

One thought broadly I had was, perhaps there's some cleverness we might be able to do on the Rust end here where calls get sent through a channel to imgui. Obviously naively this wouldn't work as-is, since the communication is bidirectional, though.

The only thing which is possible is having multiple imgui contexts

Yeah, I see. I had assumed this was mostly impossible with how the library worked, but hadn't dug into it much, hrm.

@Gekkio
Copy link
Contributor Author

Gekkio commented Dec 4, 2020

I think it's pretty clear now that @thomcc has the right skills and a lot of good ideas for imgui-rs, so...the job is yours 😄 🎉
Let's discuss via e-mail how to proceed in practice.

About the mutex: 99% of API functions in Dear ImGui operate using a global mutable static context which can be switched. In imgui-rs, this is modeled using "active" and "suspended" contexts, so you can have multiple context structs but only one of them can be active. Operations that switch the active context are behind the global mutex, but API functions that use the active context (99% of the API) are not, because just by having a Context we have a guarantee it's the current active one. In practice this means imgui-rs allows you to construct and render more than one UI in sequence but not in parallel, because that simply is not possible while upstream uses one instance of global mutable state for almost everything. I don't think the imgui-rs API is super great or useful, but at least it's possible to use more than one context without UB or breaking core rules of Rust.

About no_std: Dear ImGui requires an allocator, but it's switchable. In theory it should be possible to use a custom allocator that operates with preallocated memory instead of the default heap. imgui-rs doesn't have a huge amount of places where allocation is needed but in some places Vec/String is used.

@thomcc
Copy link
Member

thomcc commented Dec 4, 2020

And it's done! I hope everybody is happy with the direction I take imgui-rs!

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

No branches or pull requests

6 participants