Fix subwindow size limits (MainWindow::restoreWidgetState)

[yohannd1]

Previously fixed #8202, but that one got fixed first by #3532.
This PR now just corrects the subwindow size on load.

[yohannd1]

This was previously simply a fix for negative positions, but I've recently realized the minimum size hint code was also wrong. For example, upon loading the project the "Project Notes" window would have its width set to 800 if it was saved as less, even though its actual minimal amount when resizing is around 200 or 300.

Today I pushed a commit that updates the code style and uses w->minimumSizeHint() to decide whether the minimum size is right, and it seems to be working mostly fine.

One potential concern was re-introducing a bug (discussed in comments on the function in question) where in corrupt project files the size would be null, and I manually tried editing a .mmp file and the bug doesn't seem to be there. There seems to be a slight discrepancy between the actual minimum size the window allows and the one reported in the code I wrote.

[yohannd1]

There seems to be a slight discrepancy between the actual minimum size the window allows and the one reported in the code I wrote.

It seems that was because I was using the content widget for size hint, not the subwindow itself - I believe I've fixed it now.

[messmerd]

Haven't tested it, but the changes are pretty simple and straightforward

[sqrvrt]

any particular reason to use qMax instead of std::max?

[sqrvrt]

On further looking, the max as a whole at least in current state seems redundant, QWidget::setGeometry says it adjust the size implicitly.

If others think that it's better to leave this to be explicit, the .isValid() check would be redundant since size is guaranteed to be valid in prior, and x-y position is not checked. In theory removing the check in upstream would make this behave exactly like this PR.

[sakertooth]

any particular reason to use qMax instead of std::max?

The general rule of thumb is to prefer the STL when possible. It also helps in our effort to move away from Qt (more importantly in the core, i.e., the lmms namespace)

[yohannd1]

I believe qMax was being used before in the code - as of the latest commit I've changed it to std::max.

And, as for the explicit check, in previous tests I had the minimum size being loaded incorrectly, but QWidget::setGeometry wasn't being used yet (and indeed it does update correctly).

Nonetheless, the current normalGeometry.isValid() check introduces a specific case where, when the saved width/height is zero (e.g. in "corrupt" projects, as mentioned in an old comment), the window position is being ignored. For that case, I believe just adding just doing std::max(1, value) is alright, as the geometry rect will be valid and the resize method will adjust the size properly. Also implemented that in the latest commit.

[sqrvrt]

Nonetheless, the current normalGeometry.isValid() check introduces a specific case where, when the saved width/height is zero (e.g. in "corrupt" projects, as mentioned in an old comment), the window position is being ignored

Then wouldn't it be better to just remove the check? i.e.

long pile of useless diff

tldr: remove if and deindent block lol

-	if (normalGeometry.isValid())
-	{
-		// first restore the window, as attempting to resize a maximized window causes graphics glitching
-		win->setWindowState(win->windowState() & ~(Qt::WindowMaximized | Qt::WindowMinimized));
-
-		win->setGeometry(normalGeometry);
-
-		// set the window to its correct minimized/maximized/restored state
-		Qt::WindowStates winState = win->windowState();
-		auto winState = win->windowState();
-		winState = de.attribute("maximized").toInt()
-			? (winState | Qt::WindowMaximized)
-			: (winState & ~Qt::WindowMaximized);
-	}
+	// first restore the window, as attempting to resize a maximized window causes graphics glitching
+	win->setWindowState(win->windowState() & ~(Qt::WindowMaximized | Qt::WindowMinimized));
+
+	win->setGeometry(normalGeometry);
+
+	// set the window to its correct minimized/maximized/restored state
+	Qt::WindowStates winState = win->windowState();
+	auto winState = win->windowState();
+	winState = de.attribute("maximized").toInt()
+		? (winState | Qt::WindowMaximized)
+		: (winState & ~Qt::WindowMaximized);

And then again, without the check there's currently nothing that would behave differently compared to the version without explicit minimum heights, so unless you want to keep them for clarity they can be removed as well.

sorry if i'm repeating myself, i just want to be sure that i'm clear

[yohannd1]

That does make sense! I've now removed it. Just in case, I've added a comment (not sure if it is of much help either way...):

There used to be validity checks here, but they are not needed anymore.

Also, since win->windowState() returns a Qt::WindowStates, I've noticed the window-maximization logic can be simplified to this:

// Set the window to its correct "maximized?" state.
auto winState = win->windowState();
winState.setFlag(Qt::WindowMaximized, de.attribute("maximized").toInt());
win->setWindowState(winState);