Updating backend ImGui/ImPlot/ImNodes to latest versions

SamuMazzi commented 9 months ago

name: Updating backend ImGui/ImPlot/ImNodes about: Updated backends title: Updating backend ImGui/ImPlot/ImNodes assignees: ''

After a loooooot of work and time, we've finally finished the PR. A huge thank you to @v-ein for all the help and support.

Description

This PR aims to update the ImGui/ImPlot/ImNodes backends of DPG to way more recent versions.

Currently, DPG relies on really old version of these packages (~2/3 years old), now this is the situation before/after the PR:

ImGui: 1.83 (May 2021) -> 1.91.0 (July 2024)
ImPlot: 0.11 (May 2021) -> 0.17 (January 2024)
ImNodes: 0.5 (April 2022) -> updated to the latest available (March 2024)

Those versions are the latest available at the moment of merging the PR and they are supported by DPG but this doesn't mean that we implemented every single feature from those libraries (many are still missing like multi-selection, several flags in different components, new API for shortcuts, etc...).

Below will be listed a summary of the major changes and new features. We tried to be as much backward compatible as possible. And hopefully you may not need to change anything in your current code.

Issues solved

Fix #2173, #2013, #1593 (last 2 issues are closed but not solved, but they are all related)
Fix #2108
Fix #2016

Changes

Now there can be 3 x-axes other than the currently supported 3 y-axes
New StackToolWindow
New query API for plots. Now multiple queries can be created and managed. Check the demo and the new parameters in add_plot to have more info
- Note: From now on, the query callback receives coordinates as ((x1, y1, x2, y2), (x1, y1, ...)). This differs from the old behavior where the callback was getting (x1, x2, y1, y2), i.e. first the X range, then the Y range. The new format describes a sequence of rectangles in the plot's coordinate space.
Now label parameter in add_separator will put a label inside the separator
ctrl + tab navigation across windows independent from keyboard_navigation parameter
Add Presentation section to MetricsTool like in the original ImGui
enable parameter in add_group will apply mvStyleVar_DisabledAlpha to all its children

Here is a list of all the new and deprecated arguments and functions.

Note: all deprecated functions and arguments are still accepted by DearPyGui API. Some of them may still work, some are ignored, but none should crash your application.

New functions

add_axis_tag
add_bar_group_series
add_inf_line_series
add_digital_series
get_plot_query_rects
set_axis_limits_constraints
reset_axis_limits_constraints
set_axis_zoom_constraints
reset_axis_zoom_constraints

New arguments

Function	Arguments
`add_2d_histogram_series`	`col_major`
`add_button`	`repeat`
`add_child_window`	`always_auto_resize` `always_use_window_padding` `auto_resize_x` `auto_resize_y` `frame_style` `resizable_x` `resizable_y`
`add_colormap_scale`	`format` `mirror` `reverse_dir`
`add_combo`	`fit_width`
`add_custom_series`	`no_fit`
`add_drag_line`	`delayed` `no_cursor` `no_fit` `no_inputs`
`add_drag_point`	`clamped` `delayed` `no_cursor` `no_fit` `no_inputs` `offset`
`add_group`	`enabled`
`add_heat_series`	`col_major`
`add_histogram_series`	`cumulative` `horizontal`
`add_input_text`	`always_overwrite` `auto_select_all` `ctrl_enter_for_new_line` `escape_clears_all` `no_horizontal_scroll` `no_undo_redo`
`add_line_series`	`loop` `no_clip` `segments` `shaded` `skip_nan`
`add_pie_series`	`ignore_hidden`
`add_plot`	`max_query_rects` `min_query_rects` `no_frame` `no_inputs` `override_mod` `query_color` `zoom_mod` `zoom_rate`
`add_plot_axis`	`auto_fit` `foreground_grid` `no_highlight` `no_initial_fit` `no_label` `no_menus` `no_side_switch` `opposite` `pan_stretch` `range_fit` `scale` `tick_format`
`add_plot_legend`	`no_buttons` `no_highlight_axis` `no_highlight_item` `no_menus` `sort`
`add_scatter_series`	`no_clip`
`add_stair_series`	`pre_step` `shaded`
`add_stem_series`	`horizontal`
`add_subplots`	`share_series`
`add_table_column`	`angled_header` `no_header_label`
`add_text_point`	`offset`
`add_tree_node`	`span_full_width` `span_text_width`
`add_window`	`unsaved_document`
`configure_app`	`anti_aliased_fill` `anti_aliased_lines` `anti_aliased_lines_use_tex` `docking_shift_only`

