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.

Sign up for GitHub

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

Use DIPs for the window bounds when tearing out #15094

Merged
merged 3 commits into from
Apr 4, 2023
Merged

Use DIPs for the window bounds when tearing out #15094

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
53c2669
Use DIPs for the window bounds when tearing out
zadjii-msft Apr 3, 2023
302c8f1
easier
zadjii-msft Apr 3, 2023
28f583a
std::ignore the tests is what I did
zadjii-msft Apr 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/cascadia/TerminalApp/TerminalWindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,11 +613,11 @@ namespace winrt::TerminalApp::implementation
if (_contentBounds)
{
// If we've been created as a torn-out window, then we'll need to
// use that size instead. _contentBounds is in raw pixels. Huzzah!
// Just return that.
// use that size instead. _contentBounds is in DIPs. Scale
// accordingly to the new pixel size.
return {
_contentBounds.Value().Width,
_contentBounds.Value().Height
_contentBounds.Value().Width * scale,
_contentBounds.Value().Height * scale
};
}

Expand Down
12 changes: 6 additions & 6 deletions src/cascadia/WindowsTerminal/AppHost.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,6 @@ winrt::TerminalApp::TerminalWindow AppHost::Logic()
void AppHost::_handleMoveContent(const winrt::Windows::Foundation::IInspectable& /*sender*/,
winrt::TerminalApp::RequestMoveContentArgs args)
{
winrt::Windows::Foundation::Rect rect{};
winrt::Windows::Foundation::IReference<winrt::Windows::Foundation::Rect> windowBoundsReference{ nullptr };

if (args.WindowPosition() && _window)
Expand Down Expand Up @@ -1280,12 +1279,13 @@ void AppHost::_handleMoveContent(const winrt::Windows::Foundation::IInspectable&
dragPositionInPixels.y -= nonClientFrame.top;
windowSize = windowSize - nonClientFrame.size();

// Convert to DIPs for the size, so that dragging across a DPI boundary
// retains the correct dimensions.
const auto sizeInDips = windowSize.scale(til::math::rounding, 1.0f / scale);
til::rect inDips{ dragPositionInPixels, sizeInDips };
Copy link
Member

Choose a reason for hiding this comment

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

should the drag position be DIP'd as well? after all... we use it for positioning

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea this is weird - we already know exactly the pixel location we want the window to open at. We don't want a position to scale based off the resolution of the target display.

We could theoretically get the scale of the target display and unscale the position to DIPs then scale back, but that doesn't seem valuable? 🤷


// Use the drag event as the new position, and the size of the actual window.
rect = winrt::Windows::Foundation::Rect{ static_cast<float>(dragPositionInPixels.x),
static_cast<float>(dragPositionInPixels.y),
static_cast<float>(windowSize.width),
static_cast<float>(windowSize.height) };
windowBoundsReference = rect;
windowBoundsReference = inDips.to_winrt_rect();
}

_windowManager.RequestMoveContent(args.Window(), args.Content(), args.TabIndex(), windowBoundsReference);
Expand Down
4 changes: 2 additions & 2 deletions src/inc/til/size.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
}

template<typename TilMath, typename T, typename = std::enable_if_t<std::is_floating_point_v<T>>>
constexpr size scale(TilMath math, const T scale) const
[[nodiscard]] constexpr size scale(TilMath math, const T scale) const
{
return {
math,
Expand All @@ -84,7 +84,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
};
}

constexpr size divide_ceil(const size other) const
[[nodiscard]] constexpr size divide_ceil(const size other) const
{
// The integer ceil division `((a - 1) / b) + 1` only works for numbers >0.
// Support for negative numbers wasn't deemed useful at this point.
Expand Down