Skip to content

Commit

Permalink
Snap to character grid when resizing window (#3181)
Browse files Browse the repository at this point in the history
When user resizes window, snap the size to align with the character grid
(like e.g. putty, mintty and most unix terminals). Properly resolves
arbitrary pane configuration (even with different font sizes and
padding) trying to align each pane as close as possible.

It also fixes terminal minimum size enforcement which was not quite well
handled, especially with multiple panes.

This PR does not however try to keep the terminals aligned at other user
actions (e.g. font change or pane split). That is to be tracked by some
other activity.

Snapping is resolved in the pane tree, recursively, so it (hopefully)
works for any possible layout.

Along the way I had to clean up some things as so to make the resulting
code not so cumbersome:
1. Pane.cpp: Replaced _firstPercent and _secondPercent with single
   _desiredSplitPosition to reduce invariants - these had to be kept in
   sync so their sum always gives 1 (and were not really a percent). The
   desired part refers to fact that since panes are aligned, there is
   usually some deviation from that ratio.
2. Pane.cpp: Fixed _GetMinSize() - it was improperly accounting for
   split direction
3. TerminalControl: Made dedicated member for padding instead of
   reading it from a control itself. This is because the winrt property
   functions turned out to be slow and this algorithm needs to access it
   many times. I also cached scrollbar width for the same reason.
4. AppHost: Moved window to client size resolution to virtual method,
   where IslandWindow and NonClientIslandWindow have their own
   implementations (as opposite to pointer casting).

One problem with current implementation is I had to make a long call
chain from the window that requests snapping to the (root) pane that
implements it: IslandWindow -> AppHost's callback -> App ->
TerminalPage -> Tab -> Pane. I don't know if this can be done better.

## Validation Steps Performed
Spam split pane buttons, randomly change font sizes with ctrl+mouse
wheel and drag the window back and forth.

Closes #2834
Closes #2277
  • Loading branch information
mcpiroman authored and DHowett committed Jan 8, 2020
1 parent 913c5ec commit d4c5276
Show file tree
Hide file tree
Showing 20 changed files with 765 additions and 81 deletions.
7 changes: 7 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,13 @@ namespace winrt::TerminalApp::implementation
return _settings->GlobalSettings().GetShowTabsInTitlebar();
}

// Method Description:
// - See Pane::CalcSnappedDimension
float AppLogic::CalcSnappedDimension(const bool widthOrHeight, const float dimension) const
{
return _root->CalcSnappedDimension(widthOrHeight, dimension);
}

// Method Description:
// - Attempt to load the settings. If we fail for any reason, returns an error.
// Return Value:
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ namespace winrt::TerminalApp::implementation
winrt::Windows::UI::Xaml::ElementTheme GetRequestedTheme();
LaunchMode GetLaunchMode();
bool GetShowTabsInTitlebar();
float CalcSnappedDimension(const bool widthOrHeight, const float dimension) const;

Windows::UI::Xaml::UIElement GetRoot() noexcept;

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/AppLogic.idl
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace TerminalApp
Windows.UI.Xaml.ElementTheme GetRequestedTheme();
LaunchMode GetLaunchMode();
Boolean GetShowTabsInTitlebar();
Single CalcSnappedDimension(Boolean widthOrHeight, Single dimension);
void TitlebarClicked();
void WindowCloseButtonClicked();

Expand Down
73 changes: 73 additions & 0 deletions src/cascadia/TerminalApp/Pane.LayoutSizeNode.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

#include "pch.h"
#include "Pane.h"

Pane::LayoutSizeNode::LayoutSizeNode(const float minSize) :
size{ minSize },
isMinimumSize{ true },
firstChild{ nullptr },
secondChild{ nullptr },
nextFirstChild{ nullptr },
nextSecondChild{ nullptr }
{
}

Pane::LayoutSizeNode::LayoutSizeNode(const LayoutSizeNode& other) :
size{ other.size },
isMinimumSize{ other.isMinimumSize },
firstChild{ other.firstChild ? std::make_unique<LayoutSizeNode>(*other.firstChild) : nullptr },
secondChild{ other.secondChild ? std::make_unique<LayoutSizeNode>(*other.secondChild) : nullptr },
nextFirstChild{ other.nextFirstChild ? std::make_unique<LayoutSizeNode>(*other.nextFirstChild) : nullptr },
nextSecondChild{ other.nextSecondChild ? std::make_unique<LayoutSizeNode>(*other.nextSecondChild) : nullptr }
{
}

// Method Description:
// - Makes sure that this node and all its descendants equal the supplied node.
// This may be more efficient that copy construction since it will reuse its
// allocated children.
// Arguments:
// - other: Node to take the values from.
// Return Value:
// - itself
Pane::LayoutSizeNode& Pane::LayoutSizeNode::operator=(const LayoutSizeNode& other)
{
size = other.size;
isMinimumSize = other.isMinimumSize;

_AssignChildNode(firstChild, other.firstChild.get());
_AssignChildNode(secondChild, other.secondChild.get());
_AssignChildNode(nextFirstChild, other.nextFirstChild.get());
_AssignChildNode(nextSecondChild, other.nextSecondChild.get());

return *this;
}

// Method Description:
// - Performs assignment operation on a single child node reusing
// - current one if present.
// Arguments:
// - nodeField: Reference to our field holding concerned node.
// - other: Node to take the values from.
// Return Value:
// - <none>
void Pane::LayoutSizeNode::_AssignChildNode(std::unique_ptr<LayoutSizeNode>& nodeField, const LayoutSizeNode* const newNode)
{
if (newNode)
{
if (nodeField)
{
*nodeField = *newNode;
}
else
{
nodeField = std::make_unique<LayoutSizeNode>(*newNode);
}
}
else
{
nodeField.release();
}
}
Loading

0 comments on commit d4c5276

Please sign in to comment.