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

Columns API work #125

Closed
ocornut opened this issue Feb 9, 2015 · 59 comments
Closed

Columns API work #125

ocornut opened this issue Feb 9, 2015 · 59 comments

Comments

@ocornut
Copy link
Owner

ocornut commented Feb 9, 2015

This thread to discuss the existing Columns() system. They are still a bit confusing at times and not as well supported as other part of the API. Some discussions there:

#124
#85

@ocornut
Copy link
Owner Author

ocornut commented Feb 9, 2015

OK, so I see a few problems

  • In general we need a better definition of columns (sizing policy). Can maybe introduce a ColumnsHeader api that will handle both the nicer display of a header AND passing on sizing policy. Optionally those can also act as button for sorting, passing the sorting information to the user.

  • The usage of Separator() within column needs to be tweaked / clarified. Separator currently span all columns, however they only advance the cursor vertically on the column calling them so it is pretty much inconsistent/buggy (workaround is to call Separator on all columns for now).

  • On Dale's last issue from thread Optimization question: Is there a way to compute only the "visible" portion of data for submission to ImGui? #124, due to manually clipping the contents of entries that are laid in columns. The problem with the top line bits disappearing is when we do fast forward with SetCursorPosY() we are already within the first column so we are making that cell very big. As we call NextColumn() the cursor resets itself to the "top". One workaround is to get out of columns first.

            ImGui::Columns(1);
            ImGui::CalcListClipping(items_count, items_height, &display_start, &display_end);
            ImGui::SetCursorPosY(ImGui::GetCursorPosY() + display_start * items_height);
            ImGui::Columns(4);
    

Note that this particular case is part of an optimisation to skip entries. A normal small-sized columns display wouldn't have to worry about those calls. It's not terribly complicated but all this smell a bit fishy and confusing (it can be hard to "get" why xx or xx happening) which is a sign that an improvement is probably needed eventually. I'll have to think about it for a bit.

  • Also consider the feature of user dragging and re-ordering columns. Shouldn't be too hard to get to work, but will also exacerbate those issues of "are we working on the first column or in the whole set ?", enforcing a clearer approach.

@unpacklo
Copy link

unpacklo commented Feb 9, 2015

What do you mean by "sizing policy"? Are you referring to setting up the column widths so that the window width isn't split evenly amongst the columns?

screenshot-imgui opengl2 example
vs
screenshot-imgui opengl2 example-1

That kind of a feature would be really nice to have. Most of the time, I do have some sort of idea of how wide the contents of each column should be, so just having a default to initialize to would suffice for me. If the user changes the widths, would be nice to remember their settings maybe? An issue here though might be the columns could be set so that the total width exceeds the window width. Not sure how you handle this currently. Looks like you just end up trying to make the window bigger?

With regard to the column header, sorting would be awesome. I had already been thinking about coming up with my own implementation using the current operations exposed, but it would have been pretty laborious to make happen. Basically, I would have had an array of indices which holds the display order of the column contents. Something like this https://gist.github.com/Roflraging/64c18e50099488837bf5.

screenshot-imgui opengl2 example-2
screenshot-imgui opengl2 example-3
screenshot-imgui opengl2 example-4
screenshot-imgui opengl2 example-5

Except, of course, the buttons would just be the top of those columns instead of being actual separate buttons. In any case, this would be really convenient to have since I need to do a lot of manual state maintenance right now to achieve this.

Something I'll need to spend more time thinking about is where the display order should live. This might be something that could live on ImGui side and it wouldn't feel too "retained mode" and be a plus for convenience.

@ocornut
Copy link
Owner Author

ocornut commented Feb 9, 2015

Sizing policy: I haven't thought of the details but probably specifying a mixture of "size in pixels", "% of remaining size" (after size in pixels are consumed) and "fill remaining space".

Columns header:
The sorting itself would still be your responsibility.
Your can currently move the buttons to be in place of the column header, and you'll be closer to final functionality (except it wouldn't look exactly as neat as how it should eventually look). Just replace the "Index" text by a button called "Index" that does what you already do. You can add a "v" or "^" characters to the end of the button label to denote the current active sorting, thought that would be a little cheap looking..*

I would personally compact your sample code quite a bit (I am a bit obsessed with code compaction) using ternary, Etc.

Seeing those big list we should also add a mechanism for alternating a subtle background color to increase readability. So much to do!

(*) we'd probably want to feature stock icons later on.

@emoon
Copy link
Contributor

emoon commented Feb 10, 2015

Would it be possible to also have Tree Items in this?

@ocornut
Copy link
Owner Author

ocornut commented Feb 10, 2015

That should work already.
tree in columns

@emoon
Copy link
Contributor

emoon commented Feb 10, 2015

Ah, cool :) Thanks!

@unpacklo
Copy link

I just saw #129 and having a selection feature like that except for columns would be really convenient.

Right now we have some pretty janky stuff to kind of get that functionality by making column contents buttons and we detect when a button is pressed to do something with that row of data.

@ocornut
Copy link
Owner Author

ocornut commented Feb 12, 2015

Well in your column header instead of calling Text() you can call Selectable(). You'll still need to do a bit of manual work. An eventual Column Header api will be a little better.

Based on your gist, you can replace

ImGui::Text("Path")

By

if (ImGui::Selectable("Path", comparison == pathCompare))
{
   reverse = (comparison == pathCompare) ? !reverse : false;
   comparison == pathCompare;
}

And you can remove the button and the text at the top, so that's the only thing you'll have.

@unpacklo
Copy link

Sorry, I should have been more clear.

The actual functionality I'm looking for is being able to select items in the column and have the entire row be indicated as selected across all the columns. So it's more like the ListBox() function but it operates over multiple columns instead of being restricted to just one column:

screenshot-imgui opengl2 example-8
screenshot-imgui opengl2 example-9
screenshot-imgui opengl2 example-10
screenshot-imgui opengl2 example-11

        const int cols = 5;
        const int rows = 100;
        static char items[cols][rows][32] = {};
        static bool selected[cols][rows] = {};
        static bool needInit = true;

        if (needInit)
        {
            for (int col = 0; col < cols; ++col)
            {
                for (int row = 0; row < rows; ++row)
                {
                    snprintf(items[col][row], sizeof(items[col][row]), "Col %d, Row %d", col + 1, row + 1);
                }
            }

            needInit = false;
        }

        ImGui::Begin("Column row selection");
        ImGui::Columns(cols);

        for (int col = 0; col < cols; ++col)
        {
            ImGui::Text("Col %d", col + 1);
            ImGui::Separator();

            for (int row = 0; row < rows; ++row)
            {
                if (ImGui::Selectable(items[col][row], &selected[col][row]))
                {
                    memset(selected, 0, sizeof(selected));

                    for (int selectedCol = 0; selectedCol < cols; ++selectedCol)
                    {
                        selected[selectedCol][row] = true;
                    }
                }
            }

            ImGui::NextColumn();
        }

        ImGui::Columns(1);
        ImGui::End();

Or is there some easy way to achieve this row selection functionality with the current API that I'm not aware of?

The actual information I'm looking for is to see which item or row index is currently selected. The GUI indication that something is selected is a nice touch which was made easy to do because of #129 selection!

@ocornut
Copy link
Owner Author

ocornut commented Feb 15, 2015

I see, you can't exactly do that right at the moment. Only one column will appear "hovered". But your example seems to be a good workaround, and you can simplify it as:

        const int cols = 5;
        const int rows = 100;
        static char items[cols][rows][32] = {};
        static int selected_row = -1;
        static bool needInit = true;

        if (needInit)
        {
            for (int col = 0; col < cols; ++col)
            {
                for (int row = 0; row < rows; ++row)
                {
                    snprintf(items[col][row], sizeof(items[col][row]), "Col %d, Row %d", col + 1, row + 1);
                }
            }

            needInit = false;
        }

        ImGui::Begin("Column row selection");
        ImGui::Columns(cols);

        for (int col = 0; col < cols; ++col)
        {
            ImGui::Text("Col %d", col + 1);
            ImGui::Separator();

            for (int row = 0; row < rows; ++row)
            {
                if (ImGui::Selectable(items[col][row], selected_row == row))
                {
                    selected_row = row;
                }
            }

            ImGui::NextColumn();
        }

        ImGui::Columns(1);
        ImGui::End();

What we can do is make Selectable span all columns, I'll have to see how this is better one. There's a bit of juggling the library may need to do with scissor rectangles.

@unpacklo
Copy link

One of the annoying parts about the current workaround is coming up with the label for the Selectable. Because the function takes in just a const char* label and doesn't allow for a format string and varargs, I need to prep the label ahead of time in a buffer myself:

for (int i = displayStart; i < displayEnd; ++i)
{
    char buffer[32];
    snprintf(buffer, sizeof(buffer), "%zu", pack.manifest->bytes[i]);

    if (ImGui::Selectable(buffer, i == selected))
    {
        selected = i;
    }
}

When I would really prefer to be able to do something like this:

for (int i = displayStart; i < displayEnd; ++i)
{
    if (ImGui::Selectable(i == selected, "%zu", pack.manifest->bytes[i]))
    {
        selected = i;
    }
}

I work around this currently with my own function to do the format string printing:

static bool ImGuiSelectable(bool selected, int id, const char *format, ...)
{
    char buffer[1024] = {};
    va_list list;
    va_start(list, format);
    vsnprintf(buffer, sizeof(buffer), format, list);
    va_end(list);

    ImGui::PushID(id);
    bool changed = ImGui::Selectable(buffer, selected);
    ImGui::PopID();

    return changed;
}

I provide the id so I can avoid collisions to keep the Selectable working.

@ocornut
Copy link
Owner Author

ocornut commented Feb 16, 2015

Hmm yes.. this has nothing to do with columns or the workaround for selectable on columns tho?

The reason I didn't go for a printf style api is because there's a second optional ImVec2 size parameter (default to 0.0f,0.0f = meaning full width, height of text).

Now I could invert the parameter order (the api is very new and if any early adopter used it it won't be too damaging of a change for them) and add two extra variants:

Selectable(bool, const char* fmt, ...)
Selectable(bool_, const char_ fmt, ...)
Selectable(bool, ImVec2, const char* fmt, ...)
Selectable(bool_, ImVec2, const char_ fmt, ...)

It's a bit "meh" but it probably does the job?

@ocornut
Copy link
Owner Author

ocornut commented Feb 17, 2015

Not sure about that anymore, because Selectable is mimicing the Button/Checkbox apis so inverting the parameter would be a little odd ? I guess TreeNode already follows that pattern though, and the format string are really helpful.

The way I suggest that you can extend the library by declare your own functions in the ImGui namespace (they can be declared/implemented in your own files or imgui_user.*). then you have a consistent syntax from the user's point of view. ImGui::SelectableEx() or whatever fits you best.

@unpacklo
Copy link

I definitely don't think its necessary to add in my particular case to the API. The basics are there to let people create convenience functions, as I have.

Just thought I'd give you a short update on some of the column work I've been doing recently. I implemented the sorting and selection as was mentioned earlier in this issue and also hacked up the column header thing we talked briefly about.

You can see some of this in action at http://youtu.be/uZEvcTREKFc. The column header is a separate set of columns and the actual content portion is a child with the same number of columns so I can scroll independently. What I had to do was grab out the offsets from the child content columns and set the header offsets to (nearly) the same values. This is why I can resize the child columns but not the header ones (by clicking the borders).

To get the header to line up with the children, I had to basically just hand tweak the offsets so that they line up, due to the fact that the child eats up some of the window space. You can see the tiny separation between the header and contents columns; about one or two pixels.

Through the process of implementing this, I actually didn't come up against any serious pains. Really, just the header part is a total kludge. I end up grabbing the content column offsets every frame, saving them off and then setting the header offsets the next frame. The other thing I do is I initialize the column offsets on the first frame so that each column has a pleasing width to start with, instead of just splitting the window evenly amongst all the columns.

I don't really have any standalone example code that you can compile, but almost everything in my code is based on snippets mentioned earlier. Nevertheless, I'll see if I can rip out the core that can compile and run independently of my own code.

@ocornut
Copy link
Owner Author

ocornut commented Feb 22, 2015

Don't worry about the code. I see what you are doing. There's lot to fix and improve, that's a good target to get sorting header + scrolling region done properly within ImGui.

I just realised that scrolling region add another pile of confusion - because they behave in a very specific manner when using (0,0) fill mode and don't declare their size to the parent window in this case, which easily leads to confusing mess when you don't get it right.

Working on various fixes now.

Nice to see it working for you already :)

@ocornut
Copy link
Owner Author

ocornut commented Feb 22, 2015

I fixed a bug (first one of the 3 above) which should make the pixel alignment correct.
However how are you creating the header section, are you creating them within another subchild ?

I found that to be necessary in order to keep alignment correct when the main window also has a scrollbar. Unfortunately it also means you have to explicitly size the child window because there's no way to automatically size a child window yet (mostly a problem of deciding on the API for that). So it's all a little messy but we're sorting through it little by little. Thanks for trying those things out.

ocornut added a commit that referenced this issue Feb 22, 2015
Actually ignore window padding, and don't subtract
window->DC.ColumnsStartX like older version did.
@ocornut
Copy link
Owner Author

ocornut commented Feb 22, 2015

Tip -
I need to clarify that there are lots of things you can do with SameLine() instead of Columns().
Columns() guarantee clipping and resizing but that's not always needed.
For simple "label -- value" display you can do simple alignment with

ImGui::Text("label"); 
ImGui::SameLine(column_x); 
ImGui::Text("value");

@unpacklo
Copy link

I create the header section just as a separate column call at the top level in the window. Then I do a BeginChild()/EndChild() for the actual contents of each column to get the separate scrolling portion. You can find the code dump of that part here: https://gist.github.com/Roflraging/a7469c63552365671b3d

You can see my functions GetColumnOffsetsFromImGui() (line 114) and SendColumnOffsetsToImGui() (line 53) are basically just used to save off the offsets of the content columns and then set the header column offsets so that they line up with the content columns.

There's also my little hack to get my columns to be initialized with offsets that are sized more sensibly for the actual contents in each column (line 108 to line 112, with initialization values set in lines 45 to 49).

ocornut added a commit that referenced this issue Apr 18, 2015
@ocornut
Copy link
Owner Author

ocornut commented Apr 18, 2015

c82f909
c46d563

Fixed an issue when actively dragged column being visually clipped would not release the active state (extension of a more general bug that was fixed recently).

Fixed an issue with feedback loops being created when dragging columns of an auto-resizing window (because columns width are stored as % of total window size. Maybe I will change that). Let me know if this causes any problems.

The contents sizing behaviours of columns is broken (and always was) btw it uses the maximum X cursor position to notify the host window of size, instead of the sum of contents width of all columns. I'll fix that later.

@ocornut
Copy link
Owner Author

ocornut commented May 1, 2015

We need a way to easily extends a the height of column to fill remaining vertical space (or be a fixed size). See past discussions in #170

@PathogenDavid
Copy link
Contributor

Ah, that makes sense in my book.

@ocornut
Copy link
Owner Author

ocornut commented Jun 25, 2019 via email

@RobertoMalatesta
Copy link

RobertoMalatesta commented Jun 25, 2019

Hi @ocornut

"when the new version is solid/useful enough I’ll put it on a test branch "

Consider doing it even if incomplete/stillindev
so some of us can give you some (I hope) constructive feedback.

"Currently encoding values 0 to 3 for each axis into flags, maybe that's enough?"

OK. Progressive refinement will do, I think.
Then in future APIs can be kept simple with a 0 default modifiable by an int < numcols/rows.
(and fixed cols/rows could be placed on right/bottom as well, and... <--forget this for now :D).

R

