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

Missing Bindings: ImPlot plot methods with length #261

Closed
phraktle opened this issue Aug 20, 2024 · 8 comments · Fixed by #264
Closed

Missing Bindings: ImPlot plot methods with length #261

phraktle opened this issue Aug 20, 2024 · 8 comments · Fixed by #264
Assignees
Labels
missing binding Some of the original library API is missing

Comments

@phraktle
Copy link

phraktle commented Aug 20, 2024

Version

1.87.1

What part of the binding has gaps?

implot

What is missing?

1.86.x exposed the length parameter on plot* methods. This is no longer available in 1.87.0 bindings, so one can only plot the full buffer. However there are legitimate use-cases where one needs this (eg. resizable buffers).

@phraktle phraktle added the missing binding Some of the original library API is missing label Aug 20, 2024
@SpaiR SpaiR self-assigned this Aug 20, 2024
@SpaiR
Copy link
Owner

SpaiR commented Aug 21, 2024

This can be done, but there's one issue. In about half of the methods, there is an optional int argument that comes right after the buffer size argument, which creates a naming collision. There are a few ways to resolve this - one option is to change the buffer size parameter to a long type to create a unique signature. Or to add a suffix like "V" to the method name on the Java side (e.g., plotLineV).

Since I don't use this functionality, I'm curious to hear other perspective on what would be the more convenient approach.

cc @calvertdw (I know you're actively using ImPlot in your app, so this might be relevant for you as well.)

@phraktle
Copy link
Author

Do you mean the offset parameter? Maybe you could have two variants: one where neither is specified (which is convenient for the simple cases where you want to plot the full buffer) and one where both length and offset is specified.

@SpaiR
Copy link
Owner

SpaiR commented Aug 21, 2024

Yes, that seems like a viable solution. However, I'm interested in your use case. How do you use ImPlot functions in your work?

@phraktle
Copy link
Author

A common theme for me is visualization of real-time streaming data. Incoming new data is appended to a double[] buffer (two actually: timestamps for X, values for Y). When the buffers are full they are expanded (grown by a fixed ratio) – so the plot calls need to disregard the yet-unused region. And since these buffers get quite large over time, I want to avoid copying/double-buffering.

@calvertdw
Copy link
Contributor

I haven't had time to look at it yet, but just to say I'm open to changes! It's not hard for me fix them as we upgrade. I'll take a closer look in a bit.

P.S. @SpaiR I just saw the latest release, I'm so hyped to upgrade!! It's been a long wait :P

@calvertdw
Copy link
Contributor

calvertdw commented Aug 24, 2024

Finally got a chance to take a minute and understand this.

I am completely on board with @phraktle's suggestion:

Do you mean the offset parameter? Maybe you could have two variants: one where neither is specified (which is convenient for the simple cases where you want to plot the full buffer) and one where both length and offset is specified.

If that works, either you specify offset and length, or nothing. That would be perfect. I don't really like either appending a random letter or using a long. Using the long would be particularly bad, because it'd be really easy to accidentally pass the length as an int.

I also share the use case of real-time streaming and use the same approach of reusing larger buffers.

@SpaiR
Copy link
Owner

SpaiR commented Aug 26, 2024

I implemented the approach we discussed here. I believe that should help.
The changes are available in version v1.83.3

cc @phraktle @calvertdw

@phraktle
Copy link
Author

Thank you @SpaiR, 1.87.3 works well (and pretty fonts are also back :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
missing binding Some of the original library API is missing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants