hoffstadt / DearPyGui

Dear PyGui: A fast and powerful Graphical User Interface Toolkit for Python with minimal dependencies
https://dearpygui.readthedocs.io/en
MIT License
13.03k stars 678 forks source link

move_item_up/down crashes if there's a table in the widget tree #2268

Open v-ein opened 8 months ago

v-ein commented 8 months ago

Version of Dear PyGui

Version: 1.10.1 Operating System: Windows 10

My Issue/Question

If move_item_up or move_item_down is used on a widget that resides somewhere below a table in the widgets tree (as seen by the depth-first tree lookup), DPG crashes on the move_item_YYY call.

The reason is that MoveChildUp/Down functions in mvItemRegistry.cpp perform a depth-first lookup on the widgets tree, comparing each item's UUID to the specified UUID - that is, instead of using GetItem they search for the target object on their own. This is because they need to manipulate the parent object (the list of children in it). The following line of code does not expect child to be null:

if (child->uuid == uuid)

On the other hand, mvTable adds null pointers to slot 2 each time a column is added. I couldn't find why it does that and how slot 2 is used afterwards (it's reserved for draw commands, but why a column needs null at the corresponding location in slot 2?).

void mvTable::onChildAdd(std::shared_ptr<mvAppItem> item)
{

    if (item->type == mvAppItemType::mvTableColumn)
    {
        ...
        childslots[2].push_back(nullptr);

When MoveChildUp/Down perform their lookup, they stumble upon those nullptr's in slot 2 of the table. Boom.

To Reproduce

Steps to reproduce the behavior:

  1. Run the example below. It crashes right at startup.

Expected behavior

No crash.

Screenshots/Video

None.

Standalone, minimal, complete and verifiable example

import dearpygui.dearpygui as dpg

dpg.create_context()
dpg.setup_dearpygui()
dpg.create_viewport(title="Test", width=600, height=600)

with dpg.window():
    with dpg.table():
        dpg.add_table_column()

    btn = dpg.add_text("Lorem")
    dpg.move_item_up(btn)

dpg.show_viewport()
dpg.start_dearpygui()
dpg.destroy_context()
v-ein commented 8 months ago

There might be other places in the code that do not expect to see nullptr in the widget tree. I haven't checked that. It would be easy to add a non-null check to MoveChildYYY but I'd prefer to fix the root cause - if the table does not need those nulls, maybe get rid of them in the first place.

bzczb commented 6 months ago

Looking at mvTable::draw(), slot 2 was supposed to be for items in column headers, but I don't see any place where they're set.

AddItemWithRuntimeChecks() in mvItemRegistry.cpp has a case for tooltips that go into slot 2 when their grandparent is an mvTable. Maybe it was meant for tooltips to go with column headers.