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

sandbox: Condensed renderUI + fellow subsections #319

Open
wants to merge 76 commits into
base: main
Choose a base branch
from

Conversation

tcoyvwac
Copy link
Contributor

  • Reduced renderUI code-size in sandbox.
  • Refactored to use standard library algorithms and some dispatch-(v)-table techniques for readability.
    • This is to flatten the nested if-statements; reducing arrow-code.
    • Codebase being C++11 required dispatch-(v)-table technique.

@tcoyvwac
Copy link
Contributor Author

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

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

@tcoyvwac tcoyvwac force-pushed the fix/refactor/renderui-overlay-actions branch 2 times, most recently from 4b51c99 to 328dffc Compare December 25, 2022 23:47
@tcoyvwac tcoyvwac force-pushed the fix/refactor/renderui-overlay-actions branch 3 times, most recently from 4c6f96e to 7b898ab Compare December 26, 2022 00:21
@patriciogonzalezvivo
Copy link
Owner

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!

@tcoyvwac tcoyvwac force-pushed the fix/refactor/renderui-overlay-actions branch from cf1f792 to 8d6aec4 Compare December 28, 2022 07:56
@tcoyvwac tcoyvwac force-pushed the fix/refactor/renderui-overlay-actions branch 2 times, most recently from 1075f9f to af4979d Compare March 22, 2023 15:52
added render_m_showPasses

added render_m_plot

added render_cursor

added render_drag_and_drop_prompt

added render_help

added new namespace, prefix to internal functions
added triggered_text
added print_fbo_text

added print_buffers_text
added helper variables
added do_something_lightmap

added do_something_pyramid

added do_something_doublebuffers

added do_something_singlebuffer

defined stronger typename: renderer_process_info_t

strongtyped do_something_* functions

made helper local variable

added prompt_id parameter

moved Uniforms& assignment into function parameter list

null-mask unneeded parameters
added metadata-type to parameter list

strongtyped metadata{}

strongtyped vtable_metatadata_with_pred_t

made vtable_metatadata_with_pred_t{} a local strongtype
rename struct-name to: vtable_render_pass_t

strongtype render_pass_args_t

renamed function-names to: do_pass_{*}

added namespace: render_pass_actions
prefer "return on false"

prefer "return on false" (by continue;)

prefer "return on false"
@tcoyvwac tcoyvwac force-pushed the fix/refactor/renderui-overlay-actions branch from af4979d to 8d1d75c Compare October 22, 2023 18:31
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

Successfully merging this pull request may close these issues.

2 participants