imgui icon indicating copy to clipboard operation
imgui copied to clipboard

Demo code markers (imgui_demo.cpp)

Open pthom opened this issue 4 years ago • 43 comments

Hello,

This PR adds some "DemoCode" markers inside imgui_demo.cpp. They are used by imgui manual.

Basically, DemoCode and DemoCode_ are two macros that can optionally add a "Code" button like this image (DemoCode_ with an underscore will call ImGui::SameLine() first and align to the right).

By default, they will do nothing, unless gImGuiDemoCallback is not null. gImGuiDemoCallback is an optional callback, which is defined like this:

typedef void (*ImGuiDemoCallback)(int line_number, const char* demo_title);
ImGuiDemoCallback gImGuiDemoCallback = NULL;

For example, imgui manual uses it like this:

// Redefinition of ImGuiDemoCallback, as defined in imgui_demo.cpp
typedef void (*ImGuiDemoCallback)(int line_number, const char* demo_title);
extern ImGuiDemoCallback gImGuiDemoCallback;
 
....
gImGuiDemoCallback = implImGuiDemoCallbackDemoCallback;

As an example, the following extract from imgui_demo.cpp:

    DemoCode("Widgets");
    if (ImGui::TreeNode("Basic"))
    {
        DemoCode("Widgets/Basic");

        DemoCode_("Widgets/Basic/Button");
        static int clicked = 0;
        if (ImGui::Button("Button"))
            clicked++;
        if (clicked & 1)
        {
            ImGui::SameLine();
            ImGui::Text("Thanks for clicking me!");
        }

        DemoCode_("Widgets/Basic/Checkbox");
        static bool check = true;
        ImGui::Checkbox("checkbox", &check);

        DemoCode_("Widgets/Basic/RadioButton");
        static int e = 0;
        ImGui::RadioButton("radio a", &e, 0); ImGui::SameLine();

Can then be parsed into a hierarchical table of contents like this:

image

(Side note: in the imgui manual code, the number of "/" denotes the title hierarchy level).

And will display buttons like this (if gImGuiDemoCallback is not null): image


Feel free to close this PR if you think it is not appropriate, since it concern an aside project (imgui manual).

I suspect that the fact that imgui manual's in itself is not using orthodox C++ can be an issue for you; and that it might be the reason why you did not comment any further on it (and I can totally understand this :-).

However the modifications in imgui_demo.cpp are agnostic to the way the callbacks are made (so that another GUI could be built around them).

Also, IMHO the "DemoCode" markers can help navigate in the imgui_demo.cpp file. For example, below, the code below at line 3782 becomes clearer if we add a DemoCode marker that summarize the hierarchical context of what we are reading.

    if (ImGui::TreeNode("Padding"))
    {
        DemoCode("Tables & Columns/Padding");

pthom avatar Jan 02 '21 15:01 pthom

Thank you for the update Pascal!

Feel free to close this PR if you think it is not appropriate, since it concern an aside project (imgui manual). I suspect that the fact that imgui manual's in itself is not using orthodox C++ can be an issue for you; and that it might be the reason why you did not comment any further on it (and I can totally understand this :-).

I'm mostly generally overwhelmed by amount of tasks in-flight so didn't give any time to your idea yet. I'm generally favorable to it but you are right that the C++ use in imgui_manual was off-putting for dear imgui codebase and didn't help moving it nearer the top of the pile :) As a general rule of thumb, if I feel a proposal needs too much polish work on my side it's likely to move lower in the pile.

Some feedback:

  • I feel it might be adequate it to make the macros upper-case there, they'll more self-explicit as code markers.
  • The button identifier doesn't matter so you could use ("Code##%d", __LINE__) instead, of simply PushID(__LINE); Button(); PopID(); to avoid the printf.
  • There are hard-coded size contents in the code (the 70 seems that it should be CalcTextSize("Code").x + style.FramePadding.x * 2.0f and the 25.0f I don't know).
  • I would go all in and include FILE in the macros to potentially allow this to be used in other code.
  • Various glitches are visible with elements being offsets, e.g. image
  • Positioning could be improved for cases where there is no space: image Instead of using ImGui::SameLine(ImGui::GetWindowSize().x - 70.f); it could be something like Max(GetCursorPos().x, GetWindowContentRegionMax() - button_width) where button_width == CalcTextSize("Code").x + style.FramePadding.x * 2.0f.
  • The arbitrary naming of DemoCode() vs DemoCode_() is a little odd to expose to the end-user and generally the output with those empty lines seems to odd to me, why not trying to have everything on existing lines instead of using empty lines?
  • **Generally if you replace the first test with if (ImGui::GetIO().KeyShift && gImGuiDemoCallback) and use a key modifier to toggle visibility I think we should aim to have other items not move.

One good aspect of this is that we could later aim to implement a basic dependency-free version of your solution in imgui_demo., probably less polished, with less fancy text editor.

ocornut avatar Jan 11 '21 11:01 ocornut

As a quick experiment here's what I tried:

static void _ShowDemoCallbackButton(int line_number, const char* demo_title, bool before_widgets)
{
    //if (!gImGuiDemoCallback)
    //    return;

    ImVec2 backup_cursor = ImGui::GetCursorPos();
    if (!before_widgets)
    {
        ImGui::SetItemAllowOverlap();
        ImGui::SameLine();
    }

    float x = ImGui::GetContentRegionMax().x - ImGui::CalcTextSize(">").x - ImGui::GetStyle().ItemSpacing.x;
    if (x <= ImGui::GetCursorPosX())
    {
        //ImGui::SetCursorPos(backup_cursor); // Hide when there's no room, less ugly?
        //return;
    }
    else
    {
        ImGui::SetCursorPosX(x);
    }

    ImGui::PushID(line_number);
    if (ImGui::SmallButton(">"))
    {
        if (gImGuiDemoCallback)
            gImGuiDemoCallback(line_number, demo_title);
    }
    ImGui::PopID();
    if (ImGui::IsItemHovered())
        ImGui::SetTooltip("Code for section '%s' at line %d", demo_title, line_number);

    if (before_widgets)
        ImGui::SetCursorPos(backup_cursor);
}

#define DEMO_MARKER(demo_title)         _ShowDemoCallbackButton(__LINE__, demo_title, false)    // Submit button from after item/line submission
#define DEMO_MARKER_BOL(demo_title)     _ShowDemoCallbackButton(__LINE__, demo_title, true)     // Submit button from before item/line submission

image

Still doesn't look super great with lots of widgets though.

ocornut avatar Jan 11 '21 11:01 ocornut

Here's a completely different idea:

  • We make the marker record the current position ImGui::GetCursorScreenPos() + current ClipRect.
  • We always call the marker function at the top of a block. So essentially the marker would implicitly define zones assumed to be rectangles.
  • The marker macro doesn't display anything.
  • Move all markers before containers

Now we have zones:

image

From there, the documentation window (in imgui_manual or can be prototyped in imgui_demo) can use those rectangle for picking a section. E.g. has an always visible "CODE LOOKUP" button which use the position of markers and position of mouse to highlight the current section and allow picking from one. e.g. using similar visuals as RenderRectFilledWithHole() to dim other parts of the screen.

Hacky experiment above:

static void _ShowDemoCallbackButton(int line_number, const char* demo_title, bool before_widgets)
{
    struct Marker
    {
        ImVec2 Min;
        ImVec2 Max;
        ImVec2 ClipMin;
        ImVec2 ClipMax;
        char   Desc[256];
    };

    static Marker marker;

    ImVec2 curr_pos = ImGui::GetCursorScreenPos();

    // Display previous marker
    marker.Max.x = ImGui::GetWindowPos().x + ImGui::GetContentRegionMax().x;
    marker.Max.y = curr_pos.y;
    ImGui::GetForegroundDrawList()->AddRect(marker.Min, marker.Max, IM_COL32(255, 255, 0, 255));
    ImGui::GetForegroundDrawList()->AddText(marker.Min, IM_COL32(255, 255, 0, 255), marker.Desc);

    // Register new marker
    marker.Min = marker.Max = curr_pos;
    marker.ClipMin = ImGui::GetWindowDrawList()->GetClipRectMin();
    marker.ClipMax = ImGui::GetWindowDrawList()->GetClipRectMin();
    strncpy(marker.Desc, demo_title, IM_ARRAYSIZE(marker.Desc));
    marker.Desc[IM_ARRAYSIZE(marker.Desc) - 1] = 0;
}

ocornut avatar Jan 11 '21 11:01 ocornut

Thanks a lot for your answers and suggestions Omar. I will have look at them and come back to you in a few days.

pthom avatar Jan 12 '21 14:01 pthom

Hello Omar,

Many thanks for your suggestions. I added some more commits, and I think we are onto something interesting! If you are in hurry, just look at the demo just below, and read the details later.

Demo


Details

Possible use of the DEMO_MARKER outside of imgui manual:

You wrote:

One good aspect of this is that we could later aim to implement a basic dependency-free version of your solution in imgui_demo., probably less polished, with less fancy text editor.

Well, my opinion is that the DEMO_MARKER macro can also be useful inside imgui even without the manual, and even without a side editor, since the tooltip show the exact location of the corresponding code in imgui_demo.cpp: image

Only show code button if zone is hovered

As you can see, only one "Code" button is shown at a time. This is much less ugly. This button is only shown if the mouse is inside the window, and it is hovering the demo ZoneBoundings.

In order to do this, I iterated on your "markers" idea, and I added a class DemoZonesRegistry, which can be summarized as below:

class DemoZonesRegistry
{
private:
    struct ZoneBoundings
    {
        int SourceLineNumber; // Source code location
        float MinY, MaxY;     // Location of this zone inside its parent window
    };
public:
    DemoZonesRegistry();
    bool DemoZone(int line_number);  // starts a demo zone, and returns true if the button shall be shown
private:
    ImVector<ZoneBoundings> AllZonesBoundings; // Holds all demo zone boundings
}

DemoCode -> DEMO_MARKER

The macro is now in upper case. Also I reviewed all the calls to this macro in order to make them more coherent.

We now have two macros:

  • DEMO_MARKER will put the button to the right of the next widget
  • DEMO_MARKER_BLANK_LINE will put the button to the right of a blank line

Most calls (>95%) are now using DEMO_MARKER. The only cases where there is a blank line are:

  • inside menus (because there might be a check or submenu to the right of the next widget)
  • immediately after ImGui::Begin() calls

Misc

  • added FILE in the macros to potentially allow this to be used in other code
  • Removed hard-coded size constraints
  • Corrected positioning glitches
  • Improved for cases where there is no space: the code button will not appear when there is not enough space to the right

pthom avatar Jan 22 '21 23:01 pthom

(you can check it at https://pthom.github.io/imgui_manual_online/manual/imgui_manual.html )

pthom avatar Jan 22 '21 23:01 pthom

I added a param hovering_zone_onlyto the callback, so that the GUI can react to the user only hovering a given zone, like so:

typedef void (*ImGuiDemoCallback)(const char* file, int line_number, const char* demo_title, bool hovering_zone_only);

I you now look at the manual, you will see that the TOC on the right now follows the mouse position in the demo (if "Follow Demo" is checked).

pthom avatar Jan 23 '21 14:01 pthom

And then, we could suppress the code button and use the "Ctrl-Shift" modifier to display a tooltip, that would work even outside the manual.

See below:

image

(However, note that if no indication is shown in the demo itself, the user might not now whether there is an actual DEMO_MARKER tag for the widget he is looking at).


There could also be an alternative way to show them : image


I did not yet push a change that suppresses the code button, or one that uses the "{?}", as I prefer to wait for your opinion

pthom avatar Jan 24 '21 09:01 pthom

Well, my opinion is that the DEMO_MARKER macro can also be useful inside imgui even without the manual, and even without a side editor, since the tooltip show the exact location of the corresponding code in imgui_demo.cpp:

This is a really good point! Makes this even more attractive to merge in existing demo even without a text editor in demo.

  • If I try your demo, the demo marker seems to be stealing inputs while grabbing a dock node separator.
  • The visible overlap when resizing a window small is a little ugly
  • The discrepancy between the fact this is checking entire zones, not widgets, can be confusing when hovering around. So when you hover to the right-side of a tree-node, the tree-node don't react but the help marker changes...

The last paragraph of my previous feedback post suggested flipping things around and have a modal "Help" tool to avoid those issues, I should have been clearer about that. Say you have 1 "Help/Code Lookup" button in a window, when activated it would: hold on active id (which prevent hovering other stuff, to reduce noise) + and highlight the zone by e.g. dimming the rest of the screen using 8 transparent rectangle + display tooltip and maybe clicking activate the callback action if any. It's using the exact same underlying data but picking is explicit and modal and because of that the interactions can be designed differently.

ocornut avatar Jan 25 '21 14:01 ocornut

Addendum: it's not 100% ideal because it requires activation from the user, and a button needs to be placed in more location (especially menus/popups), but might provide a better user experience ihmo.

ocornut avatar Jan 25 '21 14:01 ocornut

I did some experiments; and I would like to explore a bit further with you concerning the different possibilities.

Say you have 1 "Help/Code Lookup" button in a window, when activated it would hold on active id (which prevent hovering other stuff, to reduce noise)

I'm not sure I understood your idea, since the active id would be deduced from the zone hovered by the mouse; and this id would be lost as soon as we move the mouse to the button.

May be a combination of two keyboard shortcuts (activate highlighting for the current zone / deactivate highlighting) could work, but I'm afraid it would not be user friendly. So, I guess I did not understand your idea.

  • highlight the zone by e.g. dimming the rest of the screen using 8 transparent rectangle

I tried this, and I'm afraid that the discrepancy you noted ("this is checking entire zones, not widgets and can be confusing") might get even worse.


Anyhow, I did a video since it is a lot easier to explain with live demonstrations. It is 2 minutes long, and it explores 4 different possibilities:

  • No indication whatsoever
  • Demo Label {?} with tooltip
  • Global keyboard shortcut (e.g Ctrl-Shift)
  • Highlight with or without tooltip as a global option (I could not find a way to implement your "modal idea")

Can you have a look at it ( https://traineq.org/imgui_demo_alternatives.mp4 ) and tell me what you think?

Thanks!

pthom avatar Jan 25 '21 20:01 pthom

Thanks for the detailed video and investigating those ideas further.

Have a look at Tools > Metrics/Debugger > Tools > Item Picker for the type of interaction I was thinking. It could be bound on a button called e.g. "Code Picker.." and also bound on a keyboard shortcut. Maybe that button can be on a floating window which you can enable in the demo, and then once you enable this floating window it's always visible.

The tool modal in the sense that once you enable it, it stays open until clicking or pressing space.

When the tool activated, if if make it steals the active id using SetActiveID(GetID("code picker")) unless clicked or escaped, then items in the window won't react to hovering or clicking, which seems desirable. This also helps focusing on the fact that we are targeting rectangular zones, not specific items (e.g. not a tree node). The only problem is that SetActiveID() is not a public API but we can find a way to solve this (for now ok to test with imgui_internal.h. As a super quick hack, if you use ButtonEx()withImGuiButtonFlags_PressedOnClickReleaseAnywhere` it will steal the active id but the interaction will require holding mouse button, and dragging to target zone (whereas we want the interaction to be "click once to activate tool", "click on zone"). But that's simpler to try very quickly. (Btw while writing this I noticed that "Item Picker" doesn't use that, I think it should).

For the dimming overlay, if you use GetForegroundDrawList() instead of GetWindowDrawList() you can cover the whole screen, may be nicer if the overlay is subtle (but probably not ok if code panel update while just moving mouse). Or maybe we can display all zones at all time with a subtle color (and then use the window drawlist) and display the hovered one more prominently. I think it is ok to put an emphases on zone especially if clicking item is disabled.

ocornut avatar Jan 26 '21 11:01 ocornut

Hello Omar,

A quick update on the status. As there are quite some details, I made a video that tries to summarize them :

https://traineq.org/imgui_demo_edit.mp4 (ignore the part about SetActiveID(), I have solved the issue)

Below is an abstract of the video:


Simple fixes:

  • No more button: I completely removed the code button that were appearing on the right
  • Dim full viewport: the full screen/viewport is now dimmed

Work inside imgui_demo with a code editor/viewer:

I would like to preview what a code editor might render on the demo itself; so I added a quick and dirty code viewer inside imgui_demo.cpp. Consider it as a draft that can be removed or disabled later, if you wish.

Note: as there is no way to embed resources inside the build; this code window will read the code via a simple call to fopen(__FILE__, "r"). I know, this is dirty; however if the file is not found after deployment, it will handle this gracefully.

Decide between several ways to pinpoint a demo code location

As of now, there are several ways to pinpoint the code location:

  • using the "Ctrl-Shift" hotkey
  • using the "Help/Code Lookup" picker button with SetActiveID
  • using the "Help/Code Lookup" picker button without SetActiveID + right-click to stop picking (see video in the next comment)
  • using the follow mode

The problem is that these modes are sometimes inconsistent. Your opinion will greatly interest me.

Issue with SetActiveID

I had an issue, which I solved using the following flag: ImGui::IsWindowHovered(ImGuiHoveredFlags_AllowWhenBlockedByActiveItem)

Note: when using the existing "Metrics/Item Picker" , items are still updated when hovered on my side.


Note concerning the CI / build mode "without C++ runtime": are delete operator forbidden in the CI ?

The CI was failing on my code in the mode "without C++ runtime with an error that says "Undefined symbol operator delete(void*, std::align_val_t)" See for example: https://github.com/ocornut/imgui/pull/3689/checks?check_run_id=1787291104

I "corrected" the CI by removing the destructor of DemoMarkersCodeWindow; but there is now a leak.

It seems strange, since for example ExampleAppConsole does have a destructor.

pthom avatar Jan 28 '21 22:01 pthom

Another idea concerning the "Help/Code Lookup" button: instead of grabbing the ActiveID, it could let the user navigate the trees and menus, and require the user to right click to exit the picking mode. IMHO, this is quite usable.

See a demo here: http://traineq.org/imgui_demo_edit2.mp4

pthom avatar Jan 29 '21 11:01 pthom

Thank you, will look at the update whenever I have time.

Use IM_NEW/IM_DELETE to fix your allocation problem.

(As a minor suggestion, The PR has 58 commits it may be good to squash it in 1 or a few "milestones" commits e.g. 3-4)

ocornut avatar Feb 01 '21 12:02 ocornut

Thanks for the suggestion about IM_DELETE.

I squashed all the commits into one, and added some documentation.

pthom avatar Feb 01 '21 21:02 pthom

Hello Omar,

I squashed & rebased the code onto the latest commit of the docking branch.

I know that you are extremely busy, so take your time and review this whenever you want.

There are two decisions that still need to be made, but I tried to make it easier to decide by providing two temporary flags (that can easily be removed once the decisions are made).

If you look at imgui_demo.cpp, line 179 you will see the flags: DEMOMARKER_RIGHTCLICK and DEMOMARKER_SHOWCODEWINDOW.

These flags correspond to the following choices:

  • shall the "Help/Code Lookup" mode be exited via a right click, or shall it "grab the active id" ?
  • shall we keep with the basic imgui_demo.cpp code viewer that I added?

Concerning the right click, my opinion is that it is more usable (see previous video here: http://traineq.org/imgui_demo_edit2.mp4); so I enabled it in the defaults flag config.

pthom avatar Feb 22 '21 18:02 pthom

Hello,

I did some more work on the manual inside imgui_demo.cpp.

Note: I added two new commits to this PR: one that embeds imgui_demo.cpp into the emscripten builds and one that adds a DemoMarkerTools namespace

However, I would need some additional input in order to be able to continue. In order to make the decisions easy, this message will provide interactive links to demonstrations that showcase the different possibilities.

  1. Question 1: Do we want to grab the active ID when clicking the "Help/Code lookup" button.

    In that case, tree and menus cannot be opened. The user is forced to open them before using this button. IMHO, this is not very usable.

    Interactive demo here: Left click / No Code Viewer

  2. Question 2: Do we prefer to not grab the active ID when clicking the "Help/Code lookup" button and require the user to right-click

    IMHO, this is more usable, since the user can then navigate to wherever he wants (even inside menus), and see the corresponding code location.

    Interactive demo here: Right click / No code viewer

  3. Question 3: Do we want to include a low tech code viewer?

    imgui_demo.cpp can provide a basic CodeViewer that would read the code via a simple call to fopen(__FILE__, "r")

    This way, it is possible to display the corresponding code when using the "Help/Code Lookup" button. Of course, this will work only if the imgui_demo.cpp is still available at the same location as when compiling.

    Interactive demo here: Right Click / Code viewer

    But, as the demo shows, this could also work with emscripten: this commit enable to embed imgui_demo.cpp code into emscripten, like this:

    source ./emsdk_env.sh
    USE_FILE_SYSTEM=1 make -C examples/example_emscripten_opengl3
    make -C examples/example_emscripten_opengl3 serve
  1. Question 4: Do we want to include more functionality in this code viewer? Search and link to github?

    This can provide a more pleasant experience.

    Interactive demo here: Right click / Code Viewer with Search & github button

Below is a screenshot that shows the code viewer + search + github button: image

However, things then get a little more complex, since this requires to add some more code to imgui_demo.cpp (mosty in order to parse the tags and to open the hyperlinks).

~~I did not include those modifications (i.e related to Question 4) into the original DemoCode branch (which is the subject of this PR), but I added them into a new branch (named DemoCodeSearch).~~
This adds mainly two commits:

  • Parsing (this commit parses imgui_demo.cpp's DEMO_MARKER tags, and adds a search filter)
  • BrowseToUrl (this commit adds a BrowseToUrl() function and thus requires some platform dependent calls)

pthom avatar Feb 24 '21 18:02 pthom

Hello,

Just a quick update: I updated the code to solve the conflicts originating from the latest commits to the docking branch.

Also, in order to make the process easier for you, I put everything in this branch, and I took the decisions that, IMHO, were the most user friendly, i.e "use a right click", "include a code viewer", and provide a "Open Github" button.

So, we now have four commits, listed below in chronological order:

  • 872c7abf : imgui_demo.cpp: Add DEMO_MARKERS and DemoCodeWindow
  • 823765a0 : embed imgui_demo.cpp code into emscripten builds
  • d373f3f5 : imgui_demo.cpp/Code viewer: Parse DemoMarkers tags / add demo search
  • 8ccdea87 : imgui_demo.cpp: add BrowseToUrl (called from Code viewer)

Live demo here

pthom avatar Mar 21 '21 15:03 pthom

Thanks for the careful and detailed update, they are great. Sorry I haven't been able to explore this further. Will eventually do!

Small feedbacks:

  • Something I notice is that in demo, hovering a child window leads to the highlight being disabled, whereas I feel that in the absence of a nested region, hovering a child window should behave like hovering the parent. In IsMouseHoveringZoneBoundings() would add the _ChildWindows flag to the IsWindowHovered() call perhaps?
  • I also guess using absolute coordinate (GetCursorScreenPos().y) would simplify some code (can remove the GetWindowPos() and GetScrollY() calls in IsMouseHoveringZoneBoundings).
  • Might be useful, it's a little obscure but technically in the public api: ImGuiTextFilter::ImGuiTextRange has a split function which doesn't reallocate strings. If you duplicate the source code once then you can write zero markers inside that buffer, and you have a single allocation, less memory management and probably less code?

ocornut avatar Mar 22 '21 12:03 ocornut

Ok, I took into account your suggestions.

StartsWith(): use strncmp

Done

I also guess using absolute coordinate (GetCursorScreenPos().y) would simplify some code (can remove the GetWindowPos() and GetScrollY() calls in IsMouseHoveringZoneBoundings).

Done

Something I notice is that in demo, hovering a child window leads to the highlight being disabled, whereas I feel that in the absence of a nested region, hovering a child window should behave like hovering the parent. In IsMouseHoveringZoneBoundings() would add the _ChildWindows flag to the IsWindowHovered() call perhaps?

Done. I tried the flag ImGuiHoveredFlags_ChildWindows, but it caused issues inside menus (the region below the menus would trigger when the menus are opened). The flag ImGuiHoveredFlags_RootAndChildWindows gave better results. However, in order to not trigger whenever we are inside a submenu, I had to also add a test for the x coordinate.

See IsMouseHoveringZoneBoundings()'s new version here: https://github.com/pthom/imgui/blob/DemoCode/imgui_demo.cpp#L656

Might be useful, it's a little obscure but technically in the public api: ImGuiTextFilter::ImGuiTextRange has a split function which doesn't reallocate strings. If you duplicate the source code once then you can write zero markers inside that buffer, and you have a single allocation, less memory management and probably less code?

Done. I am now using ImGuiTextRange::split, which saves a few lines of code. ~~However it does not save allocations, since the pointers inside ImGuiTextRange are const char * (and thus I cannot write zero markers, except if I const_cast, which I did not).~~ Also, there are no more allocations: see https://github.com/pthom/imgui/blob/DemoCode/imgui_demo.cpp#L311


pthom avatar Mar 22 '21 23:03 pthom

Now that we have a code viewer, please give me a few days to explore some additional ideas about picking and navigation. I think it can be improved.

pthom avatar Mar 23 '21 14:03 pthom

Ok, after a few days; I think we finally have something that is truly user friendly:

Live demo here

Basically, we now have a "Code lookup" checkbox. Whenever active, the code viewer will follow the mouse location. This mode can be exited via the "Esc" key, or by unchecking the checkbox. This is much more user friendly, especially since the demo zones are now complete (including children windows).

Note: in order to make the checkbox always visible, I had to add ShowDemoWindow into a child. It works nicely. image

IMHO, we have reached a status where the usability is quite good, for touch and mouse devices. Also, we now have a user interface that behaves identically in the standalone manual, and the full manual.

pthom avatar Mar 29 '21 11:03 pthom

Just a quick update: I rebased the commits on the latest docking commit (june, 4th). The CI is OK on my fork, but is awaiting approval here.

pthom avatar Jun 04 '21 19:06 pthom

Hello Omar,

I rebased the commit onto the latest version of the docking branch. Also, I rewrote the history in order to keep only two commits (hopefully, this might bring us closer to a possible merge).

  • 93969d67 ("[Doc] imgui_demo.cpp: Add DEMO_MARKERS and DemoCodeWindow") This contains all the modifications inside imgui_demo.cpp. Also, I rewrote the commit message so that it describes (hopefully) accurately what was done.

  • 08a2f3af ("[Doc] Emscripten makefiles: Embed imgui_demo.cpp code into the build") This commit only modifies the emscripten makefiles, so that the emscripten builds embeds the source code for imgui_demo.cpp

You will need to relaunch the CI workflow (it works on my side).

Thanks.

pthom avatar Oct 19 '21 14:10 pthom

Hello Omar,

Sorry to bug you again two days in a row.

I did two changes:

  • This PR is now targeting the master branch: I suspect this might increase the target audience for the PR. I was initially afraid that this backport might be difficult, but it was not. When merging to the docking branch, it might create 2 simple conflicts, for which I provide a resolution below.

    PS: I hope you will not be disappointed that I changed the target branch without consulting you (I can totally go back to targeting the docking branch if you so desire)

  • I corrected 2 lines of code in imgui_demo which were targeting C++11. The code now compiles without any warning using the C++98 makefiles from the examples folder.

Potential conflicts and resolutions when merging to the docking branch:

Conflict 1: Line 896:

<<<<<<< HEAD
    if (show_app_main_menu_bar)       ShowExampleAppMainMenuBar();
    if (show_app_dockspace)           ShowExampleAppDockSpace(&show_app_dockspace);     // Process the Docking app first, as explicit DockSpace() nodes needs to be submitted early (read comments near the DockSpace function)
=======
    if (show_app_main_menu_bar)       ShowExampleAppMainMenuBar(NULL);
>>>>>>> 8c6744fe... [Doc] imgui_demo.cpp: Add DEMO_MARKERS and DemoCodeWindow

can be solved like this:

    if (show_app_main_menu_bar)       ShowExampleAppMainMenuBar(NULL);
    if (show_app_dockspace)           ShowExampleAppDockSpace(&show_app_dockspace);     // Process the Docking app first, as explicit DockSpace() nodes needs to be submitted early (read comments near the DockSpace function)
    if (show_app_documents)           ShowExampleAppDocuments(&show_app_documents);     // Process the Document app next, as it may also use a DockSpace()

Conflict 2: line 7889

<<<<<<< HEAD
        if (ImGui::IsWindowDocked())
            ImGui::Text("Warning: Sizing Constraints won't work if the window is docked!");
=======
        DEMO_MARKER("Examples/Constrained Resizing window");
>>>>>>> 8c6744fe... [Doc] imgui_demo.cpp: Add DEMO_MARKERS and DemoCodeWindow

can be solved like this:

        DEMO_MARKER("Examples/Constrained Resizing window");
        if (ImGui::IsWindowDocked())
            ImGui::Text("Warning: Sizing Constraints won't work if the window is docked!");

pthom avatar Oct 20 '21 13:10 pthom

It's not clear why you added a bool* parameter to ShowExampleAppMainMenuBar(), it seems unused and that's the thing causing a conflict ?

Thanks for your patience and apologies for not getting back to this right now.

Now that there is a single macro (instead of the two originals) it is probable I could do a first step and merge the markers, that would solve the conflicts across branch and narrow down the PR to code. I'll see what I can do. (Don't worry about changing anything, if i end up just merging the markers it'll be easy for me to do and when you rebase it won't conflict or very little). Can easily design a way to make it a runtime function plug (part of IO) even.

ocornut avatar Oct 20 '21 14:10 ocornut

It's not clear why you added a bool* parameter to ShowExampleAppMainMenuBar(), it seems unused and that's the thing causing a conflict ?

You are right, it's not clear, and it is unrelated to this PR.

When developping the imgui_manual app, I also found a way to automatically extract all the example apps code from imgui_demo.cpp; and to make standalone cpp/executables from them. Those generated cpp files can be seen here: https://github.com/pthom/imgui_manual/tree/master/playground/ExampleApps In that context, I had to make the signature of all function identical and ShowExampleAppMainMenuBarwas the only with a different signature.

Do you want me to remove this modification from the PR?

It is probable I could do a first step and merge the markers

Seems like a good idea!

Do you want me to re-write the history in order to add the markers in an independent commit, or do you prefer to do it yourself?

pthom avatar Oct 20 '21 14:10 pthom

Do you want me to remove this modification from the PR?

Keep your PR the way it works simpler/better for you now, when I merge bits I may revert that specific section myself.

Do you want me to re-write the history in order to add the markers in an independent commit, or do you prefer to do it yourself?

I can do it myself it's easy (selective merging), when you rebase aftewards it won't conflict.

ocornut avatar Oct 20 '21 14:10 ocornut

I removed the modification around ShowExampleAppMainMenuBarand I'll let you do a selective merge when you have time :-)

pthom avatar Oct 20 '21 15:10 pthom