ocornut added a commit that referenced this issue Jul 18, 2019
…idgets.cpp (#125)

Also moved NextColumn between BeginColumn and NextColumn which makes it easier to work on that code.
ocornut added a commit that referenced this issue Jul 18, 2019
…tents rectangle within each column would wrongly use a WindowPadding.x instead of ItemSpacing.x like it always did. (#125, #2666)
ocornut added a commit that referenced this issue Jul 18, 2019
…rious values of ItemSpacing.x and WindowPadding.x. In particular, the right-most edge now reaches up to the clipping rectangle while ensuring that the right-most column clipping width matches others. (#125, #2666)
ocornut added a commit that referenced this issue Jul 19, 2019
… with various values of ItemSpacing.x and WindowPadding.x. In particular, the right-most edge now reaches up to the clipping rectangle while ensuring that the right-most column clipping width matches others. (#125, #2666)"

This reverts commit 6c16ba6.
ocornut added a commit that referenced this issue Jul 19, 2019
…e (removing WindowPadding.x*0.5 worth of asymmetrical/extraneous padding). (#125, #2666)

+ Moved a few things in BeginColumns().
ocornut added a commit that referenced this issue Jul 19, 2019
@ocornut
Copy link
Owner Author

ocornut commented Nov 7, 2019

Still working on those forever-coming-soon Tables. Improved the demo. Added first pass of settings persistance today. I've got one major thing I need to fix (which may take a few days) and I think I can start throwing the new API at some early private testers and enlarging that pool ..

image

@RobertoMalatesta
Copy link

RobertoMalatesta commented Dec 5, 2019

some early private testers

@ocornut Do you accept self candidatures ?

If yes, contact me privately and I'll try to give you a hand.

-R

ocornut added a commit that referenced this issue Dec 17, 2019
While the current code technically supports it, future code may not so we're putting the restriction ahead.
@ocornut
Copy link
Owner Author

ocornut commented Dec 30, 2019

New Tables API available for early testing: #2957
We will soon be able to close this issue :)
But I am sure there are many things not handled by Tables so will need your feedback there. Thanks!

ocornut added a commit that referenced this issue Mar 26, 2020
sergeyn pushed a commit to sergeyn/imgui that referenced this issue Mar 30, 2020
ocornut added a commit that referenced this issue Apr 2, 2020
…preemptively limited to 64 columns with an assert. (#3037, #125)

Essentially reverting 9d44406.
ocornut added a commit that referenced this issue Jun 8, 2020
…d channel (some stress tests in debug builds went 3->2 ms). (#125)

This change benefits Columns but was primarily made with Tables in mind.
ocornut added a commit that referenced this issue Jun 10, 2020
…ackground channel (some stress tests in debug builds went 3->2 ms). (#125)"

This reverts commit 9b3ce49.
ocornut added a commit that referenced this issue Nov 18, 2020
…ColumnFlags_*. (#125, #513, #913, #1204, #1444, #2142, #2707)

Affected: ImGuiColumnsFlags_None, ImGuiColumnsFlags_NoBorder, ImGuiColumnsFlags_NoResize, ImGuiColumnsFlags_NoPreserveWidths, ImGuiColumnsFlags_NoForceWithinWindow, ImGuiColumnsFlags_GrowParentContentsSize. Added redirection enums. Did not add redirection type.
@ocornut
Copy link
Owner Author

ocornut commented Nov 18, 2020

Everyone, as per 72de6f3 I have renamed the INTERNALS flags exposed for imgui_internal's BeginColumns() api. The commits tagged every issue tangentially related to those flags.

Basically

  • ImGuiColumnsFlags_NoBorder becomes ImGuiOldColumnFlags_NoBorder
  • ImGuiColumnsFlags_NoResize becomes ImGuiOldColumnFlags_NoResize,
  • ImGuiColumnsFlags_NoPreserveWidths becomes ImGuiOldColumnFlags_NoPreserveWidths
  • ImGuiColumnsFlags_NoForceWithinWindow becomes ImGuiOldColumnFlags_NoForceWithinWindow
  • ImGuiColumnsFlags_GrowParentContentsSize becomes ImGuiOldColumnFlags_GrowParentContentsSize

Those flags were never public API but given the fact that they were used by some columns users, I have defined redirection enums in imgui_internal.h when IMGUI_DISABLE_OBSOLETE_FUNCTIONS is not defined. Being internals api those old names will likely be obsoleted a little faster than the usual ~2 years.

As you can imagine this is in order to reduce naming confusion with the upcoming Tables api that has lots of columns falgs.

@frink
Copy link
Contributor

frink commented Nov 27, 2020

FYI: This didn't get merged to the docking branch. As a result, it's causing merge conflicts.

@ocornut
Copy link
Owner Author

ocornut commented Sep 10, 2021

Closing this 🎉

@ocornut ocornut closed this as completed Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants