rokups / ImNodes

Node graph implementation for Dear ImGui. Used in https://github.com/rokups/rbfx
MIT License
656 stars 57 forks source link

Horizontal scrollbar doesn't function #5

Open sphaero opened 5 years ago

sphaero commented 5 years ago

Changing the Window flags to:

    if (ImGui::Begin("ImNodes", nullptr, ImGuiWindowFlags_AlwaysVerticalScrollbar | ImGuiWindowFlags_HorizontalScrollbar))

The horizontal scrollbar doesn't appear to scroll.

sphaero commented 5 years ago

actually the vertical scrollbar also doesn't function. It conflicts with canvas->offset.

A fix would be to do something like:

    canvas->offset.x = -ImGui::GetScrollX();
    canvas->offset.y = -ImGui::GetScrollY();

But then scroll with the mouse fails which could be fixed with something like:

 (ImGui::IsMouseDragging(1))
        {
            canvas->offset += io.MouseDelta;
            ImGui::SetScrollX(-canvas->offset.x);
            ImGui::SetScrollY(-canvas->offset.y);
        }

But this only works if the window is smaller than the canvas. So it bites in the tail again. Perhaps a scrollbar is useless anyway, I'm not sure. Perhaps I'll dive into it again when it's needed.

rokups commented 5 years ago

You could also try putting canvas between BeginChild()/EndChild().

sphaero commented 5 years ago

That would be a nice workaround as well. I'll try that. Hope you don't mind me shooting in these little bugs. I'll get to pull requests once I find more time! Otherwise let me know how I can help.

rokups commented 5 years ago

Hope you don't mind me shooting in these little bugs. I'll get to pull requests once I find more time! Otherwise let me know how I can help.

All of that is greatly appreciated :] This will give more battle-testing before i get around to using this code. I scrapped my initial idea for which i needed nodes so they werent put to use yet.

rokups commented 4 years ago

@sphaero did BeginChild()/EndChild() work?

sphaero commented 4 years ago

From what I tested it doesn't:

