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

7 gui #8

Open
wants to merge 69 commits into
base: master
Choose a base branch
from
Open

7 gui #8

wants to merge 69 commits into from

Conversation

danielskeenan
Copy link
Contributor

@danielskeenan danielskeenan commented Dec 15, 2024

Fixes #7.

Screenshot_20241215_155435

This ended up opening several large stinky cans of worms. There were a lot of assumptions in the current code around immediate CLI output that needed changing. Additionally, displaying information in a GUI requires more discrete access to data than the current program allowed. (i.e. no easy way to get the currently assigned probe level).

The end result was perhaps more refactoring than I planned on (or perhaps you wanted...). I had to touch the important bits today (the probe arrangement algorithm) and while it doesn't seem like I broke anything, I wanted to start a PR to get some more eyes.

Big changes

  • Requires Qt >= 6.7
  • Bumped to C++20
  • Added vcpkg to bring in deps. Can still use system deps without vcpkg, but makes it easier.
  • Use spdlog for command-line output, such that when run as a CLI it outputs to console as before, but when run as a GUI output is logged.
  • Site/Probe properties are compiled in, rather than be loaded from file at runtime. This was needed to give users a starting point; the alternative was forcing a text editor which, I feel, would defeat the purpose of a GUI.

Things still on my to-do list:

  • Recolor site links if they are providing a benefit.
  • Allow manual changing of assigned probes.
  • Display real-time changes to totals instead of only after running a solution.

One stray observation - while the program will build and run on Windows (VS2022), the simulation does not. I check the current code on Windows, and it freezes on the first iteration as well. I suspect multithreading issues, but haven't poked that hornets nest yet.

@minneyar
Copy link
Owner

There we go, that was actually pretty straightforward; the GUI was not setting the number of threads the solver should use, and the value was being initialized to 0, so it was simply not creating any threads. I added a widget to configure that and a default value for it, and now it's running for me.

@danielskeenan
Copy link
Contributor Author

Qt 6.2 still didn't fully embrace CMake, so had to add extra deploy scripts for that.

Runs on Windows now, but extraordinarily slow compared to on Linux. With 12 threads on my 12-core i7-8700, it averages about 1 second per iteration on Ubuntu 24.04. On Windows, it's about 10 seconds per iteration.

@danielskeenan danielskeenan marked this pull request as ready for review December 29, 2024 21:16
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.

GUI?
2 participants