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.

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

fix: improve the way frameless windows are handled on Windows #16596

Merged
merged 4 commits into from
Jan 31, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions atom/browser/native_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,13 @@ void NativeWindow::InitFromOptions(const mate::Dictionary& options) {
} else if (options.Get(options::kCenter, &center) && center) {
Center();
}

bool use_content_size = false;
options.Get(options::kUseContentSize, &use_content_size);

// On Linux and Window we may already have maximum size defined.
extensions::SizeConstraints size_constraints(GetContentSizeConstraints());
extensions::SizeConstraints size_constraints(
use_content_size ? GetContentSizeConstraints() : GetSizeConstraints());
int min_height = 0, min_width = 0;
if (options.Get(options::kMinHeight, &min_height) |
options.Get(options::kMinWidth, &min_width)) {
Expand All @@ -94,8 +99,6 @@ void NativeWindow::InitFromOptions(const mate::Dictionary& options) {
options.Get(options::kMaxWidth, &max_width)) {
size_constraints.set_maximum_size(gfx::Size(max_width, max_height));
}
bool use_content_size = false;
options.Get(options::kUseContentSize, &use_content_size);
if (use_content_size) {
SetContentSizeConstraints(size_constraints);
} else {
Expand Down
38 changes: 30 additions & 8 deletions atom/browser/native_window_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,27 @@ void FlipWindowStyle(HWND handle, bool on, DWORD flag) {
style &= ~flag;
::SetWindowLong(handle, GWL_STYLE, style);
}

// Similar to the ones in display::win::ScreenWin, but with rounded values
// These help to avoid problems that arise from unresizable windows where the
// original ceil()-ed values can cause calculation errors, since converting
// both ways goes through a ceil() call. Related issue: #15816
gfx::Rect ScreenToDIPRect(HWND hwnd, const gfx::Rect& pixel_bounds) {
float scale_factor = display::win::ScreenWin::GetScaleFactorForHWND(hwnd);
gfx::Rect dip_rect = ScaleToRoundedRect(pixel_bounds, 1.0f / scale_factor);
dip_rect.set_origin(
display::win::ScreenWin::ScreenToDIPRect(hwnd, pixel_bounds).origin());
return dip_rect;
}

gfx::Rect DIPToScreenRect(HWND hwnd, const gfx::Rect& pixel_bounds) {
float scale_factor = display::win::ScreenWin::GetScaleFactorForHWND(hwnd);
gfx::Rect screen_rect = ScaleToRoundedRect(pixel_bounds, scale_factor);
screen_rect.set_origin(
display::win::ScreenWin::DIPToScreenRect(hwnd, pixel_bounds).origin());
return screen_rect;
}

#endif

class NativeWindowClientView : public views::ClientView {
Expand Down Expand Up @@ -238,7 +259,7 @@ NativeWindowViews::NativeWindowViews(const mate::Dictionary& options,
if (!has_frame()) {
// Set Window style so that we get a minimize and maximize animation when
// frameless.
DWORD frame_style = WS_CAPTION;
DWORD frame_style = WS_CAPTION | WS_OVERLAPPED;
if (resizable_)
frame_style |= WS_THICKFRAME;
if (minimizable_)
Expand Down Expand Up @@ -1076,7 +1097,10 @@ bool NativeWindowViews::IsVisibleOnAllWorkspaces() {
}

gfx::AcceleratedWidget NativeWindowViews::GetAcceleratedWidget() const {
return GetNativeWindow()->GetHost()->GetAcceleratedWidget();
if (GetNativeWindow() && GetNativeWindow()->GetHost())
return GetNativeWindow()->GetHost()->GetAcceleratedWidget();
else
return nullptr;
Copy link
Member

Choose a reason for hiding this comment

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

CI failed on Linux with:

../../electron/atom/browser/native_window_views.cc:1103:12: error: cannot initialize return object of type 'gfx::AcceleratedWidget' (aka 'unsigned long') with an rvalue of type 'nullptr_t'
    return nullptr;

}

NativeWindowHandle NativeWindowViews::GetNativeWindowHandle() const {
Expand All @@ -1091,8 +1115,8 @@ gfx::Rect NativeWindowViews::ContentBoundsToWindowBounds(
gfx::Rect window_bounds(bounds);
#if defined(OS_WIN)
HWND hwnd = GetAcceleratedWidget();
gfx::Rect dpi_bounds = display::win::ScreenWin::DIPToScreenRect(hwnd, bounds);
window_bounds = display::win::ScreenWin::ScreenToDIPRect(
gfx::Rect dpi_bounds = DIPToScreenRect(hwnd, bounds);
window_bounds = ScreenToDIPRect(
hwnd,
widget()->non_client_view()->GetWindowBoundsForClientBounds(dpi_bounds));
#endif
Expand All @@ -1113,17 +1137,15 @@ gfx::Rect NativeWindowViews::WindowBoundsToContentBounds(
gfx::Rect content_bounds(bounds);
#if defined(OS_WIN)
HWND hwnd = GetAcceleratedWidget();
content_bounds.set_size(
display::win::ScreenWin::DIPToScreenSize(hwnd, content_bounds.size()));
content_bounds.set_size(DIPToScreenRect(hwnd, content_bounds).size());
RECT rect;
SetRectEmpty(&rect);
DWORD style = ::GetWindowLong(hwnd, GWL_STYLE);
DWORD ex_style = ::GetWindowLong(hwnd, GWL_EXSTYLE);
AdjustWindowRectEx(&rect, style, FALSE, ex_style);
content_bounds.set_width(content_bounds.width() - (rect.right - rect.left));
content_bounds.set_height(content_bounds.height() - (rect.bottom - rect.top));
content_bounds.set_size(
display::win::ScreenWin::ScreenToDIPSize(hwnd, content_bounds.size()));
content_bounds.set_size(ScreenToDIPRect(hwnd, content_bounds).size());
#endif

if (root_view_->HasMenu() && root_view_->IsMenuBarVisible()) {
Expand Down
2 changes: 2 additions & 0 deletions atom/browser/native_window_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,8 @@ class NativeWindowViews : public NativeWindow,

ui::WindowShowState last_window_state_;

gfx::Rect last_normal_placement_bounds_;

// There's an issue with restore on Windows, that sometimes causes the Window
// to receive the wrong size (#2498). To circumvent that, we keep tabs on the
// size of the window while in the normal state (not maximized, minimized or
Expand Down
78 changes: 72 additions & 6 deletions atom/browser/native_window_views_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
#include "atom/browser/browser.h"
#include "atom/browser/native_window_views.h"
#include "atom/common/atom_constants.h"
#include "atom/browser/ui/views/root_view.h"
#include "content/public/browser/browser_accessibility_state.h"
#include "ui/base/win/accessibility_misc_utils.h"
#include "ui/display/win/screen_win.h"
#include "ui/gfx/geometry/insets.h"

// Must be included after other Windows headers.
#include <UIAutomationCoreApi.h>
Expand Down Expand Up @@ -183,6 +186,45 @@ bool NativeWindowViews::PreHandleMSG(UINT message,

return false;
}
case WM_GETMINMAXINFO: {
WINDOWPLACEMENT wp;
wp.length = sizeof(WINDOWPLACEMENT);

// We do this to work around a Windows bug, where the minimized Window
// would report that the closest display to it is not the one that it was
// previously on (but the leftmost one instead). We restore the position
// of the window during the restore operation, this way chromium can
// use the proper display to calculate the scale factor to use.
if(!last_normal_placement_bounds_.IsEmpty() && GetWindowPlacement(GetAcceleratedWidget(), &wp)) {
last_normal_placement_bounds_.set_size(gfx::Size(0,0));
wp.rcNormalPosition = last_normal_placement_bounds_.ToRECT();
SetWindowPlacement(GetAcceleratedWidget(), &wp);

last_normal_placement_bounds_ = gfx::Rect();
}

return false;
}
case WM_NCCALCSIZE: {
if (!has_frame() && w_param == TRUE) {
NCCALCSIZE_PARAMS* params = (NCCALCSIZE_PARAMS*)l_param;
RECT PROPOSED = params->rgrc[0];
RECT BEFORE = params->rgrc[1];

// We need to call the default to have cascade and tile windows
// working (https://github.com/rossy/borderless-window/blob/master/borderless-window.c#L239),
// but we need to provide the proposed original value as suggested in
// https://blogs.msdn.microsoft.com/wpfsdk/2008/09/08/custom-window-chrome-in-wpf/
DefWindowProcW(GetAcceleratedWidget(), WM_NCCALCSIZE, w_param, l_param);

params->rgrc[0] = PROPOSED;
params->rgrc[1] = BEFORE;

return true;
} else {
return false;
}
}
case WM_COMMAND:
// Handle thumbar button click message.
if (HIWORD(w_param) == THBN_CLICKED)
Expand Down Expand Up @@ -258,15 +300,40 @@ void NativeWindowViews::HandleSizeEvent(WPARAM w_param, LPARAM l_param) {
// Here we handle the WM_SIZE event in order to figure out what is the current
// window state and notify the user accordingly.
switch (w_param) {
case SIZE_MAXIMIZED:
case SIZE_MAXIMIZED: {
// Frameless maximized windows are size compensated by Windows for a
// border that's not actually there, so we must conter-compensate.
// https://blogs.msdn.microsoft.com/wpfsdk/2008/09/08/custom-window-chrome-in-wpf/
if (!has_frame()) {
float scale_factor = display::win::ScreenWin::GetScaleFactorForHWND(
GetAcceleratedWidget());

int border =
GetSystemMetrics(SM_CXFRAME) + GetSystemMetrics(SM_CXPADDEDBORDER);
if (!thick_frame_) {
border -= GetSystemMetrics(SM_CXBORDER);
}
root_view_->SetInsets(gfx::Insets(border).Scale(1.0f / scale_factor));
}

last_window_state_ = ui::SHOW_STATE_MAXIMIZED;
if (consecutive_moves_) {
last_normal_bounds_ = last_normal_bounds_before_move_;
}

NotifyWindowMaximize();
break;
}
case SIZE_MINIMIZED:
last_window_state_ = ui::SHOW_STATE_MINIMIZED;

WINDOWPLACEMENT wp;
wp.length = sizeof(WINDOWPLACEMENT);

if(GetWindowPlacement(GetAcceleratedWidget(), &wp)) {
last_normal_placement_bounds_ = gfx::Rect(wp.rcNormalPosition);
}

NotifyWindowMinimize();
break;
case SIZE_RESTORED:
Expand All @@ -278,10 +345,7 @@ void NativeWindowViews::HandleSizeEvent(WPARAM w_param, LPARAM l_param) {
switch (last_window_state_) {
case ui::SHOW_STATE_MAXIMIZED:
last_window_state_ = ui::SHOW_STATE_NORMAL;

// Don't force out last known bounds onto the window as Windows
// actually gets these correct

root_view_->SetInsets(gfx::Insets(0));
NotifyWindowUnmaximize();
break;
case ui::SHOW_STATE_MINIMIZED:
Expand All @@ -293,7 +357,9 @@ void NativeWindowViews::HandleSizeEvent(WPARAM w_param, LPARAM l_param) {

// When the window is restored we resize it to the previous known
// normal size.
SetBounds(last_normal_bounds_, false);
if (has_frame()) {
SetBounds(last_normal_bounds_, false);
}

NotifyWindowRestore();
}
Expand Down
19 changes: 15 additions & 4 deletions atom/browser/ui/views/root_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,18 @@ void RootView::Layout() {
return;

const auto menu_bar_bounds =
menu_bar_visible_ ? gfx::Rect(0, 0, size().width(), kMenuBarHeight)
: gfx::Rect();
menu_bar_visible_
? gfx::Rect(insets_.left(), insets_.top(),
size().width() - insets_.width(), kMenuBarHeight)
: gfx::Rect();
if (menu_bar_)
menu_bar_->SetBoundsRect(menu_bar_bounds);

window_->content_view()->SetBoundsRect(
gfx::Rect(0, menu_bar_bounds.height(), size().width(),
size().height() - menu_bar_bounds.height()));
gfx::Rect(insets_.left(),
menu_bar_visible_ ? menu_bar_bounds.bottom() : insets_.top(),
size().width() - insets_.width(),
size().height() - menu_bar_bounds.height() - insets_.height()));
}

gfx::Size RootView::GetMinimumSize() const {
Expand Down Expand Up @@ -217,4 +221,11 @@ void RootView::UnregisterAcceleratorsWithFocusManager() {
focus_manager->UnregisterAccelerators(this);
}

void RootView::SetInsets(const gfx::Insets& insets) {
if (insets != insets_) {
insets_ = insets;
Layout();
}
}

} // namespace atom
5 changes: 5 additions & 0 deletions atom/browser/ui/views/root_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>

#include "atom/browser/ui/accelerator_util.h"
#include "ui/gfx/geometry/insets.h"
#include "ui/views/view.h"
#include "ui/views/view_tracker.h"

Expand Down Expand Up @@ -39,6 +40,8 @@ class RootView : public views::View {
// Register/Unregister accelerators supported by the menu model.
void RegisterAcceleratorsWithFocusManager(AtomMenuModel* menu_model);
void UnregisterAcceleratorsWithFocusManager();
void SetInsets(const gfx::Insets& insets);
gfx::Insets insets() const { return insets_; }

// views::View:
void Layout() override;
Expand All @@ -56,6 +59,8 @@ class RootView : public views::View {
bool menu_bar_visible_ = false;
bool menu_bar_alt_pressed_ = false;

gfx::Insets insets_;

// Map from accelerator to menu item's command id.
accelerator_util::AcceleratorTable accelerator_table_;

Expand Down