[Goal] Make ImPlot3D handle >1M triangles

[miEsMar]

Hey, first of all, nice work. Glad you took the time to initialize it!

I am trying to transition to ImPlot3D to visualize Bispectra of nongaussian processes.
To do so, I am trying to do it via PlotMesh(), as it seems to be the only API providing indexed vertex drawing.
However, there seem to be some issues on this.

1. Why don't you accept raw vertices pointer (i.e. const T* vtx, where you template on the typename T)? Since those getters are just wrappers around vertices/indices, I don't think they add real value to the process, other than forcing the user to copy them over to construct ImPlot3DPoint *.
2. In RenderPrimitives, there seems to be a very low limit on the n. of primitives to be drawn. Is there a reason why you limit that?
   I have about 1M triangles that I am already drawing with a hand-written Vulkan shader, but I don't seem to be able to draw them with ImPlot3D.
3. This is a parsonal consideration. Are those Getters wrapper classes really required? I think you'd be much better just forwarding list of vertices (and indices where necessary) as the user provides them. I prefer C over C++, but, in a C++ fashion, why not switching to std::vector<T *> for both vertices and indices (assuming vertices coordinates are contiguous in memory)?

BTW, I don't yet know how many primitives I ended up adding and wrongly render to the 3D plot, but using immediate present mode, framerate dropped from about 200 FPS, to <20FPS.
Do you think we can optimize the core to allow for higher frame rates even with a large amount of vertices/triangles?

[brenocq]

1. Why don't you accept raw vertices pointer (i.e. const T* vtx, where you template on the typename T)? Since those getters are just wrappers around vertices/indices, I don't think they add real value to the process, other than forcing the user to copy them over to construct ImPlot3DPoint *.

I try to my best to be consistent with the other APIs, for your use case, do you prefer to have const T* xs, const T* ys, const T* zs instead of ImPlot3DPoint* vtx as arguments for the PlotMesh method? We could potentially also support offset and stride when using the const T* version.

2. In RenderPrimitives, there seems to be a very low limit on the n. of primitives to be drawn. Is there a reason why you limit that?

Snippet from the README:

Dear ImGui, by default, uses 16-bit indexing, which might cause issues with high-density 3D visualizations such as complex surfaces or meshes. This can lead to assertion failures, data truncation, or visual glitches. To avoid these problems, it's recommended to:

• Option 1: Enable 32-bit indices by uncommenting #define ImDrawIdx unsigned int in your ImGui imconfig.h file.
• Option 2: Ensure your renderer supports the ImGuiBackendFlags_RendererHasVtxOffset flag. Many official ImGui backends already support this functionality.

A lot of users face this problem, In the future, I may implement a workaround for this vertex limit when using 16-bit indexing. For now, these options should fix your problem.

3. This is a parsonal consideration. Are those Getters wrapper classes really required? I think you'd be much better just forwarding list of vertices (and indices where necessary) as the user provides them. I prefer C over C++, but, in a C++ fashion, why not switching to std::vector<T *> for both vertices and indices (assuming vertices coordinates are contiguous in memory)?

ImPlot3D tries to follow the same scheme as ImPlot, thus we use the Getters under the hood, which is fast enough for most users and keeps the codebase clean. We don't use std::vector in ImPlot3D because we try to follow the ImGui standard (which does not use any STD containers).

Do you think we can optimize the core to allow for higher frame rates even with a large amount of vertices/triangles?

In the future, I want to speed up the rendering by allowing the user to save the mesh/scatter/lines on the GPU to avoid passing the vertices from the CPU to the GPU every time before rendering. I'm not yet sure how this will look like, but this is a must for people wanting to visualize point clouds or large meshes.

[miEsMar]

Thanks for answering.

I try to my best to be consistent with the other APIs, for your use case, do you prefer to have const T* xs, const T* ys, const T* zs instead of ImPlot3DPoint* vtx as arguments for the PlotMesh method? We could potentially also support offset and stride when using the const T* version.

Not really. I was more about saying to also support bare bone const size_t n_vtxs, const T* vtxs, const size_t n_idxs, const ImDrawIdx *idxs (maybe is Vulkan specific?), to avoid in such cases (e.g. Vulkan indexed drawing) to need to create x, y, z lists which require an additional O(N) + C to copy from raw vertex data to 3D lists of coordinates.

Option 1: Enable 32-bit indices by uncommenting #define ImDrawIdx unsigned int in your ImGui imconfig.h file.

Oh yeah, forgot about this. Thanks, now I am able to render it.

In the future, I want to speed up the rendering by allowing the user to save the mesh/scatter/lines on the GPU to avoid passing the vertices from the CPU to the GPU every time before rendering. I'm not yet sure how this will look like, but this is a must for people wanting to visualize point clouds or large meshes.

I would definitely love to provide a little help on this if I am able to.
Also because, as of now, rendering my surface using ImPlot3D makes FPS drop from >300 to 0.1, which makes the UI practically unresponsive.