Yatekii / imgui-wgpu-rs

Dear imgui renderer for wgpu-rs.
MIT License
256 stars 86 forks source link

add simple api #38

Closed benmkw closed 3 years ago

benmkw commented 4 years ago

Motivation:

Hello World with this api becomes:

fn main() {
    imgui_wgpu::simple_api::run(Default::default(), (), |ui, _| {
        imgui::Window::new(imgui::im_str!("hwllo world")).build(&ui, || {
            ui.text(imgui::im_str!("Hello world!"));
        });
    });
}

This API pulls in additional dependencies like winit so I added it under a feature flag which makes clear that API stability is not guaranteed to make maintenance easier and updating dependencies easier. Note that even if this breaks users code its still easier for the common case where it does not break and the existing copy paste option still exists if it breaks in a way the user can't fix immediately. Thus I think this is an improvement over the current state.

Maintenance should be fairly easy as its basically a copy of hello world with slight modifications. Doing this maintenance in one place makes it easier for users to update imgui-wgpu versions because they don't have to handle that API directly for simple cases (I myself have multiple small projects already where I end up fixing the same things repeatedly).

The config struct supports use cases which I would personally want and could potentially be expanded a bit. There is an inflection point where this simple API is the wrong level of abstraction though, its only supposed to be used for simple projects and also examples.

Feedback is very much welcome and wanted.

benmkw commented 4 years ago

ci fails because I added a code example to the docs

Yatekii commented 4 years ago

ci fails because I added a code example to the docs

You can fix this by marking it with no_run :)

benmkw commented 4 years ago

maybe after letting this sit for a bit it should also be mentioned in the Readme, not sure this has to happen immediately though

I found another CI issue, so cargo test runs without the optional features and thus the two examples I added failed. https://github.com/rust-lang/cargo/issues/4663

I can just remove them and put them in the docs with no_run, this will mean they might regress which is not nice. The other option would be to enable the feature by default which pulls in a bunch of dependencies which is also not so nice. I'd go for the docs option.

cwfitzgerald commented 4 years ago

You can run CI with --all-features to test everything.

benmkw commented 4 years ago

@4bb4 you might want to take a look at this and say if you think this is a good idea/ API cause you did something similar in https://github.com/4bb4/implot-rs/blob/master/implot-examples/implot-wgpu-demo/src/main.rs

Yatekii commented 4 years ago

Thank god you spotted all those typos my lazy ass didn't ...

benmkw commented 4 years ago

Thanks for the feedback :)

especially the thing with Fifo is something really helpful and a benefit of such an API as this can be done once and used again and again 👍

I used the API for one other implot thing already and it worked nicely.

benmkw commented 3 years ago

If we want to have the implot example in there I will have to PR the imgui version bump to implot after this implot-wgpu 0.12 release and then update this PR and move to a specific version or at least a specific commit of implot or else there will be imgui version issues. Because this makes maintenance harder we can also remove it and maybe it will be added to implot at some point which might be a better place.