Selected Tree Node/Left functionality

[Cthutu]

I've finally converted my editor away from wxWidgets to use ImGui but have fallen at the first hurdle. I've scoured the demos and source code and can't see a way to do a basic tree. By basic tree I mean:

1. Tree leafs do not have open arrows
2. Items can have a selected state so they're rendered differently
3. Can react to tree nodes being selected or unselected

My only solution so far is to copy and paste the TreeNode, TreeNodeV and CollapsingHeader functions and alter them to provide the functionality I need. I don't mind sharing this functionality when I'm done but I am little stumped about how I would handle reacting to nodes being selected or unselected (with Shift and Ctrl keys supported) in the ImGui way (i.e. immediate).

Any advice on the matter?

[ocornut]

1. You can use Bullet, BulletText()

2/3
This is a common request, we perhaps need to introduce a new API but I have spent much time thinking about it in details.

The way TreeNode() works is that it stores the open/closed state in imgui storage because it makes sense in the majority of cases. Whereas a "selected" state is most likely something that the users wants to manage themselves in the majority of cases.

Right now the workaround is to do:

bool opened = TreeNode(id, "##nolabel);
SameLine()
Selectable();

Which works but the mouse-detection region is incorrect. Basically we want to combine both into one. Note that both TreeNode and Selectable independently need more options to select how to behave in term of hit detection, in particularly for TreeNode we'd want an extra flag. So perhaps we need to introduce TreeNodeEx to allow for that flag and allow also for selection state. There we could also pass in extra flags for arrow placement.

bool?? TreeNodeEx(const char* id, bool* selected, TreeNodeFlags flags, const char* fmt)

Unfortunately as you pointed we can't return both "opened" and "clicked" (note that Selectable have both bool* and bool versions that are both useful, the later rely on the user checking the return value). I haven't fully thought about what's the best way to handle that yet. We could return two bools in a flag set but that's ugly and very unusual in our API.

I am little stumped about how I would handle reacting to nodes being selected or unselected (with Shift and Ctrl keys supported) in the ImGui way (i.e. immediate).

As it would make more sense to let the user store that state in whichever way they see fit, the API can only give information about state change (e.g = node clicked) and Ctrl-state multiple selection is best left to the user to handle.

About Shift, we don't have keyboard navigation controls (#323) at all yet anyway, and that alone would be a big task.

Not much help, but also linking:
#190
#431
#324

[Cthutu]

OK, let me show you what I've done:

    bool ImGui::TreeNodeV(const void* ptr_id, bool isSelected, bool isLeaf, const char* fmt, va_list args)
    {
        bool opened = false;

        if (isSelected)
        {
            ImVec2 pos = ImGui::GetCursorScreenPos();
            ImU32 col = ImColor(ImGui::GetStyle().Colors[ImGuiCol_HeaderHovered]);
            ImGui::RenderFrame(
                pos,
                ImVec2(pos.x + ImGui::GetContentRegionMax().x, pos.y + ImGui::GetTextLineHeight()),
                col,
                false);
        }

        ImGuiWindow* window = GetCurrentWindow();
        if (window->SkipItems)
            return false;

        ImGuiState& g = *GImGui;
        ImFormatStringV(g.TempBuffer, IM_ARRAYSIZE(g.TempBuffer), fmt, args);

        if (!ptr_id)
            ptr_id = fmt;

        if (!isLeaf)
        {
            ImGui::PushID(ptr_id);
            opened = ImGui::CollapsingHeader(g.TempBuffer, "", false);
            ImGui::PopID();
        }
        else
        {
            ImGui::Text(g.TempBuffer);
        }

        bool result = !isLeaf && opened;

        if (result)
        {
            ImGui::TreePush(ptr_id);
        }

        return result;
    }

    bool ImGui::TreeNode(const void* ptr_id, bool isSelected, bool isLeaf, const char* fmt, ...)
    {
        va_list args;
        va_start(args, fmt);
        bool s = TreeNodeV(ptr_id, isSelected, isLeaf, fmt, args);
        va_end(args);
        return s;
    }

I am totally agree about the user managing most of the state but that is not the part I am stumped about. You see the TreeNode() functions already return some information, whether they are opened or not. If I need to return more information, like a node has been clicked, I can't do that via a return value like Button does. So I am wondering how else I would do it. The only thing I can think of is a std::function parameter so I can do a lambda. But I am wondering if there is a more ImGui way to handle this.

I don't require the tree node system to store selected state and I can handle the Ctrl/Shift state outside too. Just want to know that a node has been clicked on.

The final thing I have to fix is the Text call in my function to highlight when the mouse hovers over it.

[Cthutu]

Your links provide some invaluable information - thanks!

[ocornut]

Either add an extra pointer parameter, or return flags. Perhaps returning flags would be acceptable for a TreeNodeEx() function, so it doesn't return a bool anymore but a set of flags.

Quick notes and ideas:

• Your RenderFrame aabb calculation is incorrect if the text covers multiple lines. Unfortunately or fortunately that behavior would better be moved down to the function that display the rectangle for howering. Some refactor probably needs to be done here, currently the shared function is CollapsingHeader which is also an end-user exposed function and that's rather weird. Will have to think what is the best way to clean that up. It might be that TreeNodeEx() may be the ultimate handle-it-all function and both CollapsingHeader and TreeNode calls it.
• Your leaf path doesn't handle clicking. I see the value in allowing the support displaying/handling leaf in the same function, however it is likely that the user may want more complex/custom leaves so it would be better to allow let the user handle that. And for that purpose we may need to provide helper to use the selection system (user can pass an explicit or if possible implicit aabb corresponding to the leaf).
• Please try following the coding style of ImGui if you aim for the code to be integrated into the project and usable by others. Some refactoring required means that I would probably do the finishing here anyway but worth keeping that in mind :)

[Cthutu]

Yeah you're right about the leaf path doesn't handle clicking. That was on my list of things to figure out. For some reason I forgot about it when inquiring above.

Are you saying that CollapsingHeader can be used for a node without a open triangle, or would I have to adjust that function? I was going to copy & paste the code for that function to solve the selectable leaf problem and butcher it to do what I needed it to do.

I wasn't planning on integrating it back into the project since I don't know enough about the internals of ImGui yet to feel comfortable about doing that. But if that was the case, a helper function to do handle leaves would be a better idea. Perhaps a BeginSelectable()...EndSelectable(). E.g.

if (ImGui::TreeNodeEx("non-leaf", ImGuiTreeNodeFlag_Selected, "Non-leaf node"))
{
    ImGui::Indent();
    ImGui::BeginSelectable();
    ImGui::Text("Leaf Node");
    if (ImGui::EndSelectable())
    {
        // Leaf node clicked
    }
    ImGui::Unindent();
    ImGui::TreePop();
}

Obviously this snippet wouldn't handle the colouring of the leaf node that was selected, but would highlight when the mouse hovered over it.

[ocornut]

I would suggest moving the bulk of CollapsingHeader into a new TreeNodeEx() and figuring out a way to share that code (adding a new set of flags). Copy and pasting make sense for working locally without worrying about further integration but ultimately we want to avoid it.

User could also just use Selectable() or their own stuff if we want selection for lead, perhaps that isn't needed at all.

It's ok if you don't do it in a way that is 100% integrable, posting your ideas/work here is helpful as I can eventually take the base idea and finish it. Sorry I am merely thinking out loud there!

[Cthutu]

I'll write the code so that it contains ImGui-only code and get it to work for my needs, then I will post it and perhaps you can suggest changes to it so that it will be a better fit for integration, then I will do the work and start a pull request. Could you suggest the signature for the TreeNodeEx() function as you see it?

[ocornut]

int TreeNodeEx(const char* id, bool* selected, TreeNodeFlags flags, const char* fmt) ;; return TreeNodeResult flags

About thought about it enough so I may be wrong there.

[Cthutu]

And you would expect a TreeNodeV to call in to TreeNodeExV?

[ocornut]

Yes!

[Cthutu]

I'm curious, what is the rationale with 'selected' being a input/output parameter? Currently, I've managed to do it with selected being an input parameter with the selection state being determined by the user. If that was the case, we could use ImGuiTreeNodeFlags_Selected to determine that.

[Cthutu]

I guess that would solve the "did I click it" problem as that wouldn't be needed, since the only thing you'd want to know is whether it was selected or not.

[ocornut]

what is the rationale with 'selected' being a input/output parameter?

You are right - it doesn't have to. Sometimes it is convenient that it does the flipping, this is why for example MenuItem has both versions. However in this case we are already dealing with a more-complex-than-average pattern requiring code from the user anyway, and we are already in the middle of too-many-API-explosion so it is probably better to just stick to bool selected version, either has a bool either as you say as a flag.

I would say go for flag, my past experience has been that bool parameters have mostly been a bad idea. The only reason I would go for bool selected over a flag would be to make it a little more consistent with other API but all in all weighting both I think flag may be better.

I guess that would solve the "did I click it" problem as that wouldn't be needed, since the only thing you'd want to know is whether it was selected or not.

Not sure to understand this sentence, sorry.

[Cthutu]

Not sure to understand this sentence, sorry.

Since the treenodes maintain whether they're open or not, the only other information a treenode can give is whether they're selected or not. The I/O parameter would give that information, so TreeNodeEx is free to still return a bool at whether the tree is open or not.

[ocornut]

It's a little tricky because if you want the user to react to selection (to apply their selection data which may not be a native bool or handle CTRL selection), then user having to check the output-bool means they probably need to also store the previous value and compare themselves, which would be a little cumbersome. So returning a flag allows to simplify that for the user.

[ocornut]

@Cthutu Been working on this (and various other tree related features today). After half a dozen of iterations I think I've got it all solved but I would be interested in seeing what you've done, out of curiosity and to see if there is an idea I may have overlooked.

[ocornut]

Here's the demo that now does this:

ShowHelpMarker("Click to select, CTRL+Click to toggle, double-click to open");
static int selection_mask = 0x02;   // Dumb representation of what may be user-side selection state. You may carry selection state inside or outside your objects in whatever format you see fit.

int node_clicked = -1;
for (int i = 0; i < 5; i++)
{
    ImGuiTreeNodeFlags node_flags = ((selection_mask & (1 << i)) ? ImGuiTreeNodeFlags_Selected : 0) | ImGuiTreeNodeFlags_OpenOnDoubleClick;
    bool opened = ImGui::TreeNodeEx((void*)(intptr_t)i, node_flags, "Selectable Child %d", i);
    if (ImGui::IsItemClicked()) 
        node_clicked = i;
    if (opened)
    {
        ImGui::Text("blah blah");
        ImGui::TreePop();
    }
}
if (node_clicked != -1)
{
    // Update selection state. Process outside of tree loop to avoid visual inconsistencies during the clicking-frame.
    if (ImGui::GetIO().KeyCtrl)
        selection_mask ^= (1 << node_clicked);  // CTRL+click to toggle
    else
        selection_mask = (1 << node_clicked);   // Click to single-select
}

You can:

• Pass a flag to display a node as selected
• Click is done via polling IsItemClicked() to be consistent with existing functions. I initially went for a set of return flags as discussed above but it became really error prone and confusing so I moved away from it.

Looking at your code above I am not sure there is a need for a concept of Leaf, but we now have much more leeway for adding options via ImGuiTreeNodeFlags.

Here are some unimplemented flags ideas:

ImGuiTreeNodeFlags_OpenOnArrowOnly    = 1 << 7,   // FIXME: TODO: Open with single click on arrow?
ImGuiTreeNodeFlags_UnindentArrow      = 1 << 8,   // FIXME: TODO: Unindent tree so that Label is aligned to current X position
ImGuITreeNodeFlags_SpanAllAvailWidth  = 1 << 9,   // FIXME: TODO: Extend hit box horizontally even if not framed
ImGuiTreeNodeFlags_NoScrollOnOpen     = 1 << 10,  // FIXME: TODO: Automatically scroll on TreePop() if node got just opened and contents is not visible

[ocornut]

Looking at your code above I am not sure there is a need for a concept of Leaf,

Never mind, I got confused.
I will implement this as @kylawl and you proposed:

Basically what I'm proposing is a flag on collapsible header prevents it from expanding and hides the arrow (something to the effect of "ImGui_DisableCollapse". Or a bullet text that can have an empty bullet I guess, but It seems like it should really just be on the collapsible.

[ocornut]

Added ImGuiTreeNodeFlags_OpenOnArrow flag.
Combining ImGuiTreeNodeFlags_OpenOnArrow + ImGuiTreeNodeFlags_OpenOnDoubleClick allows to have single-click on arrow OR double-click on main content to open, which matches e.g. common Windows tree behavior.

One problem in the color scheme here is that if you look at this out of context, there's no way to distinguish the collapsing header from the selected tree node. So may need to re-arrange the color enum.

Remaining

• Sort out leaf / indentation confusion
• Tweak color scheme to distinguish headers from selected tree node.

[ocornut]

From @Cthutu

Hi,

I've added a new flag to ImGui called ImGuiTreeNodeFlags_NoBullet that removes the bullet. I think it's an important feature since the bullet is not necessarily functional like the arrow. It disables the RenderBullet() call in TreeNodeBehavior(). I had my own TreeNodeEx function to implement a folders/file view (think resource view in Unity) and I updated to see your new version. These bullets suddenly appeared and it looked ugly to me.

I tried to create a pull request but it failed (permissions problem).

What do you think of the idea?

[pdoane]

Also agree with Cthutu that we don't want a bullet to display on the leaf nodes.

[ocornut]

Understood, will likely follow that. At the moment I yet have to fully understand what may constitute a leaf node aside from the trivial one-liner case, how it behave with layout of multiple widgets and how we can work out with indent. This is why I'm not worried about this flag right now (and AlwaysOpen may be removed or removed once I figure those things out).

[ocornut]

Removed the bullet from the _Leaf flag.

FYI the only purpose of ImGuiTreeNodeFlags_Leaf are:

• convenience if you want Selectable nodes, but Selectable() could be used as well there.
• convenience to do a PushTree()/PopTree()

But simple text display it'll work but not the more efficient code-path.

So you can also use Bullet() BulletText() if you don't mind the bullet.
Or you can do SetCursorPosX(GetCursorPox() + GetTreeNodeToLabelSpacing())

May be worth coming up with a helper to do this directly.
Not sure if it should be a function to just advance the cursor
void TreeAdvanceToLabelPos(); // advance cursor x position by GetTreeNodeToLabelSpacing()
?

And one that does that + display text.
void TreeLeafText(const char* fmt,...); // = TreeAdvanceToLabelPos(), Text()

[ocornut]

Added TreeAdvanceToLabelPos(). I think everything is this thread is complete now?