Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plots with more than 16384 points get silently truncated #30

Closed
jpieper opened this issue May 11, 2020 · 8 comments
Closed

Plots with more than 16384 points get silently truncated #30

jpieper opened this issue May 11, 2020 · 8 comments

Comments

@jpieper
Copy link
Contributor

jpieper commented May 11, 2020

Tested with 218ed09

https://github.com/jpieper/implot/tree/20200511-bigplot

20200511-bigplot

You can see that the more points which are culled, the further the plot gets drawn. I traced down a ways and couldn't figure out where the truncation was happening.

@epezent
Copy link
Owner

epezent commented May 11, 2020

Probably related to this (from the README):

If you plan to render several thousands lines or points, then you should consider enabling 32-bit indices by uncommenting #define ImDrawIdx unsigned int in your imconfig.h file, OR handling the ImGuiBackendFlags_RendererHasVtxOffset flag in your renderer (the official OpenGL3 renderer supports this). If you fail to do this, then you may at some point hit the maximum number of indices that can be rendered.

@jpieper
Copy link
Contributor Author

jpieper commented May 11, 2020

Yep, that did the trick, thanks!

@sergeyn
Copy link
Contributor

sergeyn commented May 25, 2020

Hi, PR #41 removes the need to enable 32bit indices. Regards!

@epezent epezent unpinned this issue Jun 1, 2020
@wsphillips
Copy link

wsphillips commented Jun 3, 2020

I'm not sure if this has been noted, but to be sure: using backends that support the ImGuiBackendFlags_RendererHasVtxOffset flag does not solve this issue. Only setting compile flags for 32-bit indices improves the total number of unculled points. As per ocornut/imgui#2591 Dear imgui prefers to stick with 16-bit indices for compatibility reasons:

If you've been using 32-bits indices, I would appreciate if you try to revert back to 16-bit indices and use this feature (need to update your renderer).

Below is a screenshot of the backend test provided in ocornut/imgui#2591 which seems to handle high vertex count appropriately side-by-side with ImPlot displaying a sin curve plot set for 100k points (instead of 1000 as in the demo). Similar to the OP, there's culling of point ranges >16k. [Edit: tested with default vulkan backend implementation of imgui]

Perhaps #41 solves this, I haven't tested it yet. But it's likely related to a lack of check similar to what's done in imgui_draw.cpp lines 610-617 where when _VtxCurrentIdx passes 16k, it should be reset to 0 and then adding a new drawing command.

vtxoffset

@epezent
Copy link
Owner

epezent commented Jun 3, 2020

Thanks for the notice.

ImGuiBackendFlags_RendererHasVtxOffset used to work before we started using DrawList.PrimReserve internally for performance reasons. #41 should fix it (it works by doing exactly what you suggest, as far as I understand). That PR currently only handles line plots, so I'm having to do a bit of reorganizing and abstraction to accommodate our other plot types before it can be merged. Expect it to be fixed soon.

Looping in @sergeyn if he wants to comment.

@epezent epezent reopened this Jun 3, 2020
@sergeyn
Copy link
Contributor

sergeyn commented Jun 3, 2020

nothing to comment. #41 fixes it once and for all. @wsphillips - you are correct, it'll render as much as can fit into current drawcmd and then allocate new one. And yeah, only line plots, I assume @epezent deals with the rest.

@epezent
Copy link
Owner

epezent commented Jun 7, 2020

@wsphillips , #41 has been merged and this should be fixed. Feel free to reopen the issue if you continue to have problems.

@epezent epezent closed this as completed Jun 7, 2020
@wsphillips
Copy link

@wsphillips , #41 has been merged and this should be fixed. Feel free to reopen the issue if you continue to have problems.

Yes! Confrimed working on my end. Thanks @epezent and @sergeyn !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants