Prevent splitting panes into 0 width/height #2401 (#2450)

Fixes a crash that can occur when splitting pane that was so small that the target panes would have a width/height of 0, causing DxRenderer to fail when creating the device resources.

This PR prevents both the call to `App::AddHorizontal/VerticalSplit` and the creation of the `TermControl` if the split would fail.

Closes #2401

## Details

`App::_SplitPane` calls `focusedTab->CanAddHorizontalSplit/CanAddHorizontalSplit` before it initializes the `TermControl` to avoid having to deal with the cleanup. If a split cannot occur, it will simply return. 

**Question: Should we beep or something here?**

It then follows the same naming/flow style as the split operation, so: `Tab::CanAddHorizontalSplit -> Pane::CanSplitHorizontal ->Pane::_CanSplit`. The public pane methods will handle leaf/child the same as the current Split methods.

`_CanSplit` reuses existing logic like `_root.GetActualWidth/Height`, `Pane::_GetMinSize`, and the `Half` constant.

## Validation Steps Performed

1. Open a new tab
2. Attempt to split horizontally/vertically more than 6-8 times

Success: Pane will will eventually stop splitting rather than crashing the process.
This commit is contained in:
Richard Szalay 2019-08-21 08:38:45 +10:00 committed by Dustin L. Howett (MSFT)
parent 0c454f53e9
commit 09d79cb422
5 changed files with 118 additions and 2 deletions

View file

@ -1494,11 +1494,19 @@ namespace winrt::TerminalApp::implementation
const auto controlConnection = _CreateConnectionFromSettings(realGuid, controlSettings);
TermControl newControl{ controlSettings, controlConnection };
const int focusedTabIndex = _GetFocusedTabIndex();
auto focusedTab = _tabs[focusedTabIndex];
const auto canSplit = splitType == Pane::SplitState::Horizontal ? focusedTab->CanAddHorizontalSplit() :
focusedTab->CanAddVerticalSplit();
if (!canSplit)
{
return;
}
TermControl newControl{ controlSettings, controlConnection };
// Hookup our event handlers to the new terminal
_RegisterTerminalEvents(newControl, focusedTab);

View file

@ -777,6 +777,31 @@ void Pane::_ApplySplitDefinitions()
}
}
// Method Description:
// - Determines whether the pane can be split vertically
// Arguments:
// - splitType: what type of split we want to create.
// Return Value:
// - True if the pane can be split vertically. False otherwise.
bool Pane::CanSplitVertical()
{
if (!_IsLeaf())
{
if (_firstChild->_HasFocusedChild())
{
return _firstChild->CanSplitVertical();
}
else if (_secondChild->_HasFocusedChild())
{
return _secondChild->CanSplitVertical();
}
return false;
}
return _CanSplit(SplitState::Vertical);
}
// Method Description:
// - Vertically split the focused pane in our tree of panes, and place the given
// TermControl into the newly created pane. If we're the focused pane, then
@ -806,6 +831,31 @@ void Pane::SplitVertical(const GUID& profile, const TermControl& control)
_Split(SplitState::Vertical, profile, control);
}
// Method Description:
// - Determines whether the pane can be split horizontally
// Arguments:
// - splitType: what type of split we want to create.
// Return Value:
// - True if the pane can be split horizontally. False otherwise.
bool Pane::CanSplitHorizontal()
{
if (!_IsLeaf())
{
if (_firstChild->_HasFocusedChild())
{
return _firstChild->CanSplitHorizontal();
}
else if (_secondChild->_HasFocusedChild())
{
return _secondChild->CanSplitHorizontal();
}
return false;
}
return _CanSplit(SplitState::Horizontal);
}
// Method Description:
// - Horizontally split the focused pane in our tree of panes, and place the given
// TermControl into the newly created pane. If we're the focused pane, then
@ -834,6 +884,40 @@ void Pane::SplitHorizontal(const GUID& profile, const TermControl& control)
_Split(SplitState::Horizontal, profile, control);
}
// Method Description:
// - Determines whether the pane can be split.
// Arguments:
// - splitType: what type of split we want to create.
// Return Value:
// - True if the pane can be split. False otherwise.
bool Pane::_CanSplit(SplitState splitType)
{
const bool changeWidth = _splitState == SplitState::Vertical;
const Size actualSize{ gsl::narrow_cast<float>(_root.ActualWidth()),
gsl::narrow_cast<float>(_root.ActualHeight()) };
const Size minSize = _GetMinSize();
if (splitType == SplitState::Vertical)
{
const auto widthMinusSeparator = actualSize.Width - PaneSeparatorSize;
const auto newWidth = widthMinusSeparator * Half;
return newWidth > minSize.Width;
}
if (splitType == SplitState::Horizontal)
{
const auto heightMinusSeparator = actualSize.Height - PaneSeparatorSize;
const auto newHeight = heightMinusSeparator * Half;
return newHeight > minSize.Height;
}
return false;
}
// Method Description:
// - Does the bulk of the work of creating a new split. Initializes our UI,
// creates a new Pane to host the control, registers event handlers.

View file

@ -49,7 +49,10 @@ public:
bool ResizePane(const winrt::TerminalApp::Direction& direction);
bool NavigateFocus(const winrt::TerminalApp::Direction& direction);
bool CanSplitHorizontal();
void SplitHorizontal(const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control);
bool CanSplitVertical();
void SplitVertical(const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control);
void Close();
@ -79,6 +82,7 @@ private:
bool _HasFocusedChild() const noexcept;
void _SetupChildCloseHandlers();
bool _CanSplit(SplitState splitType);
void _Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control);
void _CreateRowColDefinitions(const winrt::Windows::Foundation::Size& rootSize);
void _CreateSplitContent();

View file

@ -203,6 +203,15 @@ void Tab::Scroll(const int delta)
});
}
// Method Description:
// - Determines whether the focused pane has sufficient space to be split vertically.
// Return Value:
// - True if the focused pane can be split horizontally. False otherwise.
bool Tab::CanAddVerticalSplit()
{
return _rootPane->CanSplitVertical();
}
// Method Description:
// - Vertically split the focused pane in our tree of panes, and place the
// given TermControl into the newly created pane.
@ -216,6 +225,15 @@ void Tab::AddVerticalSplit(const GUID& profile, TermControl& control)
_rootPane->SplitVertical(profile, control);
}
// Method Description:
// - Determines whether the focused pane has sufficient space to be split horizontally.
// Return Value:
// - True if the focused pane can be split horizontally. False otherwise.
bool Tab::CanAddHorizontalSplit()
{
return _rootPane->CanSplitHorizontal();
}
// Method Description:
// - Horizontally split the focused pane in our tree of panes, and place the
// given TermControl into the newly created pane.

View file

@ -19,7 +19,9 @@ public:
void SetFocused(const bool focused);
void Scroll(const int delta);
bool CanAddVerticalSplit();
void AddVerticalSplit(const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl& control);
bool CanAddHorizontalSplit();
void AddHorizontalSplit(const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl& control);
void UpdateFocus();