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

Add Bindings for implot #53

Closed
charlesdaniels opened this issue May 26, 2020 · 8 comments
Closed

Add Bindings for implot #53

charlesdaniels opened this issue May 26, 2020 · 8 comments

Comments

@charlesdaniels
Copy link

charlesdaniels commented May 26, 2020

See relevant discussion here.

I am opening this issue to track progress and discuss binding implot into GUI.

I had started looking at this and then got distracted. Hopefully I will get at least something basic going before I get distracted again :).

So far, I have really, really basic sin() plot working (relevant commit). I will continue working in the same fork of GIU until this is far enough to open a PR.

If @AllenDang would prefer, we can close this, and open a "WiP" pull request, to remain un-merged until ready.


Pics or it didn't happen:

screenshot_2020-05-26_at_15_56_48

@AllenDang
Copy link
Owner

@charlesdaniels Issue of PR both is ok :). Thanks for your contribution!

@charlesdaniels
Copy link
Author

Ok, I got the line plot demo implemented. I think this lays most of the groundwork for how the rest of the imgui-go part of the bindings aught to work.

Here are a few issues I think are worth discussing:


@AllenDang -- thoughts on how we should wrap the imgui binding up as a GIU widget? The biggest issue I think is the several different ways to generate plots -- you can use a getter callback, a single list (index used as X axis), a list of X,Y points, or a list of ImVec2. I don't think it makes sense to have four separate widgets... maybe the PlotLineWidget could be an interface with 4 implementations? I'd like your insight on this, since I want the API to be consistent with the rest of GIU.

Also, thoughts on whether there should be one big fat PlotWidget that handles all the types, or if there should be a separate PlotBarsWidget, PlotScatterWidget, and so on? Notably, not all type of data are supported for all plot types.


@AllenDang -- it would be nice if you could update our in-tree fork of imgui-go. I had to manually back-port support for collapse-able headers. This wasn't a big deal, but it looks like there are some nice updates that we are missing.


There are some issues with getting the flags to come across the FFI, see implot #43.


It is probably worth talking to the inkyblackness/imgui-go folks at some point. They may be interested in this binding, and I'm of the mind that such things should live as far up-stream as possible.


@AllenDang -- It seems like it might be better to move the implot stuff into giu/imgui/implot or giu/implot rather than giu/imgui, because I've already bumped into some name conflicts. I think I would have the same troubles with getting Cgo to find things as I did when I tried to do this out of tree. Do you have any thoughts on this?

@AllenDang
Copy link
Owner

@charlesdaniels
For the parameters to generate plot, I prefer to only provide one implementation with simple raw value array. This makes it's easier to understand.

For update imgui-go from up stream, it's a little bit complex, since I've changed a lot (delete some methods, change some interface, etc...). Can you tell more specific what are those nice updates? I could merge them in manually.

For the location to put implot, I suggest to put *.h *.cpp in giu/imgui to let cgo work more easily.

@charlesdaniels
Copy link
Author

@AllenDang

For the parameters to generate plot, I prefer to only provide one implementation with simple raw value array. This makes it's easier to understand.

I think that's reasonable. If people really need to do something special, they can always use the new Wrapper Widget that I created.

I will plan to go ahead and write bindings for the full thing, that way if the imgui-go folks or someone else on the 'net wants it, they will have it. We can then pick and choose what functionality to wrap at the GIU level (e.g. only supporting x,y points as you said).

For update imgui-go from up stream, it's a little bit complex, since I've changed a lot (delete some methods, change some interface, etc...). Can you tell more specific what are those nice updates? I could merge them in manually.

This was the commit I was referring to. I got my CollapsingHeader from here. It's a simple widget, but a handy one I think. This commit has a few other goodies in it.

For the location to put implot, I suggest to put *.h *.cpp in giu/imgui to let cgo work more easily.

This is how I have it right now.

@AllenDang
Copy link
Owner

@charlesdaniels Any progress? I'm eager to try it.

@charlesdaniels
Copy link
Author

I got a bit bogged down in writing all the wrapper code, and ultimately had to set this aside for other obligations I'm afraid.

I think I have the general pattern of how it should work ironed out, so finishing this is ostensibly just "a small matter of programming".

If I have time in the future, I may pick this back up, but for now I'm afraid I have to table it.

@AllenDang
Copy link
Owner

@charlesdaniels Got it. It's ok.

@alfarok
Copy link

alfarok commented May 16, 2022

Has there been any progress on this? I am interested in using some of the other plotting methods included with implot, such as the PlotCandlestick():

candlestick charts

I tried to re-create them using just bar and line plots but the bar plots always begin at 0. Any suggestions on how to tackle this would be greatly appreciated, thanks!

@AllenDang @charlesdaniels

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

3 participants