Initial window height calculation regression

[mcpiroman]

Initial window height is off by titlebar height. This got regressed by #3394 in:

terminal/src/cascadia/WindowsTerminal/AppHost.cpp

Lines 197 to 210 in 357e835

	RECT islandFrame = {};
	bool succeeded = AdjustWindowRectExForDpi(&islandFrame, WS_OVERLAPPEDWINDOW, false, 0, dpix);
	// If we failed to get the correct window size for whatever reason, log
	// the error and go on. We'll use whatever the control proposed as the
	// size of our window, which will be at least close.
	LOG_LAST_ERROR_IF(!succeeded);
	if (_useNonClientArea)
	{
	islandFrame.top = -NonClientIslandWindow::topBorderVisibleHeight;
	}
	adjustedWidth = -islandFrame.left + islandWidth + islandFrame.right;
	adjustedHeight = -islandFrame.top + islandHeight + islandFrame.bottom;

That also blocks #3181 which relys on this size calculation.

Steps to reproduce

1. Set fontSize to 50
2. Set initialRows to 2
3. Launch terminal

Expected behavior

2 rows should fit in the window

Actual behavior

About 1.5 of a row fits.

cc @greg904.

[beviu]

Are you sure it's a regression? I just tried with 6f36f8b (before the PR was merged) and the size is also incorrect.

Before the PR, it was calling GetFrameMargins to calculate the size which looks like it includes the title bar's height because it calls GetDragAreaRect and adds the height to the result BUT during my debugging I found that during the initialization, the drag bar is not set yet so GetDragAreaRect returns 0 so GetFrameMargins doesn't include the title bar.

BTW here is a TODO for this (it was supposed say "the height of the title bar" for the second option instead of "tab bar" but I thought I had removed this comment so I didn't fix the comment):

terminal/src/cascadia/TerminalApp/App.cpp

Lines 337 to 342 in 357e835

	// TODO MSFT:21150597 - If the global setting "Always show tab bar" is
	// set or if "Show tabs in title bar" is set, then we'll need to add
	// the height of the tab bar here.
	return TermControl::GetProposedDimensions(settings, dpi);
	}

I didn't fix this bug because it was too hard and (I'm pretty sure, see above) the bug was already there before the PR.

Sorry for breaking your PR :(

Maybe adding static_cast<LONG>(_dragBar or _titlebar.ActualHeight() * GetCurrentDpiScale()) to the height will work? (but as I said above it will probably not work for the initial size calculation because _dragBar is not initialized)

[zadjii-msft]

For the record, I don't believe this regressed. I thought it was always broken 😝

[zadjii-msft]

Also, I think this one is probably a /dupe of #2061

[ghost]

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

[mcpiroman]

Ah ok then, sorry for my accusement. My PR stopped working after I merged with this one because of removed GetDragArea, but I didn't realized it wasn't working with initial sizing before. As to solving this problem, #3182 might help.