search

rokups / ImNodes

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

Add scrollbars to the canvas #11

Open sphaero opened 5 years ago

sphaero commented 5 years ago

This is a fix for #5.

It replaces the canvas's offset vector with the scroll tracking of the window.

sphaero commented 5 years ago

btw, I'm not sure it works with imgui version 1.70. I had some issues so I upgraded to latest but can't recall what it was exactly. I created #12 for the cmake changes

rokups commented 5 years ago

Thank you for a great PR! This is the most useful feature. There is one detail. If for example i scroll to the left using mouse - scrollbar shrinks allowing me to go back to that empty space i once viewed by using scrollbar. But that empty space is otherwise not useful. I think it might be confusing to the user giving a false impression that some nodes are located there.

Do you think we could have scollbars providing access to are a which is a combined rectangle of all nodes + rectangle of entire current visible area? This way if we scroll to the either end of the canvas using scrollbar - we would end up looking at last nodes on that end. What do you think?

sphaero commented 5 years ago

Yes indeed. I'm aware of that and I'm still thinking about how to achieve that most easily. I was thinking to use SetCursorPos (which tracks CursorMaxPos) to track the minimal position of what is drawn on the canvas. This could be done in the BeginNode method.

I'll get to that. Either by adding it to this PR or a new one?

rokups commented 5 years ago

Maybe you could add something like ImRect combined_rect field to _CanvasStateImpl, then in each EndNode() append rect of the node to combined_rect. Also append rect of entire canvas in EndCanvas() and render scrollbar there. combined_rect should also be cleared in BeginCanvas().

I'll get to that. Either by adding it to this PR or a new one?

Your call. If you figure out a better implementation than current one - feel free to force-push to branch of this PR. Do not forget to rebase on master as well ;)

sphaero commented 5 years ago

I had a go at it, but it still has some issues. Basically all I do in EndNode() is:

canvas->contents_rect.Min = ImMin(canvas->contents_rect.Min, node_pos);
canvas->contents_rect.Max = ImMax(canvas->contents_rect.Max, node_pos + node_rect.GetSize());

In BeginCanvas() I initialize the contents_rect as the visible window rect in canvas coords:

canvas->contents_rect = ImRect(canvas->rect.Min + w->Scroll, canvas->rect.Min + w->Scroll + w->InnerClipRect.GetSize());

The contents_rect also needs to size on edge scrolling.

It can work but I run in the sticky window edges issue which gets worse now. I think it's caused by the scrollbars which need to be drawn or not since it happens only if there is a scrollbar not visible. I think it might also be done with only one rect in _CanvasStateImpl instead of two.

I need to test some more

rokups commented 4 years ago

We sort of forgot this PR for a while. What is it's state? Did 60f251c solve remaining issues and is this in mergeable state?

sphaero commented 4 years ago

Don't merge it yet. I still haven't found an approach that doesn't have the side effects. I haven't worked on it either. So either close this PR or leave it open until I have found a better approach.