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

Fix #1054, use ColorPicker control, fix reset to defaults #1063

Merged
merged 9 commits into from
Jun 8, 2020

Conversation

volconst
Copy link
Collaborator

@volconst volconst commented Jun 5, 2020

See individual commits for more info.

volconst added 7 commits June 3, 2020 23:07
Color settings were copied from PronterWindow.settings
to PronterWindow before reading them from config file
into PronterWindow.settings.
Now settings except 3D view move colors (e.g. gcview_color_background)
are applied without restart
Reset was not detected for applying because
it set both widget value and setting value.
Removed confirmation message box for each reset setting and let the
settings dialog OK button confirm
as with normal changes
Use wxpython widget ColourPickerCtrl.
Tested only on Ubuntu.
Move validation callbacks from Settings.__...()
to Setting.validate()
@kliment
Copy link
Owner

kliment commented Jun 5, 2020

Tested these, everything seems to work as described, but on_settings_change crashes when 3d view is disabled. Can you investigate? Also, I have invited you to this repository as a contributor - you have been that for a long time already, and it's only fair to make it official.

@volconst
Copy link
Collaborator Author

volconst commented Jun 5, 2020

@kliment , thank you for the contributor privilege and the project. It's a great honor for me, I appreciate your trust.
I will commit a fix in the next day or two in the pull request. Although I now may have right to push myself to official repo, I prefer you or another peer to review and integrate changes.

Extracted base class BaseViz to add structure,
reuse code, and reduce hasattr()
@volconst
Copy link
Collaborator Author

volconst commented Jun 7, 2020

@kliment , please merge this request if the problem is fixed now.

@kliment
Copy link
Owner

kliment commented Jun 7, 2020

It is! but something strange is still happening. When I have 2d view selected, start the program, and switch to 3d view, and load a gcode file, it does not get shown in the detailed view window (the one that shows when clicking the preview in the main window). When I switch back to 2d view, it does show in the detail view window but not in the 2d preview window. If I switch to 3d again, the main window preview is correct but the detail view is empty. Does this happen for you too?

@volconst
Copy link
Collaborator Author

volconst commented Jun 7, 2020

I experienced this kind of behavior, but I do not think this pull request breaks it. It is more like the PR exposes the behavior by replacing the 2D <-> 3D UI on the fly. The detail view appear empty, but if you drag the layer slider it shows layers, seems to be positioned below the first layer.
The intent of this PR was initially limited to switching 3D colors settings. The rest settings probably still require restart. I am more concerned with another problem - rotating of 3D is bumpy, EVT_PAINT is rarely fired on gl canvas.

@kliment
Copy link
Owner

kliment commented Jun 7, 2020

When I set both views to 3d, loading a file shows it in the detailed view on master and doesn't show it in this PR. On both master and this PR it's not shown correctly in the main window, but this PR breaks it in the detailed view as well. This happens even if the settings were not changed since last restart.

It's a regression in my recent commit.
Used _ for a dummy int variable, and _('localization')  broke.
Added logging where the exception was
silently ignored
@volconst
Copy link
Collaborator Author

volconst commented Jun 7, 2020

@kliment , yes the PR has a bug in loading for 3D view. I will commit fix tomorrow.

@kliment kliment merged commit e621728 into kliment:master Jun 8, 2020
@kliment
Copy link
Owner

kliment commented Jun 8, 2020

Thanks, that fixes the regression! I've merged this into master. There is still a problem where the main 3d view is not populated if both views are set to 3D, but that bug was already present in master. Thank you!

@volconst
Copy link
Collaborator Author

volconst commented Jun 8, 2020

Thank you.
The unpopulated main 3D view is related with the bumpy dragging, EVT_PAINT. Dragging is fine when the left or right neighbor panel of the 3D view is hidden.

@volconst volconst deleted the color-settings branch June 9, 2020 08:30
@a2k-hanlon
Copy link
Collaborator

Thank you for your work on this @volconst!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants