-
Notifications
You must be signed in to change notification settings - Fork 53
Added plotLines() and plotHistogram() #53
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
Conversation
I am about to test these functions, so I am going to commit those for a backup.
The PlotLines() and PlotHistogram() are now tested and they can produce the exact same result from the web imgui demo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for ImGui's PlotLines and PlotHistogram functions to the zgui wrapper library. The implementation provides Zig bindings for native plot functionality that was previously unavailable.
- Added C++ wrapper functions
zguiPlotLinesandzguiPlotHistogramin zgui.cpp - Created Zig bindings with a
PlotNativestruct for consistent parameter passing - Exposed public
plotLinesandplotHistogramfunctions following library conventions
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/zgui.cpp | Added C++ wrapper functions for ImGui::PlotLines and ImGui::PlotHistogram |
| src/gui.zig | Added PlotNative struct and Zig bindings for the new plot functions |
Comments suppressed due to low confidence (3)
src/gui.zig:1833
- The field name 'v' is unclear and non-descriptive. Consider renaming it to 'values' to improve code readability and match the corresponding C++ parameter name.
v: [*]f32,
src/gui.zig:1834
- The field name 'v_count' should be renamed to 'values_count' for consistency with the 'values' field name and to improve clarity.
v_count: c_int,
src/gui.zig:1835
- The field name 'v_offset' should be renamed to 'values_offset' for consistency with other field names and to improve clarity.
v_offset: c_int = 0,
src/gui.zig
Outdated
| extern fn zguiTextLinkOpenURL(label: [*:0]const u8, url: ?[*:0]const u8) void; | ||
| pub const textLinkOpenURL = zguiTextLinkOpenURL; | ||
| //-------------------------------------------------------------------------------------------------- | ||
| const PlotNative = struct { |
Copilot
AI
Jul 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The struct name 'PlotNative' is ambiguous and doesn't clearly indicate its purpose. Consider renaming it to 'PlotArgs' or 'PlotOptions' to better reflect that it contains plotting parameters.
| const PlotNative = struct { | |
| const PlotArgs = struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bot is right. Let's use PlotArgs here for consistency.
.gitignore
Outdated
| *.DS_Store | ||
|
|
||
| # Ignore local example builds | ||
| example/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, seems I have missed that out when I was testing the library.
|
Done, I have made some changes according to the requests. |
hazeycode
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
No worries, and thank you for maintaining the libraries too! Along with other libraries in the zig-gamedev, this really solves my huge headache on setting up the backend for imgui. |
Because of the issue Add vanilla PlotLines binding by taylorh140 while I have use cases with the native plot functions, I have studied the library to see how to enable these functions. With extended the zgui.cpp and gui.zig, I have successfully enable these to functions as shown:
The inconsistent plot size is intentional because I was also trying to change the properties passed with a newly created struct named PlotNative used for the two plot functions; thus, the function call should aligns the conventions of other existing function as shown:
However, this is my first contribution, so please let me know if I have done a pull request properly.