giu icon indicating copy to clipboard operation
giu copied to clipboard

table: unable to display table with more than 64 columns

Open gucio321 opened this issue 4 years ago • 6 comments

Hi there, in refer to https://github.com/OpenDiablo2/HellSpawner/issues/335#issuecomment-893227410 I'd like to show the situation, we can notice the behavior: when I create a large table, e.g.:

        rowItems := make([]giu.Widget, numCols)
        for i := range rowItems {
                rowItems[i] = giu.Label(strconv.Itoa(i))
        }
        row := giu.TableRow(rowItems...)
full code
package main

import (
	"strconv"

	"github.com/AllenDang/giu"
)

func loop() {
	const numCols = 65

	rowItems := make([]giu.Widget, numCols)
	for i := range rowItems {
		rowItems[i] = giu.Label(strconv.Itoa(i))
	}
	row := giu.TableRow(rowItems...)

	giu.SingleWindow().Layout(
		giu.Table().Rows(row),
	)
}

func main() {
	wnd := giu.NewMasterWindow("table", 640, 480, 0)
	wnd.Run(loop)
}

i'm getting the following panic:

table: imgui_tables.cpp:323: bool ImGui::BeginTableEx(const char*, ImGuiID, int, ImGuiTableFlags, const ImVec2&, float): Assertion `columns_count > 0 && columns_count <= 64 && "Only 1..64 columns allowed!"' failed.
SIGABRT: abort
PC=0x7f7e2133d9e5 m=0 sigcode=18446744073709551610
signal arrived during cgo execution

gucio321 avatar Aug 09 '21 18:08 gucio321

This is due to a limitation of imgui not of giu.

From imgui_tables.cpp

// Sanity checks
IM_ASSERT(columns_count > 0 && columns_count <= IMGUI_TABLE_MAX_COLUMNS && "Only 1..64 columns allowed!");

So imgui asserts and you get a SIGABRT: abort

r00tc0d3 avatar Aug 09 '21 19:08 r00tc0d3

you're right. I see an issue in imgui repo: https://github.com/ocornut/imgui/issues/3572 maybe the suggested workaround, would work?

gucio321 avatar Aug 10 '21 17:08 gucio321

from https://github.com/ocornut/imgui imgui_demog.cpp:

        if (ImGui::BeginTable("table_scrolly", 3, flags, outer_size))
        {
            ImGui::TableSetupScrollFreeze(0, 1); // Make top row always visible
            ImGui::TableSetupColumn("One", ImGuiTableColumnFlags_None);
            ImGui::TableSetupColumn("Two", ImGuiTableColumnFlags_None);
            ImGui::TableSetupColumn("Three", ImGuiTableColumnFlags_None);
            ImGui::TableHeadersRow();

            // Demonstrate using clipper for large vertical lists
            ImGuiListClipper clipper;
            clipper.Begin(1000);
            while (clipper.Step())
            {
                for (int row = clipper.DisplayStart; row < clipper.DisplayEnd; row++)
                {
                    ImGui::TableNextRow();
                    for (int column = 0; column < 3; column++)
                    {
                        ImGui::TableSetColumnIndex(column);
                        ImGui::Text("Hello %d,%d", column, row);
                    }
                }
            }
            ImGui::EndTable();

gucio321 avatar Aug 18 '21 16:08 gucio321

ImGuiListClipper work on the vertical axis which is were the count can grow the most. Both TableSetColumnIndex() and TableNextColumn() return a bool reporting the column visibility so you can skip rendering your contents.

gucio321 avatar Aug 18 '21 18:08 gucio321

as an alternative, we can update the internal imgui tables mechanism like suggested in https://github.com/ocornut/imgui/issues/3572#issuecomment-885744366

so, to sum up: there are 2 possible workarounds

  1. externally affect imgui table to display only the visible rows and bypass 64 columns limit
  2. modify the tables mechanism to remove columns limit

@AllenDang should we focus on working around this issue or just wait until the upstream issue gets solved?

gucio321 avatar Sep 05 '21 19:09 gucio321

@gucio321 IMO, let's wait upstream issue gets solved. I've been suffered much to apply custom patches, for now the power-saving patch. I have to do line-by-line diff and merge each time upgrading imgui.

AllenDang avatar Sep 06 '21 05:09 AllenDang

@AllenDang upstream issue is solved now IMO, we should wait for cimgui-go migration with that anyway.

gucio321 avatar Jan 21 '23 10:01 gucio321

@AllenDang upstream issue has been closed. I think it may be fixed after cimgui migration

gucio321 avatar May 08 '23 16:05 gucio321

@gucio321 Yes, after migration this should be fixed.

AllenDang avatar May 09 '23 02:05 AllenDang

I still get the error with more than 64 columns, using @latest 0.7.0 and master is not compiling

@gucio321

fnicastri avatar Feb 03 '24 17:02 fnicastri

This works on matter, if it doesn't compile it a separated issue

gucio321 avatar Feb 03 '24 17:02 gucio321

good to know!

can you point me at a working commit?

fnicastri avatar Feb 03 '24 18:02 fnicastri

this issue was related to old imgui. master uses cimgui-go so relatively new imgui where 64 column tables works. THats why I said master works. (but may not compile, however it does for me)

gucio321 avatar Feb 04 '24 16:02 gucio321

Thank you

fnicastri avatar Feb 05 '24 10:02 fnicastri