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

g.TreeNode() does not allow a QTreeView-alike widget (Table + TreeNode) #134

Closed
folays opened this issue Jan 28, 2021 · 4 comments
Closed

Comments

@folays
Copy link

folays commented Jan 28, 2021

Example and lectures:

  1. New Tables API (alpha available for testing) ocornut/imgui#2957 (comment) for an example of a classic "filesystem tree widget" which combine a BeginTable + TreeNode
  2. Example usage in imgui (C++) of the screenshot above (Table + TreeNode) is here : https://github.com/ocornut/imgui/blob/v1.80/imgui_demo.cpp#L4427
  3. Example usage in giu of a TreeNode" (without Table`) is here : https://github.com/AllenDang/giu/blob/v0.5.2/examples/widgets/widgets.go#L132
  4. Implementation in giu of *TreeNodeWidget.Build() is here : https://github.com/AllenDang/giu/blob/v0.5.2/Widgets.go#L1861

Problem:

giu does not currently permit to record the same widget given in the screenshoot in (1).

How you are supposed to draw Table+TreeNode:
Please suppose your want to build a Table (with 2+ columns) and you want to use a TreeNode in the 1st column.
The C++ imgui TreeNode() function (or imgui-go's imgui.TreeNodeV() returns an open flag.

When C++/imgui-go .TreeNode*() functions returns the open "status", you will be able to act upon it, but you are supposed to act on it ONLY LATER.

You're supposed to immediately continue to "draw" the remaining columns FIRST in the current row.

Then, once you're at the end of the current row, you would either:

  • open == false : nothing special, you should call C++/imgui-go's TableNextRow() and then draw yours column
  • open == true : you could draw one or more sub-line(s), which you would probably want INDENTED, and once you (possibly recursively) draw those line(s), you would (if indented) have to call C++/imgui-go's imgui.TreePop()

BUT : giu is sort-of in the way of acting upon the open status.

Why:

giu proposes a Layout() function which is called immediately when giu's (*TreeNodeWidget).Build() is called.
giu's (*TreeNodeWidget)'s Layout seems to be think through for an immediate usage of "drawing the content of the TreeNode.

Except that when used in a Table, a TreeNodeWidget should be perceived as having no context beside the textual item it represents, especially given that you are supposed to draw the renaming columns FIRST.

This lead to complicated setup where the code below would work incorrectly:

// in the context of a GIU's g.Table("MyTable").Rows(...)
g.Row(
    g.TreeNode("Node1-Col1").
        Layout(
            g.Row( // NOT even needed to have the problem
                g.TreeNode("Node2-Col1").Whatevers(), // NOT even needed to have the problem
                g.Label("Node2-Col2"),
                g.Label("Node2-Col3"),
            )
        ),
    g.Label("Node1-Col2),
    g.Label("Node1-Col3"),
)

The intention of the code above should be clear, to issue a table looking like that:

+Node1-Col1        Node1-Col2        Node1-Col3
 +Node2-Col1       Node2-Col2        Node2-Col3

But it won't do that if you "open" the Node1.
Due to the implementations of giu's (*TreeNodeWidget).Build(), if the Node1 is opened, giu's (*TreeNodeWidget).Build() will immediately draw ("call Build()s) on the []Widgets contained in struct *TreeNodeWidget's member layout Layout.

Thus, the Node1's and Node2's columns will be "interleaved" (or more correctly, NOT where they are expected to be).

In this situation, you could have alleviated the problem by putting nothing in giu's (*TreeNodeWidget).Layout() function, and draw (in all cases, whether open being true or false) the remaining columns of Node1 row (you would do that in the next remaining g.Row() parameters just after/outside the g.TreeNode() call.

Then, BEFORE drawing a possible Node3 (level 0) row, you sort-of COULD draw Node2 (level 1) row.
But you cannot really do it, you would encounter two more problems (1st one alleviatable, 2nd one not at all):

1] open flag is lost, since it is not exposed outside of giu's g.TreeNode() call, nor (*TreeNodeWidget).Build(). You COULD alleviate this problem by:

1a] in the g.Row() immediately before g.TreeNode("Node1-Col1"), you could register a g.Custom(func() { myOpen = false } ) which would allow you to always "reset" your var myOpen bool variable.

2a] then in the (*TreeNodeWidget).Layout() widgets, you could have inserted a g.Custom(func() { myOpen = true }), which would allow you to "leak" the open flag outside of (*TreeNodeWidget).Build()

2] Even if you successfully leaked the open flag outside above, (*TreeNodeWidget).Build() would have already returned, and thus already called (on your behalf) imgui.TreePop(), and thus all your hope of indentation of the sub-TreeNodes are lost.

In all cases, even if there is a trick, I supposed it would be convoluted.

Others thoughts:

Your giu repo is awesome :) Please don't be upset for my remarks above.
I think the problem is more due to imgui's TreeNode having at the same time:

  • an "internal" state (the current indentation level)
  • internal state which, in a context of a parent Table should NOT be not used immediately, but acted upon it only later (when the current row is completed, OUTSIDE of the TreeNode "content", when the TableNextRow() will be considered)

This behavior of imgui seems to change a little from the routine behaviours of other imgui widgets.
Even TableNextRow() have some internal state, but all of thoses are supposed to only be used INSIDE the row being draw.

In ìmgui and imgui-go, users are expected to Push on the context of what they want to draw manually, and themselves Pop when they completed their "content draw" (e.g.: ImGui::EndTable())

giu API seems to be build around an idea of .Layout() for content, and "automatic" Pop's being done on behalf of the user, all around a general idea of giu being 10x easier to use.
This giu paragdim seems to works well with 99% of imgui widgets behavior.

This reflect in giu which currently does not support having giu's TreeNode doing a "Push" (via (*TreeNodeWidget).Build() calling imgui.TreeNodeV()) without automatic "Pop" ((*TreeNodeWidget).Build() calling imgui.TreePop() automatically).

That, and the (*TreeNodeWidget).Layout() model which does not really fit the usage of TreeNodes inside a Table.
(because the "sub-row" content must be in an "indented" (yet sibling) "next-row", and not "inside" the TreeNode).

@folays
Copy link
Author

folays commented Jan 28, 2021

Ugly workaround:

I have been able to illegally hack around this with max uglyness code:

g.TreeNode("Node1-Col1").
    Event(func() {
        imgui.TableNextColumn()
        g.Label().Build("Node1-Col2")
        imgui.TableNextColumn()
        g.Label().Build("Node1-Col3")
    }).
    Layout(
        g.Row(
            g.Label("Node2-Col1"),
            g.Label("Node2-Col2"),
            g.Label("Node2-Col3"),
        )
    )
)

I abuse the the fact that:

1] (*TreeNodeWidget).Event() is called inside the context of the imgui.TreeNodeV() and especially before (a) (*TreeNodeWidget).layout.Build() and also before (b) imgui.TreePop()

It helps me "forcing" giu to draw the "remaining columns of the line" by putting those draws in .Event()

2] if open-ed, giu will calls (*TreeNodeWidget).layout.Build() respecting to the sub-row ONLY after the current row is already draw.

Thus the first g.Row() contained in the (*TreeNodeWidget).layout won't be called prematurely.
And this sub-row will still be called in the context of the TreeNode "push" before the corresponding "pop" (and thus be correctly indented)

@folays
Copy link
Author

folays commented Jan 28, 2021

Fix Proposal:

Maybe the workaround above would helps you to figure a way to fix this behavior.

Maybe explicitly propose two (* TreeNodeWidget)'s "Layout()" functions:

  1. a new LayoutBeforeSubNode() function, with the same signature than .Layout(), which would register the widgets ...Widget parameter in a layoutBeforeSubNode struct member. Those would be iterated-over and .Build() just below t.eventHandler() in (*TreeNodeWidget).Build(), and above all would be execute IN ALL CASES (whether open being false or true).

  2. but also keep Layout the same that now ("Layouting" the "content" of the TreeNode)

@AllenDang
Copy link
Owner

@folays Thanks very much to point out this edge case. I will check it later because I'm out of office right now. I also noticed that newest Table API's demo provides a table+treenode example, I'll read your proposal and provide a solution later.

@AllenDang
Copy link
Owner

@folays TreeTableWidget is ready to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants