Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1057)

Unified Diff: chrome/browser/ui/views/frame/minimize_button_metrics_win.cc

Issue 2833363002: Add GetCaptionButtonHeightInDIPs() method (Closed)
Patch Set: Made MinimizeButtonMetrics::GetCaptionButtonHeight() static and reverted the rest of the changes Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/views/frame/minimize_button_metrics_win.cc
diff --git a/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc b/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc
index 095f2574c83c9ed91b29b7ec8972c8022cbe5d8d..a07b94a089eb4bfffb74e038399865bd30448059 100644
--- a/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc
+++ b/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc
@@ -52,6 +52,30 @@ MinimizeButtonMetrics::MinimizeButtonMetrics()
MinimizeButtonMetrics::~MinimizeButtonMetrics() {
}
+// static
+int MinimizeButtonMetrics::GetCaptionButtonHeight() {
+ // Caption button height is not affected by DPI settings.
Peter Kasting 2017/04/26 17:19:52 robliao says that in his testing, this isn't corre
+ // GetSystemMetrics(SM_CYSIZE) returns the same value regardless of the DPI
+ // settings of any monitor (28 on Windows 10). This is what needs to be
Peter Kasting 2017/04/26 17:19:52 Rob also notes he gets 22 here, not 28. I would j
+ // passed to a View, which then takes care of the scaling.
Peter Kasting 2017/04/26 17:19:52 Nit: No need for the last sentence here
+
+ // A View drawn to this height may look ~1px off at some DPI settings.
Peter Kasting 2017/04/26 17:19:52 I think "~1" is inaccurate here. Assuming the con
+ // http://crbug.com/668278 tracks pixel-precise positioning for this.
+ // (According to robliao@ and bsep@ the solution is probably
+ // GetSystemMetricsForDpi.)
+
+ // Note that the following methods all return scaled values, which is not
+ // what the caller expects:
+ // DwmGetWindowAttribute(hwnd_, DWMWA_CAPTION_BUTTON_BOUNDS, ...)
+ // SendMessage(hwnd_, WM_GETTITLEBARINFOEX, ...)
+ // display::win::ScreenWin::GetSystemMetricsForHwnd(hwnd_, SM_CYSIZE)
+ // these all return (height * scale factor) e.g. 56 at 200%
+ // display::win::ScreenWin::GetSystemMetricsInDIP(SM_CYSIZE)
+ // returns (height / by primary monitor scale factor) e.g. 22 for 125%
Peter Kasting 2017/04/26 17:19:52 I would omit this comment entirely, as your first
+
+ return GetSystemMetrics(SM_CYSIZE);
+}
+
void MinimizeButtonMetrics::Init(HWND hwnd) {
DCHECK(!hwnd_);
hwnd_ = hwnd;
@@ -123,7 +147,7 @@ int MinimizeButtonMetrics::GetMinimizeButtonOffsetForWindow() const {
TITLEBARINFOEX titlebar_info = {0};
titlebar_info.cbSize = sizeof(TITLEBARINFOEX);
SendMessage(hwnd_, WM_GETTITLEBARINFOEX, 0,
- reinterpret_cast<WPARAM>(&titlebar_info));
+ reinterpret_cast<LPARAM>(&titlebar_info));
// Under DWM WM_GETTITLEBARINFOEX won't return the right thing until after
// WM_NCACTIVATE (maybe it returns classic values?). In an attempt to

Powered by Google App Engine
This is Rietveld 408576698