Too many vertices in ImDrawList

[fenix31415]

First, thank you for this awesome project!

The problem is that I want to deal with many vertices. This is why I get the following assertion error.

As the comment suggests, it is a good idea to skip drawing nodes that are outside of the editor view.

I am not quite familiar with DearImgui, in particular with this project -- I just started using it. But I think that it is possible to skip calls here: before update, it can check whether a node inside the render window and draw it only if it is in inside.

If this edit does not breaks imgui/ImNodeFlow architecture/invariants, I even can try to implement it myself and create a PR.

Thank you!

[Fattorino]

Creating a clip area and only rendering what's visible is the approach I prefer. For this what will need to happen is... first split the update function is a logic update and a render; then call the logic update each frame and the render only if necessary.

If you want to create a PR that follows this philosophy I'll be more than happy to review it and hopefully merge it. Unfortunately, I won't be able to fix this issue before the weekend, even tho it's a very serious bug.

Let me know if you decide to work on it yourself so we don't end up working on the same thing.

[fenix31415]

Thank you for answer!

There is no rush, I will try to implement it.

Do you mean by "creating a clip area" something specific from Imgui or just a "virtual" rectangle that is being drawn?

[Fattorino]

just a custom imaginary rectangle, nothing specific with ImGui

[fenix31415]

I did something and it works. Here you can look at my branch (2 commits).

Unfortunately, I can't say for sure which code from BaseNode::Update is related to processing and which is related to drawing. I can assume that there is something connected with passing values in links. But I do not even know how to test it.

Anyway, this code works for me -- I skip all update entirely. As I said before, I use only nodes and links, behavior is always empty function for me.

I do not think it deserves a PR -- I couldn't split to the logic update and a render update. Anyway, you could look at the code and I hope it helps you. At least, with detecting a "clip area".

Also, I have no idea what win_pos is (added to the grid lines). It is always zero for me. Likely, you need to take this variable into account too (I just cannot test how it affects).

[Fattorino]

I will look into it (and I already left a comment on the PR), and I'll finish working on it this weekend. As per the win_pos, yes it's always 0 because it's dead code now that I use a separate context for rendering everything. I'll remove it now :)

[Fattorino]

One question, how exactly did you run into this error? Just by adding a lot of nodes?

[fenix31415]

Yes. ~500 nodes and ~500 links. Also, with my fix, I can zoom to see all graph and again get an error.

[fenix31415]

In case someone runs into same problem. Even when rendering only visible nodes, I get the error, as I mentioned before. I still want to zoom out to see a whole graph.

I used ImGui debug window to see all used indices. It turned out that some elements takes too many of them. It is easier to render rectangles (two triangles) instead of circles (too many triangles). I found the following ones:

• Rounded corners. I set NodeStyle::radius to zero.
• Pins. I used a custom renderer which just uses AddNgonFilled for 4 segments.
• Connections. It is a bezier, which contains many triangles. Unfortunately there is hardcoded (look at smart_bezier(..)) delta variable. I set it to 0 in the library directly. Connections became straight lines.
• Text. Every symbol costs 2 triangles. I disabled rendering text with small zoom.

Those optimizations (+ rendering only visible nodes, of course) made possible to see almost entire graph.

[Fattorino]

Yes I'm aware of the need to simplify rendering at small scales, and I'm working on a way to simplify rendering proportionally to the zoom level in order to keep as much details as possible. The idea is that the user can select the detail scaling level based on his needs and the number of nodes

[Fattorino]

with commit b9ded55 I implemented what your comment says, just removing a lot of stuff when zoomed out, in the future I plan to create something a little more flexible but now I have very little time.

Zoomed in:

Zoomed out (< 0.7):

It's a very drastic cut that's why in the future I want to implement something more gradual

(I'm still working on the clip rectangle because it's not as easy as I thought to calculate what nodes must be rendered)

[fenix31415]

Cool! It would be cool to have opportunity to tweak the zoom border.

Speaking on clip rectangle, you can look at my code. I even draw the rectangle to test that all right.

[Fattorino]

tweak the zoom border

What do you mean by that?

Also, could you send me the link to the clip-rect code again, please?

[fenix31415]

You said the current zoom border is 0.7 now. I think it shouldn't be a hardcoded constant, but lay somewhere at style.

Clip rectangle code:https://github.com/fenix31415/ImNodeFlow/blob/clip_area/src/ImNodeFlow.cpp#L308

[Fattorino]

with the reduced rendering, without the clip-rect, doest it crash? (Also, I'm still working on the clip-rect. But updating the logic to allow data flow without rendering is not as easy as I thought so I'm doing some refactoring)

[fenix31415]

Yes it still crash. But it definitely lets render mode nodes (300-400). Before this commit I could render only ~100. In total, I have 550 of them for now.

Take your time about clip rectangle. I just use my code (for clip rectangle) for now + disallowed full zoom (where I can see whole graph).

[Fattorino]

I created a "dev" branch with the clip-rect code. I have some problems determining correctly these two variables: https://github.com/Fattorino/ImNodeFlow/blob/a214631530159cbeb42807ce91103e28c1b6cf83/src/ImNodeFlow.cpp#L289 https://github.com/Fattorino/ImNodeFlow/blob/a214631530159cbeb42807ce91103e28c1b6cf83/src/ImNodeFlow.cpp#L290

[Fattorino]

I was also thinking about adding this #define ImDrawIdx unsigned int. Having 32-bit indices just makes sense

[Fattorino]

@fenix31415 can you check if it crashes using 32-bit indices?

[fenix31415]

Where should I define it?

[Fattorino]

Im not sure what's the best way, you can read the comment near the assert

[Paolo-Oliverio]

To send less data is good, but there should be something that prevent it to scale under the hood.

It seems to be related to AppendDrawData.

for (int i = 0; i < draw_data->CmdListsCount; ++i) AppendDrawData(draw_data->CmdLists[i], m_origin, m_scale);

In this loop you atre trying to merge multiple imDrawList from multiple CmdLists everyone with it's own buffers of whatever size in a single ImDrawList ImGui::GetWindowDrawList() getting bigger and bigger indices due to dl->_IdxWritePtr[i] = idx_read[i] + vtx_start. up to a point it becomes an unmanageable number and crashes.

Backends needs the separated CmdLists so it could make it's own vertex buffer management and index offsetting.

you should try to modify cmdlists in place or something else that still let them arrive as separate and manageable cmdlist to the backend