Deprecated functions

add_hline_series: use add_inf_line_series()
add_vline_series: use add_inf_line_series()
get_plot_query_area: use get_plot_query_rects()
is_plot_queried: use get_plot_query_rects()

Deprecated arguments

Function	Argument	Explanation
`add_histogram_series`	`cumlative`	Deprecated because of a typo: use `cumulative`
`add_image_button`	`frame_padding`	Not supported anymore by Dear ImGui; still works in DPG but will eventually be removed.
`add_plot`	`anti_aliased`	Not supported by ImPlot anymore. To enable/disable anti-aliasing, use `dpg.configure_app()` with the `anti_aliasing` parameters.
`add_plot`	`no_child`	Removed in ImPlot as child windows are no longer needed to capture scroll.
`add_plot`	`no_highlight`	Removed because not supported by ImPlot anymore. To control the highlighting of series use the same argument in `add_plot_legend`.
`add_plot`	`query_button` `query_mod`	This refers to the old way of querying in ImPlot, now replaced with `add_drag_rect()`.
`add_plot_axis`	`log_scale`	Use `scale=dpg.mvPlotScale_Log10` instead.
`add_plot_axis`	`time`	Use `scale=dpg.mvPlotScale_Time` instead.
`add_text_point`	`x_offset` `y_offset`	Use the `offset` argument instead.

Nerd info (for developers)

Why?

ImGui is currently sponsored by big tech companies like Adobe, Blizzard, Activision, etc. so we can expect a lot of improvements with the update (there are already ton of them) and we can expect a lot of people working on the really ugly/hard things like graphical backend libraries.

At my workplace we are about to ship a product based on DPG. Because of my job, I had to edit the backend and then I found out that what I edited was already modified in the new ImGui and then I also found out that the ImGui version we were relying on in DPG was really old. We want to stick with this library but I honestly told them that I don't see the point of relying on an almost dead (okay, no, maintenance mode) library, so we decided to either upgrade it or drop it and replace it with something else.

I know that this is quite a huge change, and that several problems can arise but I also think that it can be definitely worth it. Also with this huge community all of these changes can be "easily" tested.

OpenGL

Currently, for the build, DPG relies on gl3w in the examples folder of ImGui. This doesn't sound like a really stable thing and indeed a couple of releases later, that's been removed, so I added gl3w directly in the thirdparty folder. Also ImGui decided to make some heavy changes in order to be more cross-platform. One of these, which is also my biggest concern about all of this work, is about the buttons of the keyboard. These are now less than before, and they have been all mapped to predefined values in ImGui. So if someone used them heavily now they could have some problem.

Major concerns

As I said earlier, now there are less keyboard buttons than before, I think that they'll add more in the future, but I don't know how to solve this problem.
In my workplace we have never used ImNodes so we haven't tested that a lot. There haven't been any huge updates during these years so there shouldn't be a lot (or any) problems. If someone's willing to help and take a look, they'd be welcome.

Notes

There are still tons of updates and new features that can be implemented (i.e. Shortcuts API, Multi select, and many others), then if you want to take a look on what happened check this for ImPlot and ImGui

In ~2022 ImGui moved all the docking stuff in a separate branch, so now we're relying on that branch. This is not a big difference because it's always rebased on the main branch on every update and should be merged in the master in the version 2.0

Everything was tested with Valgrind in order to be sure that there wasn't any kind of accidental memory leak (or at least not more than those who were already there).

My main goal here was to have a real feedback from the most expert users on this work.

I hope I was clear enough! I'd like to improve this library because I think it's really great, so any kind of suggestion will be well accepted.

bandit-masked commented 9 months ago

What a massive PR! It would be very cool if it would be merged. Grazie mille!

Is there a website for this product by your company?

Atlamillias commented 9 months ago

This is a great PR!

It looks like a lot of the breaking changes could be made to not be if desired via aliases on the Python side, which is nice.

SamuMazzi commented 9 months ago

What a massive PR! It would be very cool if it would be merged. Grazie mille!

Is there a website for this product by your company?

Really thank you! Here it is the link for the product web page, a public demo should be available before the end of the second quarter!

SamuMazzi commented 9 months ago

This is a great PR!

It looks like a lot of the breaking changes could be made to not be if desired via aliases on the Python side, which is nice.

Thank you! What do you mean with aliases? How would you use them in this case?

bandit-masked commented 9 months ago

Could you reply here when the demo is up? Or you could post on #showcase on Discord.

bandit-masked commented 9 months ago

Also, if you are looking to promote your app when it's done, I'd recommend posting on Reddit. I have some experience with that, so I'd could give a few suggestions if you like.

Atlamillias commented 9 months ago

This is a great PR!

It looks like a lot of the breaking changes could be made to not be if desired via aliases on the Python side, which is nice.

Thank you! What do you mean with aliases? How would you use them in this case?

"Alias" may not have been the best word.

For example; in the case of discontinuing add_area_series, implementing a function in dearpygui.dearpygui named add_area_series that wraps add_line_series to allow for proper deprecation instead of discontinuing it altogether. Keyword arguments that would be no longer necessary or simply unsupported with this merge could be handled similarly, since most high-level dearpygui functions accept variadic keyword arguments. The behavior of some of these may differ given the update, but at least it wouldn't break the API.

While I personally don't mind when updates are API-breaking (a benefit of me not needing to work on large projects, I suppose), DPG is supposed to be in "maintenence mode"; @hoffstadt and @Pcothren may not want to have a major release, which would likely be necessary given the API change.

bzczb commented 9 months ago

This branch seems to work well with an app that uses DearPyPixl, just needed to rename mvTag to mvPlotTag so all the machinery worked.

SamuMazzi commented 9 months ago

Also, if you are looking to promote your app when it's done, I'd recommend posting on Reddit. I have some experience with that, so I'd could give a few suggestions if you like.

Thank you, I appreciate that! I'll tell you something when it'll be the moment and I'll post something in the #showcase for sure

SamuMazzi commented 9 months ago

This branch seems to work well with an app that uses DearPyPixl, just needed to rename mvTag to mvPlotTag so all the machinery worked.

That's great, thanks! I really need feedback of someone testing it!

SamuMazzi commented 9 months ago

While I personally don't mind when updates are API-breaking (a benefit of me not needing to work on large projects, I suppose), DPG is supposed to be in "maintenence mode"; @hoffstadt and @Pcothren may not want to have a major release, which would likely be necessary given the API change.

Oh okay, got it, but I think this is gonna create a "complete" new version so maybe there won't be the need of making 100% backward compatible, but if they'll say so I'm sure we'll find a way... there aren't so many breaking changes

bandit-masked commented 9 months ago

Also, if you are looking to promote your app when it's done, I'd recommend posting on Reddit. I have some experience with that, so I'd could give a few suggestions if you like. Thank you, I appreciate that! I'll tell you something when it'll be the moment and I'll post something in the #showcase for sure

The main things to take into account are:

Day of the week and time of posting. There are websites dedicated to establishing the perfect day and time for each subreddit. The quick and simple rule is to post between 8 and 9 a.m. US East Coast time so that it can gather as many possible upvotes in a day as the post becomes trending as people start working.
Post a GIF or video without any text if possible (whether that's possible depends on the subreddit). If it's just a GIF or video, the GIF or video will be shown directly in the feed, which draws more attention and upvotes to it. You can post a comment on your own post with the backstory and links. Unfortunately, r/python, which is one of the biggest communities, doesn't allow for posting a video only. I'm not sure, but it used to pick up the image associated with a GitHub link as the thumbnail for your post. Better make sure to test this, e.g. on an unknown/your own subreddit, before posting to the big subreddits.
If you get a few upvotes really quickly in the beginning, that will boost the visibility incredibly and will start a domino effect. Hence, make sure people know you are going to post and get them to upvote within the first 10 minutes.
Besides the obvious subreddits such r/python, find other subreddits to show your application. People tend to show a relatively stronger response in smaller, yet active communities, especially if its a good match.

Good luck!

Atlamillias commented 9 months ago

This branch seems to work well with an app that uses DearPyPixl, just needed to rename mvTag to mvPlotTag so all the machinery worked.

Thanks for the heads-up! I'll need to test this PR against my apps that use DearPyPixl as well, although I don't expect I'll find many problems. Docking's one, I suppose.

v-ein commented 8 months ago

While I really appreciate all the work done and still being done behind this PR, and welcome any fixes that correct possible earlier mistakes in ImGui upgrade, can we keep unrelated commits in separate PRs? (maybe published later, once this PR gets merged).

I've noticed that commits continue to trickle into this PR, and some of them look unrelated to ImGui upgrade, which increases the work to review all this and might cause extra havoc if this PR needs any substantial rework.

Unfortunately I myself haven't had a chance to review the PR yet - my apologies on that, I'm quite bogged with other stuff. Hoping to get some progress this week...

SamuMazzi commented 8 months ago

While I really appreciate all the work done and still being done behind this PR, and welcome any fixes that correct possible earlier mistakes in ImGui upgrade, can we keep unrelated commits in separate PRs? (maybe published later, once this PR gets merged).

Yeah sure! I think you refer to the "recursive" functionality (as far as I remember it should be the only "extra" thing that I added)... I was actually uncertain of pushing that commit, but I'll revert it asap

And obviously no problem for the review :)

v-ein commented 8 months ago

Just a heads up - I'm reviewing this PR as time permits. It's hard to estimate progress - I think I'm still less than 20% into it. Since there's still a lot to review, I'm not making any comments yet - will be adding them later.

ArneSchulzTUBS commented 8 months ago

Thanks for the great work. How much effort would it be to directly use ImGui's docking branch so that we can keep the docking feature in DearPyGui ? If i understand correctly, their main and docking branch are regularly synced so there aren't too many differences in the core functionality.

v-ein commented 8 months ago

+1 for keeping the docking feature. That's one of the things I was going to discuss.

v-ein commented 7 months ago

While I'm still going through all the changes, I think it's time to start making some initial comments.

First of all, a disclaimer: the text below reflects my own opinion and not that of DPG devs (I'm not one of DPG authors). Also, some of my comments might seem a bit harsh - please bear with me, it's not because of any negative attitude, it's more like giving up on proper wording :slightly_smiling_face:.

Here are some key things (other comments will be attached to code lines as usual in a PR).

I'm highly upset with the way backward compatibility is handled in this PR. In my opinion, we should do a better job on it, to minimize the hassle for authors of DPG-based projects - and for their users who update DPG and then find that their favorite application no longer works (I'm sure I've seen unhappy comments from such users on earlier breaking changes in DPG). \ IMHO, every breaking change must have a stone solid reason behind it, and sometimes I couldn't find such a reason. I might be making more comments on this in regard to particular changes.
Also about compatibility: I'm not sure how much effort it would be to keep the docking feature, but as a DPG user I would definitely want to retain it. \ I think DPG was originally built based on the docking branch of ImGui, but I might be wrong (well, docking started in 2018 and ImGui 1.83 is from 2021). In ImGui, docking periodically (and quite frequently) pulls changes from master, so I don't see a reason why we can't build it on docking now.
As soon as this PR gets approved, it's going to be merged into the master branch, and there won't be a second chance for a cleanup. This means all debugging/diagnostic thingies must be cleaned out ASAP, lest they slip through into master. This includes all mentions of <iostream> and std::cout. \ Yes, I've seen a note on this in the PR description; what I'm afraid of is that this PR will not be treated as a draft but rather as a "production PR", will get merged, and we'll see those in the production code. (BTW, I'm treating it as "prod" now because it's not clear how to handle comments, changes, etc. if we create yet another PR).
A couple of words on the doc, including descriptions for all the new arguments, functions, etc. DPG is often used by people who have little programming experience overall, and absolutely no Dear ImGui knowledge. With that, any Dear ImGui terms that are not reflected in the DPG, let alone names of ImGui internals, will cause a lot of questions if mentioned in the doc. \ I can make a number of comments like "Needs a rewrite - this is an ImGui term", but I'd suggest that we go ahead and revisit the changes in the doc now (mainly mvAppItem.cpp) without me finger-pointing all the ImGui thingies.
- This applies to some arguments names too, which might be rather counter-intuitive for non-ImGui people. For these, I'll explicitly comment on them in the code. Personally I think that, while it's good to stick to original flag/function names (that come from ImGui/ImPlot/ImNodes), we don't have to sacrifice clarity for that, and can give something a different name if it makes sense in the context.

Making comments in the PR will take quite some time (maybe a couple of days), and I'm still halfway through the review, so get your popcorn ready! :popcorn: :grin:

v-ein commented 7 months ago

One more thing I forgot to mention:

I'd like to ask a favor of whoever makes changes in future - in other PRs as well! - to avoid unnecessary re-formatting, renaming etc. because it pollutes git blame, often for no reason other than taste. git blame is a very useful tool when it comes to understanding why the code was written the way it was written. Nothing critical, just keep that in mind.

This is also what I actually mean when I type something like "the old code worked the same way, why change it?"

v-ein commented 7 months ago

A couple of words on set_decimal_point:

At the moment, it is broken for add_input_float, add_slider_float, add_drag_float (and their double counterparts). That is, it only seems to work on add_input_text with either decimal=True or scientific=True. See this comment in ImGui ticket #6719.

Do we want to add a feature to DPG in such a broken state? If so, we need to make a warning either in the release notes or in the doc (or both), and eventually update ImGui once again to pull up the fix for this.

v-ein commented 7 months ago

A note on deprecated keywords:

I've suggested DEPRECATED_REMOVE_KEYWORD_ARG in a couple of places. For all these cases, we need to document:

why the keyword is being removed;
what should the app developer do to migrate to the new version (e.g. use a keyword in a different object, or simply remove the argument, or something else).

Not sure where to keep this doc, maybe just a comment in this PR? In the end, it should go to the release notes.

v-ein commented 6 months ago

A couple of words on declaring arguments deprecated. Right now, DPG has two options to deprecate an argument:

DEPRECATED_RENAME_KEYWORD_ARG will issue a warning and pass the argument to C++ with a new name.
DEPRECATED_REMOVE_KEYWORD_ARG will issue a warning and won't pass the argument at all.

I'd suggest that we add one more option, where a warning will be issued but otherwise the argument will be passed into the C++ part the way it did before, with its original name. This can be used for arguments that we want to retain for backward compatibility (maybe delete later), but also want to point the user to a newer, better way to do the same thing. We can name it simply DEPRECATED_KEYWORD_ARG. It will also need an extra message coded in argument definition, so that we can say something like "use argument X instead".

v-ein commented 6 months ago

A note on key modifiers in the plot (this seems to be the only place where we can pass modifiers):

While it might look natural to pass dpg.mvKey_None to disable the corresponding modifier, mvKey_None doesn't actually work. ImPlot uses ImHasFlag to check if the modifier is pressed. The way ImHasFlag is implemented, ImHasFlag(keys, 0) will always return true, not false. To disable the modifier, one can pass -1 instead of mvKey_None, which is totally non-obvious. We probably need to document this somewhere.

It would be even better to modify the code so that mvKey_None leads to false in modifier tests, but that means making changes to ImPlot or even ImGui, which is a long way to go.

v-ein commented 6 months ago

When adding new axes now you need to specify which ones (i.e. dpg.add_plot_axis(mvXAxis2))

This breaks the old behavior of having multiple Y axes of type dpg.mvYAxis. While I'm all for the new syntax with dpg.mvYAxis2 and dpg.mvYAxis3, I think we can somewhat reduce the pain for DPG users by auto-assigning multiple axes of type dpg.mvYAxis to Y1/Y2/Y3 depending on their order in the list. Here's a fix that does just that:

https://github.com/v-ein/DearPyGui-fixes/compare/c62d78d3c60efa041e8a00e8ef41910af2ed441c...bugfix/auto-y-axes

In addition, the fix removes axesNames and axesFlags from mvPlotConfig since they actually got useless with the updated ImPlot.

Here's a script to test it all:

import dearpygui.dearpygui as dpg

dpg.create_context()
dpg.create_viewport(title=f"Test - {dpg.get_dearpygui_version()}", width=500, height=750)

with dpg.window(label="Old way"):
    with dpg.plot():
        dpg.add_plot_axis(dpg.mvXAxis)
        dpg.add_plot_axis(dpg.mvYAxis, label="Y1")
        dpg.add_plot_axis(dpg.mvYAxis)
        dpg.add_plot_axis(dpg.mvYAxis, label="Y3")

if hasattr(dpg, "mvYAxis2"):
    with dpg.window(label="New way", pos=(0, 350)):
        with dpg.plot():
            dpg.add_plot_axis(dpg.mvXAxis)
            dpg.add_plot_axis(dpg.mvYAxis, label="Y1")
            dpg.add_plot_axis(dpg.mvYAxis2)
            dpg.add_plot_axis(dpg.mvYAxis3, label="Y3")

dpg.setup_dearpygui()
dpg.show_viewport()
dpg.start_dearpygui()
dpg.destroy_context()

(The label on Y2 is omitted deliberately in order to test label assignment).

SamuMazzi commented 6 months ago

While it might look natural to pass dpg.mvKey_None to disable the corresponding modifier, mvKey_None doesn't actually work. ImPlot uses ImHasFlag to check if the modifier is pressed. The way ImHasFlag is implemented, ImHasFlag(keys, 0) will always return true, not false. To disable the modifier, one can pass -1 instead of mvKey_None, which is totally non-obvious. We probably need to document this somewhere.

So you're telling to just put in the explanation of the parameters that you can disable that feature by just putting -1? (Ignoring the alternative of modifying implot itself) Otherwise tell me if I got it wrong

v-ein commented 6 months ago

Yeah, kind of that. We can even define a new constant, like dpg.mvKey_ModDisabled, to abstract off that "-1", and make it more readable. And then maybe add a note in setup.about since we already have a bit of text about modifiers there. Like this:

setup.about = "Adds a plot which is used to hold series, and can be drawn to with draw commands. For all _mod parameters use mvKey_ModX enums, or mvKey_ModDisabled to disable the modifier.";

v-ein commented 5 months ago

I'm done with my "first pass" through the changes. Once all comments I added above get resolved in this way or that, I'm going to scan through the changes once more (faster this time) and see if we still have any issues.

v-ein commented 5 months ago

Here is a list of all the new and deprecated arguments and functions - I think we'll need to add it to Release Notes. Going to update this list right before merging the PR.

Update 2024/07/22: re-generated the list based on the latest commit. Only the New arguments section has changed.

Note: all deprecated functions and arguments are still accepted by DearPyGui API. Some of them may still work, some are ignored, but none should crash your application.