if ( ImGui::Begin("clientspanel", NULL,  ImGuiWindowFlags_NoTitleBar | ImGuiWindowFlags_NoMove | ImGuiWindowFlags_NoResize | ImGuiWindowFlags_NoBringToFrontOnFocus) )
        {
            ImGui::BeginChild("testchild", ImVec2(0,0), ImGuiWindowFlags_AlwaysVerticalScrollbar | ImGuiWindowFlags_HorizontalScrollbar);
            ImNodes::BeginCanvas(gCanvas);
...
rokups commented 4 years ago

Child should have no scrollbars. if you want them - add them to parent. But that is kind of pointless. Canvas is infinite itself. How can we add scrollbars to something infinite?

sphaero commented 4 years ago
if ( ImGui::Begin("clientspanel", NULL,  ImGuiWindowFlags_AlwaysVerticalScrollbar | ImGuiWindowFlags_HorizontalScrollbar | ImGuiWindowFlags_NoTitleBar | ImGuiWindowFlags_NoMove | ImGuiWindowFlags_NoResize | ImGuiWindowFlags_NoBringToFrontOnFocus) )
        {
            ImGui::BeginChild("testchild");
            ImNodes::BeginCanvas(gCanvas);

still doesn't work.

But the scrollbar should be determined on the rectangle of its contents. The min/max pos of current nodes?

If I look at how it's done in Blender, it's indeed based on the contents of the canvas which makes sense to me. I'm going to read into the scrolling examples of ImGui

sphaero commented 4 years ago

Best working code for now is to set scroll value before calling BeginCanvas

ImGui::BeginChild("testchild");
canvas.offset.y = -ImGui::GetScrollY();
ImNodes::BeginCanvas(&canvas);

However this does not work if you move nodes upward (above pos 0)

sphaero commented 4 years ago

From what I read so far I think the canvas needs a rectangle from which scroll position can be determined. But I have a feeling it could be simpler I just need more experience with ImGui.

sphaero commented 4 years ago

Another test. It seems its easiest to just draw the canvas inside a Begin/EndChild as you suggested. I don't think offset is need then. You'll get a functioning scrolling. Here's concept code:

static ImNodes::CanvasState canvas{};

static ImVector<ImVec2> nodes{};

const ImGuiStyle& style = ImGui::GetStyle();

if (ImGui::Begin("ImNodes", nullptr, ImGuiWindowFlags_AlwaysVerticalScrollbar | ImGuiWindowFlags_AlwaysHorizontalScrollbar))
{
    // child gives our own coordspace
    ImGui::BeginChild("canvaschild", canvas.rect.GetSize() );

    ImVec2 offset = ImVec2(); //offset is not needed I guess

    ImDrawList* draw_list = ImGui::GetWindowDrawList();
    const float grid = 64.0f;

    ImVec2 pos = ImGui::GetWindowPos();
    ImVec2 size = ImGui::GetWindowSize();

    // For the test I added a rect to canvas to maintain the rectangle containing nodes
    // canvas rect is in screen coords
    // set the rect to window size if it's smaller
    if (canvas.rect.GetWidth() < GetWindowWidth() )
    {
        canvas.rect.Min = pos;
        canvas.rect.Max = pos + size;
    }

    ImU32 grid_color = ImColor(1.0f, 1.0f, 0.5f, 1.0f);//canvas->colors[ColCanvasLines]);
    for (float x = fmodf(offset.x, grid); x < size.x;)
    {
        draw_list->AddLine(ImVec2(x, 0) + pos, ImVec2(x, size.y) + pos, grid_color);
        x += grid;
    }

    for (float y = fmodf(offset.y, grid); y < size.y;)
    {
        draw_list->AddLine(ImVec2(0, y) + pos, ImVec2(size.x, y) + pos, grid_color);
        y += grid;
    }
    // Node 1 is just a button to add more nodes
    ImGui::BeginGroup();
    if ( ImGui::Button("Button1") ) {
        // add extra node at random pos
        ImGui::SetCursorPos(ImVec2(0,0));
        ImVec2 pos = ImVec2( rand() % 1000 -200, rand() % 1000 );
        nodes.push_back( pos );
        // add to canvas rectangle
        ImVec2 screenpos = pos + GetCursorScreenPos();
        canvas.rect.Add( ImRect(screenpos, screenpos + ImVec2(200, 100) ) );
    }
    ImGui::EndGroup();
    // draw the rectangle of the first node
    ImGui::GetForegroundDrawList()->AddRect(ImGui::GetItemRectMin(), ImGui::GetItemRectMax(), ImColor(1.0f,0.f,0.f,1.0f) );

    // draw extra nodes
    for (int i=0; i < nodes.size(); i++)
    {
        ImGui::BeginGroup();
        ImGui::PushID(i);
        ImGui::SetCursorPos(nodes[i]);
        ImGui::Button("button");
        ImGui::PopID();
        ImGui::EndGroup();
    }

    // draw canvas.rect
    ImGui::GetForegroundDrawList()->AddRect(canvas.rect.Min, canvas.rect.Max, ImColor(1.0f,1.f,0.f,1.0f) );

    ImGui::EndChild();
}
ImGui::End();

It needs extra code to correct the canvas when nodes have a negative positions.

sphaero commented 4 years ago

Note to self: Another approach would be to render a custom Scrollbar based on ImGui::Scrollbar

ocornut commented 4 years ago

Note to self: Another approach would be to render a custom Scrollbar based on ImGui::Scrollbar

You can call ScrollbarEx() directly for that.

I'm not sure I understand/follow the rest of the thread enough to understand it. Child scrolling will be based on its content sizes which is based on CursorPosMax-CursorStartPos or explicitely overriden via SetNextWindowContentsSize.

sphaero commented 4 years ago

Hey Omar. Thanks for responding. Forget previous comments. Basically we have an infinite canvas defined by a Begin/EndCanvas.

https://github.com/rokups/ImNodes/blob/a62c20f67b3d2f877bdc23acf74687ca6442fb78/ImNodes.cpp#L213

BeginCanvas draws the grid in the content area of the window and uses an offset to position the grid correctly.

https://github.com/rokups/ImNodes/blob/a62c20f67b3d2f877bdc23acf74687ca6442fb78/ImNodes.cpp#L249-L260

The offset is controlled using the mouse: https://github.com/rokups/ImNodes/blob/a62c20f67b3d2f877bdc23acf74687ca6442fb78/ImNodes.cpp#L225-L241

Then nodes can be drawn using Begin/EndNode which can contain regular ImGui widgets.

https://github.com/rokups/ImNodes/blob/a62c20f67b3d2f877bdc23acf74687ca6442fb78/ImNodes.cpp#L375

This works fine. However a user can move around the canvas using the right mouse but can end up with no visible Nodes and no clue where to go to. A scrollbar can give a visual clue of the whereabouts of nodes on the canvas. Hence the quest for adding a scrollbar to the canvas window.

The scrollbar somewhat works but only to the right or down. See this gif for example: recorded

Using the scrollbar to move around the canvas doesn't work yet but I think that would be easier than getting the scrollbar to function in all directions. recorded

So this currently not using any Begin/EndChild. Wrapping it in a Child gives the same result.

sphaero commented 4 years ago

I have a working concept which correctly handles the scrollbars:

void ShowDemoWindow(bool*)
{
    // holds the recangle of the canvas
    static ImRect canvas_rect = ImRect(0,0,0,0);

    // Window for the canvas
    if ( ImGui::Begin("Canvas", nullptr, ImGuiWindowFlags_HorizontalScrollbar ) )
    {
        const ImGuiWindow* w = ImGui::GetCurrentWindow();
        ImGui::PushID("canvaswidget");
        ImGui::ItemAdd(w->ContentsRegionRect, ImGui::GetID("canvas"));

        ImGuiIO& io = ImGui::GetIO();
        // use mouse to move around
        if (!ImGui::IsMouseDown(0) && ImGui::IsWindowHovered())
        {
            if (ImGui::IsMouseDragging(1))
            {
                // handle edges of canvas and increase it
                if (w->Scroll.x == 0.f && io.MouseDelta.x > 0.f )
                    canvas_rect.Min.x -= io.MouseDelta.x;
                if (w->Scroll.y == 0.f && io.MouseDelta.y > 0.f )
                    canvas_rect.Min.y -= io.MouseDelta.y;
                if (w->Scroll.x == w->ScrollMax.x && io.MouseDelta.x < 0.f )
                    canvas_rect.Max.x -= io.MouseDelta.x;
                if ( w->Scroll.y == w->ScrollMax.y && io.MouseDelta.y < 0.)
                    canvas_rect.Max.y -= io.MouseDelta.y;
                // todo: decrease the canvas
                else
                {
                    ImVec2 s = w->Scroll - io.MouseDelta;
                    SetScrollX(s.x);
                    SetScrollY(s.y);
                }
            }
        }

        // draw grid in the visible area of the window
        ImDrawList* draw_list = ImGui::GetWindowDrawList();
        const float grid = 64.0f;

        ImVec2 pos = w->ClipRect.Min;
        ImVec2 size = w->ClipRect.GetSize();
        ImVec2 canvas_offset =  w->Scroll + canvas_rect.Min;

        ImU32 grid_color = ImColor(0.5f,0.f, 1.0f, 1.0f);
        for (float x = fmodf(-canvas_offset.x, grid); x < size.x;)
        {
            draw_list->AddLine(ImVec2(x, 0) + pos, ImVec2(x, size.y) + pos, grid_color);
            x += grid;
        }

        for (float y = fmodf(-canvas_offset.y, grid); y < size.y;)
        {
            draw_list->AddLine(ImVec2(0, y) + pos, ImVec2(size.x, y) + pos, grid_color);
            y += grid;
        }
        // draw the position of canvas rectangle for feedback
        ImGui::GetForegroundDrawList()->AddRect(w->ContentsRegionRect.Min, w->ContentsRegionRect.Min + canvas_rect.GetSize(), ImColor(1.0f,0.f,1.f,1.0f) );

        ImGui::PopID();

        // set the size of the canvas if it's smaller than the content region
        ImVec2 csize = canvas_rect.GetSize();
        ImVec2 wsize = w->ContentsRegionRect.GetSize();
        if ( csize.x < wsize.x )
            canvas_rect.Max.x = canvas_rect.Min.x + wsize.x;
        if ( csize.y < wsize.y )
            canvas_rect.Max.y = canvas_rect.Min.y + wsize.y;
        ImGui::ItemSize(canvas_rect.GetSize());

    }
    ImGui::End();
}

recorded

I'm not sure if I'm using ImGui in the right way like this. I think I'm telling ImGui the size of the canvas using ItemAdd and ItemSize but I'm not sure if these methods serve this purpose.

Any feedback on this approach?

ocornut commented 4 years ago

I'm not sure if I'm using ImGui in the right way like this. I think I'm telling ImGui the size of the canvas using ItemAdd and ItemSize but I'm not sure if these methods serve this purpose.

Sorry I haven't had time to dig into. ItemSize() will layout the item and push the maximum cursor position (window->DC.CursorMaxPos) which can also be done by just calling SetCursorPos() or SetCursorScreenPos(). That maximum position will be used to calculate the reach of the scrollbars.

sphaero commented 4 years ago

No worries!

Indeed doing w->DC.CursorMaxPos = w->ContentsRegionRect.Min + canvas_rect.GetSize(); instead if ItemSize() seems to work as well. I'll try SetCursorPos as well, although it kind of feels counter intuitive?

Still this concept code is not complete yet. When the canvas increases into negative direction (top left) the coordinates of widgets get messed up. See when I move the canvas to the top left direction: recorded

ocornut commented 4 years ago

I'll try SetCursorPos as well, although it kind of feels counter intuitive?

Well, CursorMaxPos record the maxmum position that has ever been reach, so it makes sense there that a dummy call to SetCursorPos would set it.

void ImGui::SetCursorScreenPos(const ImVec2& pos)
{
    ImGuiWindow* window = GetCurrentWindow();
    window->DC.CursorPos = pos;
    window->DC.CursorMaxPos = ImMax(window->DC.CursorMaxPos, window->DC.CursorPos);
}
sphaero commented 4 years ago

Yes, I understand its working now.

I corrected the canvas coordinates by a simple

ImGui::SetCursorScreenPos(w->InnerClipRect.Min - w->Scroll + ImVec2(0,0) - canvas_rect.Min);`

before adding widgets.

I've switched from using ContentsRegionRect to InnerClipRect. It works now. Still some small artefacts:

I've added these changes to a branch: https://github.com/rokups/ImNodes/compare/master...sphaero:scrollbar. Still need to test zooming before I'll do a PR