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

Hide viewer buttons when roto/tracker properties panel is minimized #748

Merged
merged 4 commits into from
Jan 11, 2022

Conversation

devernay
Copy link
Member

@devernay devernay commented Jan 11, 2022

What type of PR is this? (Check one of the boxes below)

  • Bug fix (non-breaking change which fixes an issue)

What does this pull request do?

  • addresses issue commented in #745 (review)
  • when panel is maximized, buttons are displayed even if viewer is not in render path (see Only display the overlays for nodes that are in the viewer render path and have their properties panel maximized. #744 for an explanation of that feature). This is not 100% satisfactory, but better than before IMHO
  • do not show the viewer buttons if a node is selected in the nodegraph but has its properties panel minimized
  • maximize properties panel when double-clicking a node in the nodegraph
  • minimized() and maximized() were originally two separate signals (in NodeSettingsPanel), so I kept it that way, but we could have simpler definitions of NodeGui::onSettingsPanelMinimized() and NodeGui::onSettingsPanelMaximized() that simply call NodeGui::onSettingsPanelClosed(true) or NodeGui::onSettingsPanelClosed(false).

Show a few screenshots (if this is a visual change)

N/A

Have you tested your changes (if applicable)? If so, how?

  • create a rotopaint node, connect to the viewer
  • create a rotopaint shape
  • minimize the rotopaint panel, the buttons in the Viewer should be removed [FIXED IN THIS PR]
  • create a tracker node, its buttons should appear in the viewer
  • select the rotopaint node in the nodegraph, its viewer buttons should not appear [FIXED IN THIS PR]
  • double-click the rotopaint node in the nodegraph, it will be maximized [FIXED IN THIS PR] and its viewer buttons replace those of the tracker node

- addresses issue commented in #745 (review)
- when panel maximized, buttons are displayed even if viewer is not in render path (see #744 for an explanation of that feature). This is not 100% satisfcatory, but better than before IMHO
- minimized() and maximized() were originally two separate signals (in NodeSettingsPanel), so I kept it that way, but we could have simpler definitions of NodeGui::onSettingsPanelMinimized() and NodeGui::onSettingsPanelMaximized() that simply call NodeGui::onSettingsPanelClosed(true) or NodeGui::onSettingsPanelClosed(false).
@devernay devernay marked this pull request as draft January 11, 2022 01:51
@devernay
Copy link
Member Author

still an issue: when panel is minimize but node is selected in the nodegraph, the buttons are displayed

- maximize properties panel when double-clicking a node in the nodegraph
- do not show the viewer buttons if a node is selected in the nodegraph but has its properties panel minimized
@devernay devernay removed the request for review from YakoYakoYokuYoku January 11, 2022 02:17
@devernay devernay marked this pull request as ready for review January 11, 2022 02:17
@@ -88,6 +88,8 @@ NodeSettingsPanel::NodeSettingsPanel(const MultiInstancePanelPtr & multiPanel,


QObject::connect( this, SIGNAL(closeChanged(bool)), NodeUi.get(), SLOT(onSettingsPanelClosedChanged(bool)) );
QObject::connect( this, SIGNAL(minimized()), NodeUi.get(), SLOT(onSettingsPanelMinimized(bool)) );
QObject::connect( this, SIGNAL(maximized()), NodeUi.get(), SLOT(onSettingsPanelMaximized(bool)) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might have an oopsie here (at least in Qt5)

QObject::connect: No such slot Natron::NodeGui::onSettingsPanelMinimized(bool) in ../../Gui/NodeSettingsPanel.cpp:91
QObject::connect: No such slot Natron::NodeGui::onSettingsPanelMaximized(bool) in ../../Gui/NodeSettingsPanel.cpp:92
QObject::connect: No such slot Natron::NodeGui::onSettingsPanelMinimized(bool) in ../../Gui/NodeSettingsPanel.cpp:91
QObject::connect: No such slot Natron::NodeGui::onSettingsPanelMaximized(bool) in ../../Gui/NodeSettingsPanel.cpp:92

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! These lines are useless

Copy link
Member

@YakoYakoYokuYoku YakoYakoYokuYoku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from that bad connection I could confirm that this works.

@devernay devernay merged commit b530320 into RB-2.4 Jan 11, 2022
@YakoYakoYokuYoku YakoYakoYokuYoku deleted the remove-viewer-buttons-when-panel-minimized branch June 18, 2022 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants