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 wrapper for imgui PlotLines widget #50

Merged
merged 1 commit into from
May 7, 2020

Conversation

charlesdaniels
Copy link

This patch only wraps the PlotLines widget from imgui, but the histogram
and other plotting widgets could be wrapped also.

This also adds an example in examples/plot showing both g.PlotLines()
and g.PlotLinesV().

Future work: the imgui plotting widget is a little limited, it would be
nice for GIU to support a more robust plotting system. This could
perhaps be done from scratch based on a Canvas widget, possibly by
writing a back-end for gonum/plot.

This patch only wraps the PlotLines widget from imgui, but the histogram
and other plotting widgets could be wrapped also.

This also adds an example in examples/plot showing both g.PlotLines()
and g.PlotLinesV().

Future work: the imgui plotting widget is a little limited, it would be
nice for GIU to support a more robust plotting system. This could
perhaps be done from scratch based on a Canvas widget, possibly by
writing a back-end for gonum/plot.
@AllenDang
Copy link
Owner

@charlesdaniels Thanks! Checking this project, https://github.com/epezent/implot, it might be an option to integrate with.

@charlesdaniels
Copy link
Author

As an aside relating to future work, an improved plot would probably best be done by either binding epezent/implot or by waiting for it to be merged into imgui.

@AllenDang AllenDang merged commit 4fa2d64 into AllenDang:master May 7, 2020
@charlesdaniels
Copy link
Author

Haha I guess we had the same thought at the same time!

Do you have any thoughts on the right way to do that? Would you suggest creating a separate repo that binds implot into GIU, or should it be integrated directly into GIU?

@AllenDang
Copy link
Owner

@charlesdaniels Ha! I'm currently using https://github.com/gonum/plot to display complex chart, but the down side is the chart is static. To create a extendable plot system is necessary in future.

@AllenDang
Copy link
Owner

@charlesdaniels Few days ago, I'm think about to re-implement implot in golang...

@charlesdaniels
Copy link
Author

It looks like its just ~2.2kLOC, so it could probably be transliterated. Just skimming it very briefly I don't see any C++-isms that might be hard to translate such as classes or polymorphism. I think this would be a feasible project.

@epezent
Copy link

epezent commented May 7, 2020

Hi! Just popping in to say hello. Please let me know if there are any changes that I can make to ImPlot so that your integration is easier. Also, note that @sonoro1234 made a C binding here:

https://github.com/cimgui/cimplot

@charlesdaniels
Copy link
Author

I've been mucking about with Cgo lately... I will give this a try. The imgui bindings that GIU is based on bind directly against imgui, with only a few thin C functions that convert some object types to void * and some extern functions to shuffle things around.

I assume cimgui is similar, but the main thing I'm unclear on is how imgui itself internally handles state.

I don't see anything obvious in imgui-go that looks like it's storing a pointer to somewhere that the state lives, so I assume it's a "global" within imgui itself? @epezent you are obviously a lot more familiar with the C++ version of imgui, so maybe you could comment on this? As far as changes, the most I think we would need would be a few #ifdefs at most. I will open a PR if it's applicable.

What I'm concerned about is whether or not the imgui stuff in implot will be able to "find" the imgui stuff in imgui-go.

I think if we can get bindings working, that would be a lot less work than transliterating (and then keeping up with upstream updates). It's not like GIU doesn't already implicitly depend on Cgo via imgui-go.

What I think makes the most sense if @AllenDang is not opposed, and what I will try to do when I get a chance, will be for me (or someone else) to create a repo that's just the imgui-go bindings for implot, then have GIU be a down-stream consumer of that library.

(disclaimer for @epezent -- I am not really affiliated with GIU, I'm just a drive by committer for now... but a drive by committer with an interest in live GUI plots in Golang)

Thanks for stopping by!

@epezent
Copy link

epezent commented May 7, 2020

Hmm, this is above my pay grade (never touched go or Cgo), but might it be possible to just merge implot.h and implot.cpp with the ImGui sources and regenerate the imgui-go binding?

@AllenDang
Copy link
Owner

@epezent Thanks for your great job. And @charlesdaniels I think it will be better to embed implot.h and implot.cpp inside giu. The benefit is easier to change the cpp code to suit giu's need.
Like what I did to the codeeditor. You think?

@AllenDang
Copy link
Owner

BTW, @charlesdaniels I think the PlotLinesV could use two float32 params to define size. This will be easier to use and omit to import imgui package. Can you make a new PR to that?

@charlesdaniels
Copy link
Author

Ok, I started messing with binding implot, and I've hit a bit of a wall...

https://github.com/charlesdaniels/implot-go

Namely, implot is having trouble including imgui.h. I'm not very good with Cgo yet... I have // #cgo CXXFLAGS: -std=c++11 -I./imgui -I./implot and yet I still get this error implot/implot.h:26:19: fatal error: imgui.h: No such file or directory. I think it would definitely be easier if implot was in-tree with imgui-go. I'm not sure how they feel about adding external extensions that aren't part of imgui itself though.

@AllenDang
Copy link
Owner

@charlesdaniels Add it directly into giu/imgui

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

Successfully merging this pull request may close these issues.

None yet

3 participants