High-Level Feedback

[epezent]

@brenocq I had a chance to play around with your excellent library this morning and wanted to share some intiial thoughts. Overall the quality of this library is very good and I am quite impressed by it. In particular, I think you nailed interactions, and the API will be super easy for existing ImPlot/ImGui useres. Here's some more detailed thoughts:

API / Header

• Being virtually identical to ImPlot, I don't have much feedback other than to say great job!
• Document PlotX functions
• Consider putting the MIT license in your headers

Interactions / Widget Behavior

• For dragging/selecting/scrolling axes, I naturally want to hover the labels instead of the edge of the plot next to them. Consider making the label quad the widget, similar to ImPlot.
• Right clicking a plane currently shows context menu actions for the axis normal to that plane. This is a bit weird, I think it should show menus for the two axes in that plane instead.
• ImPlot weights zooming depending on where the cursor is in location horizontally/vertically in the plot; I don't think that is happening in ImPlot3D
• Consider adding the clipping flag to the context menu
• Consider allowing the user to define the shape of the 3D plot (i.e. it is currently a cube; what if I want a prism or to enlongate one axis?)
• I don't find the (x,y,z) display particularly useful or intuitive in 3D mode; either consider only displaying it when rotated to a plane (2D mode); or consider displaying it as e.g (x: 123, y: 456) if the z plane is hovered etc. I think text labels (e.g. "x:") will be a nice addition no matter what you decide to do.

Appearance / Styling

• Consider adding shading/gradients to plot planes (e.g. darker toward the corners) to add a depth effect

Building

• I encounted several conversion of double to float build warnings

ImPlot Sharing Potential

These are things I think we could converge toward shared implementation between ImPlot and ImPlot3D, if we wanted:

• Legend drawing/behavior
• Context menus
• Colormaps and associated API
• Styling vars/colors (ImPlot may need to add a few)
• Presumably a lot more as I dive into the library...

I would be open to a having a conversation regarding this. Feel free to reach out over email (you'll find it on my website) and we can setup a Zoom meeting, if you'd like.

Visual Bugs

Some strange sorth behavior:

Lines being drawn behind surfaes:

[brenocq]

@epezent thank you A LOT for taking the time to test the library and listing all the problems that you found until now! I created a discussion/issue for each of your bullet points to make it easier to track their progress and (hopefully) distribute the workload with other contributors.

Overall the quality of this library is very good and I am quite impressed by it. In particular, I think you nailed interactions, and the API will be super easy for existing ImPlot/ImGui users.

❤️

Being virtually identical to ImPlot, I don't have much feedback other than to say great job!

🕺🏻

Document PlotX functions
Consider putting the MIT license in your headers

#27 (todo) - Yes! The code is very poorly documented as of now, I'll make sure to have proper documentation in the next few days. I'll also add the license to all files, this sounds great.

For dragging/selecting/scrolling axes, I naturally want to hover the labels instead of the edge of the plot next to them. Consider making the label quad the widget, similar to ImPlot.

#28 (todo) - I highly agree the interaction with an axis should be done with a quad that covers both tick labels and axis labels, instead of edges. I initially wanted to implement it like this, but ended up implementing it with edges because it was easier 😛

Right clicking a plane currently shows context menu actions for the axis normal to that plane. This is a bit weird, I think it should show menus for the two axes in that plane instead.

#29 (done) - I also find it very weird. I tried to move fast and left this one broken behind kkkkk the plane context menu needs to be implemented.

ImPlot weights zooming depending on where the cursor is in location horizontally/vertically in the plot; I don't think that is happening in ImPlot3D

#30 (done) - It is not happening! I will take a look into the ImPlot implementation to also have it with the ImPlot3D.

Consider adding the clipping flag to the context menu

#31 (done) - Ooops!

Consider allowing the user to define the shape of the 3D plot (i.e. it is currently a cube; what if I want a prism or to enlongate one axis?)

#15 (done) - This is actually a feature I was already planning to implement! I do agree it is needed for many applications. There is also this world mode #17 that is a bit of a wild card and I'm not 100% about it.

I don't find the (x,y,z) display particularly useful or intuitive in 3D mode; either consider only displaying it when rotated to a plane (2D mode); or consider displaying it as e.g (x: 123, y: 456) if the z plane is hovered etc. I think text labels (e.g. "x:") will be a nice addition no matter what you decide to do.

#32 (idea) - I do agree they don't look great today, I think a bit more research is needed to see how other libraries display this information. I agree text labels (e.g. "x:") are a great addition!

Consider adding shading/gradients to plot planes (e.g. darker toward the corners) to add a depth effect

#33 (idea) - I agree that overall we lack a bit of depth information from the plots. It sounds great to play with gradient colors to bring back some of the depth information. Additionally, we could try to improve depth perception by adding shadows, or some kind of lighting. Even though we can't do per-pixel shading with the ImGuiDrawList, we could potentially do per-vertex shading.

I encounted several conversion of double to float build warnings

#11 (done) - I believe this should be now fixed! @Lumengine's PR #11 fixed many std::-related problems and double/float conversion problems. Let me know if you are still having these problems with the latest main branch. Additionally, I'm planning to standardize double use across the codebase in #39.

These are things I think we could converge toward shared implementation between ImPlot and ImPlot3D, if we wanted:

• Legend drawing/behavior
• Context menus
• Colormaps and associated API
• Styling vars/colors (ImPlot may need to add a few)
• Presumably a lot more as I dive into the library...

#34 (doing) - This sounds just amazing, I would love to keep the duplicated code to a minimum to improve maintainability ❤️

I would be open to a having a conversation regarding this. Feel free to reach out over email (you'll find it on my website) and we can setup a Zoom meeting, if you'd like.

Yes sir! I would love to, I'll send you the email soon. Looking forward to talking with you @epezent!

#35 (todo) - Very nasty-looking cube right there, I opened the issue.

#36 (todo) - Currently, lines are correctly clipped to the plot box area (only the segment that lies inside the box is rendered). The triangle clipping still needs to be implemented, and it is also the cause of some weird triangles when meshes are moved outside the plot box area.

Thank you a lot again for the amazing feedback! I will probably take some days to finish implementing all of them because I'm on vacation right now. Can't wait to have all of them fixed, if anyone else wants to contribute you are more than welcome!