patriciogonzalezvivo / glslViewer

Console-based GLSL Sandbox for 2D/3D shaders
BSD 3-Clause "New" or "Revised" License
4.57k stars 352 forks source link

sandbox: Condensed `renderUI` + fellow subsections #319

Open tcoyvwac opened 1 year ago

tcoyvwac commented 1 year ago
tcoyvwac commented 1 year ago

Happy to explain any parts of the changeset; also happy to rebase more if needed! :smile_cat:

This was a bit of a challenge as the codebase is currently strict C++11. Highly recommend my PR:#284 to reduce the pain! :sweatsmile: (PR:#284 passes all build CI so no problem right...?)_ :wink:

patriciogonzalezvivo commented 1 year ago

Happy Holidays! Thanks so much for this PRs (and update on the previous one) together with the detailed explanation on way upgrading to C++20. Also I appreciate the working around C++11 issues in this PR! I will start reviewing it in this next days, apologize in advance for doing it slowly.

Question about goals and motivations, beside flattened the code for readability (and for making it less error prune by reducing the nested logic. Did you were able to perceive performance gains on execution time?

First impressions: I'm not familiar with dispatch-(v)-table. I will start studding it : ) . At first glance, I do appreciate how clean renderUI() becomes. Great job! I'm a bit concern of how easy will be to add/modify the rendered UI in the future (the UI and ncurses console at the moment are the most experimental ends because much end up being a matter of taste or convenience, and I find my self tweaking things here and there often), In terms of code size seams there is not a significant difference, but if I understand right, that's because you the need to work around strict c++11, right?. I must admit that at the light of the dispatch-(v)-table, my original code looks naive and silly : )

Once again, thank you!