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

Maximize button for resizable instruments #7514

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
4 changes: 4 additions & 0 deletions include/InstrumentTrackWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
class QLabel;
class QLineEdit;
class QWidget;
class QMdiSubWindow;

namespace lmms
{
Expand Down Expand Up @@ -134,6 +135,9 @@ protected slots:
//! required to keep the old look when using a variable sized tab widget
void adjustTabSize(QWidget *w);

QMdiSubWindow* findSubWindowInParents();
void updateSubWindowState();

InstrumentTrack * m_track;
InstrumentTrackView * m_itv;

Expand Down
4 changes: 4 additions & 0 deletions include/SubWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ class LMMS_EXPORT SubWindow : public QMdiSubWindow
int titleBarHeight() const;
int frameWidth() const;

// TODO Needed to update the title bar when replacing instruments.
// Update works automatically if QMdiSubWindows are used.
void updateTitleBar();

protected:
// hook the QWidget move/resize events to update the tracked geometry
void moveEvent( QMoveEvent * event ) override;
Expand Down
4 changes: 4 additions & 0 deletions src/gui/SubWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,10 @@ int SubWindow::frameWidth() const
}


void SubWindow::updateTitleBar()
{
adjustTitleBar();
}


/**
Expand Down
90 changes: 66 additions & 24 deletions src/gui/instrument/InstrumentTrackWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
#include "MainWindow.h"
#include "PianoView.h"
#include "PluginFactory.h"
#include "PluginView.h"
#include "Song.h"
#include "StringPairDrag.h"
#include "SubWindow.h"
Expand Down Expand Up @@ -284,30 +283,10 @@ InstrumentTrackWindow::InstrumentTrackWindow( InstrumentTrackView * _itv ) :
updateInstrumentView();

QMdiSubWindow* subWin = getGUI()->mainWindow()->addWindowedWidget( this );
Qt::WindowFlags flags = subWin->windowFlags();

if (m_instrumentView->isResizable())
{
// TODO As of writing SlicerT is the only resizable instrument. Is this code specific to SlicerT?
const auto extraSpace = QSize(12, 208);
subWin->setMaximumSize(m_instrumentView->maximumSize() + extraSpace);
subWin->setMinimumSize(m_instrumentView->minimumSize() + extraSpace);

flags &= ~Qt::MSWindowsFixedSizeDialogHint;
flags |= Qt::WindowMaximizeButtonHint;
}
else
{
flags |= Qt::MSWindowsFixedSizeDialogHint;
flags &= ~Qt::WindowMaximizeButtonHint;

// Hide the Size and Maximize options from the system menu since the dialog size is fixed.
QMenu * systemMenu = subWin->systemMenu();
systemMenu->actions().at(2)->setVisible(false); // Size
systemMenu->actions().at(4)->setVisible(false); // Maximize
}

subWin->setWindowFlags(flags);
// The previous call should have given us a sub window parent. Therefore
// we can reuse this method.
updateSubWindowState();

subWin->setWindowIcon(embed::getIconPixmap("instrument_track"));
subWin->hide();
Expand Down Expand Up @@ -413,6 +392,8 @@ void InstrumentTrackWindow::modelChanged()
m_tuningView->keymapCombo()->setModel(m_track->m_microtuner.keymapModel());
m_tuningView->rangeImportCheckbox()->setModel(m_track->m_microtuner.keyRangeImportModel());
updateName();

updateSubWindowState();
}


Expand Down Expand Up @@ -717,5 +698,66 @@ void InstrumentTrackWindow::adjustTabSize(QWidget *w)
w->update();
}

QMdiSubWindow* InstrumentTrackWindow::findSubWindowInParents()
{
// TODO Move to helper? Does not seem to be provided by Qt.
auto p = parentWidget();

while (p != nullptr)
{
auto mdiSubWindow = dynamic_cast<QMdiSubWindow*>(p);
if (mdiSubWindow)
Copy link
Contributor

Choose a reason for hiding this comment

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

These 2 are other good candidates for "inlining".

{
return mdiSubWindow;
}
else
{
p = p->parentWidget();
}
}

return nullptr;
}

void InstrumentTrackWindow::updateSubWindowState()
JohannesLorenz marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

What I find strange about this method: It is no way specific to Instrument Track SubWindows (except the "isResizable" which I plan to generalize to both Instruments and Effects, i.e. to PluginView, but not all SubWindow types - nonetheless, SubWindow might just do a qobject_cast on its children, and if it's a PluginView, use the isResizable from there).

I guess your PR is only required for instruments, because they are the only SubWindow type that can be maximized and can both be resizable or not? In this case, the issue will occur for Effects sooner or later. And for the other SubWindow types, I assume it might not harm?

The advantage of putting that code into SubWindow would be cleaner encapsulation and no need to find the parent.

{
auto subWindow = findSubWindowInParents();
if (subWindow && m_instrumentView)
{
Qt::WindowFlags flags = subWindow->windowFlags();

if (m_instrumentView->isResizable())
{
// TODO As of writing SlicerT is the only resizable instrument. Is this code specific to SlicerT?
Copy link
Contributor

Choose a reason for hiding this comment

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

Note, this is still not fixed - see my previous comment.

const auto extraSpace = QSize(12, 208);
subWindow->setMaximumSize(m_instrumentView->maximumSize() + extraSpace);
subWindow->setMinimumSize(m_instrumentView->minimumSize() + extraSpace);

flags &= ~Qt::MSWindowsFixedSizeDialogHint;
Copy link
Contributor

@SpomJ SpomJ Nov 20, 2024

Choose a reason for hiding this comment

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

Consider also adding setFixedSize() here. MSWindowsFixedSizeDialogHint annoyingly appears to be a windows-only thing, or at least, isn't respected on wayland, while setFixedSize() or layout->setSizeConstraint(QLayout::SetFixedSize) is.

flags |= Qt::WindowMaximizeButtonHint;
}
else
{
flags |= Qt::MSWindowsFixedSizeDialogHint;
flags &= ~Qt::WindowMaximizeButtonHint;

// Hide the Size and Maximize options from the system menu since the dialog size is fixed.
QMenu * systemMenu = subWindow->systemMenu();
systemMenu->actions().at(2)->setVisible(false); // Size
systemMenu->actions().at(4)->setVisible(false); // Maximize
}

subWindow->setWindowFlags(flags);

// TODO This is only needed if the sub window is implemented with LMMS' own SubWindow class.
// If an QMdiSubWindow is used everything works automatically. It seems that SubWindow is
// missing some implementation details that QMdiSubWindow has.
auto subWin = dynamic_cast<SubWindow*>(subWindow);
if (subWin)
{
subWin->updateTitleBar();
}
}
}

} // namespace lmms::gui
Loading