-
Notifications
You must be signed in to change notification settings - Fork 49
Feat viewport snapshot #149
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?
Feat viewport snapshot #149
Conversation
|
Sorry, currently i'm really busy on other projects. Your code is a huge addition and I still need time to make a deep review. At first sight, i can already tell that I prefere to keep close to ui standards. Can you shift the viewport toggler to a toolbar button. I like custom splitter, but non standard ergonomy can be disturbing for user. For the rest, I'm making a first pass of notes but I dont have time to review everything today. I'll do my best to finish the check this week and fully test the features |
|
It's okay, take your time! :) Alright, I will move the toggle button position to the toolbar then. |
luckylyk
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.
Ok I had time to check your code, sorry for the delay.
I wonder if the qsplitter custom class remains necessary with the button remove. I think you can cleanup using a QSplitter out of the box.
For the rest, if you applly all the remarks, i'm up to merge !
| camera_combo_box.addItems(cameras) | ||
| camera_combo_box.currentTextChanged.connect( | ||
| lambda: self.update_camera_viewport(camera_combo_box, | ||
| picker_model_name)) |
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.
use functools.partial instead of lambda as far as it is possible
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.
Actually, the reason why I used lambda for all the trigger is to keep the code consistent in the file.
there is a combobox there that has a dynamic argument if I remember, then using functools.partial didn't help with that because it only provides static arguments.
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.
This is what I meant by using functools.partial in some cases:
self.camera_name = "front"
self.field_toggle.triggered.connect(
partial(toggle_camera_settings, picker_model_name, self.camera_name, "field_chart"))
def update_camera_viewport(self, combo_box, panel):
"""
Update the camera in the active panel when a new camera is selected from the combo box.
"""
active_camera = combo_box.currentText()
cmds.modelPanel(panel, edit=True, camera=active_camera)
self.camera_name = active_cameraIn that code, even though self.camera_name is updated later, functools.partial only captures the value of self.camera_name at the time it is created. This is why lambda is used, to ensure the current value of self.camera_name is captured each time the triggered action is invoked.
And to keep the code consistent, I used lambda for all of the triggered actions.
038ab41 to
dfecaf6
Compare


Dwpicker_maya_viewport_integration.mp4