-
Notifications
You must be signed in to change notification settings - Fork 74
Add grid settings #1269
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
base: main
Are you sure you want to change the base?
Add grid settings #1269
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1269 +/- ##
=======================================
Coverage 72.47% 72.47%
=======================================
Files 313 313
Lines 27391 27391
=======================================
Hits 19851 19851
Misses 7540 7540
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
svengoldberg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this new feature to user-select the grid resolution. Altough I also liked the old somehow automatic resolution, nevertheless I was sometimes annoyed by its "randomness". So, to me, this is an improvement!
Just find some small annotations from my side...
However, one general remark: This change "only" works for the XY grid. Tbh, I personally just found out today that there is also the option to choose XZ and YZ... To me, it looks like that your implementation is not fully compatible right now with a change of the grid.
When I start TiGL and try to choose the XZ grid, nothing happens. I need to open the settings and enter OK. Then, the change is done. Most likely, this is fixed by a small emit of a SIGNAL, I think...
| <string>Grid Size</string> | ||
| </property> | ||
| </widget> | ||
| </item> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for pickiness, but could you switch the two items to bring them in the "right" (I know that it does not matter here) order...?
| </item> | ||
| <item row="1" column="1"> | ||
| <widget class="QDoubleSpinBox" name="grid_origin_Y_spinbox"> | ||
| <property name="minimum"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also add a maximum value here?
| </item> | ||
| <item row="0" column="1"> | ||
| <widget class="QDoubleSpinBox" name="grid_origin_X_spinbox"> | ||
| <property name="minimum"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also add a maximum value here?
| <item row="2" column="1"> | ||
| <widget class="QDoubleSpinBox" name="grid_size_spinbox"> | ||
| <property name="minimum"> | ||
| <double>0.000000000000000</double> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TiGL produces a hard crash when this value is set exactely to 0 ....
Could you enter a small positive value here?
Description
Fixes #1220.
This PR adds the option to set the grid resolution via the settings. Currently, the settings are not preserved and are reset to default values each time TiGL opens. You can set the origin and grid spacing of the grid. The settings are accessible via the cog wheel / Settings menu.
How Has This Been Tested?
GUI only
Checklist: