From 4fe660785bf6573d859b5384331a4928324ec7ef Mon Sep 17 00:00:00 2001 From: Zijie He Date: Wed, 30 Aug 2017 16:58:14 -0700 Subject: [PATCH] Use GetWindowDrawableRect() instead of GetCroppedWindowRect() in WindowCapturerWin GetCroppingWindowRect() is too generic and easy to be misused. For example, in WindowCapturerWin, it's used to calculate the DesktopRect of the target window, which is inaccurate. See the screenshot in the bug, it wrongly crops borders on Windows 8+. So GetCroppingWindowRect() should be removed eventually, the logic should be placed in CroppingWindowCapturerWin. But since it's still used in the deprecated logic in MouseCursorMonitorWin, I would prefer to remove this function after MouseCursorMonitor improvement has been finished. But before that, WindowCapturerWin should use GetWindowDrawableRect() instead of GetCroppedWindowRect() to avoid the wrongly cropping behavior on Windows 8+. Bug: webrtc:8157 Change-Id: I012b663dced8105623c563dbe55ffcb8d7f17f75 Reviewed-on: https://chromium-review.googlesource.com/642092 Commit-Queue: Zijie He Reviewed-by: Jamie Walch Cr-Commit-Position: refs/heads/master@{#19614} --- .../desktop_capture/desktop_geometry.h | 3 ++ .../win/window_capture_utils.cc | 25 ++++++++----- .../win/window_capture_utils.h | 12 ++++++ .../desktop_capture/window_capturer_win.cc | 37 ++++++++++++++++--- 4 files changed, 61 insertions(+), 16 deletions(-) diff --git a/webrtc/modules/desktop_capture/desktop_geometry.h b/webrtc/modules/desktop_capture/desktop_geometry.h index 71d4173a8b..571243bd2a 100644 --- a/webrtc/modules/desktop_capture/desktop_geometry.h +++ b/webrtc/modules/desktop_capture/desktop_geometry.h @@ -105,6 +105,9 @@ class DesktopRect { int32_t width() const { return right_ - left_; } int32_t height() const { return bottom_ - top_; } + void set_width(int32_t width) { right_ = left_ + width; } + void set_height(int32_t height) { bottom_ = top_ + height; } + DesktopVector top_left() const { return DesktopVector(left_, top_); } DesktopSize size() const { return DesktopSize(width(), height()); } diff --git a/webrtc/modules/desktop_capture/win/window_capture_utils.cc b/webrtc/modules/desktop_capture/win/window_capture_utils.cc index 4241b81b2c..6b5e5733b9 100644 --- a/webrtc/modules/desktop_capture/win/window_capture_utils.cc +++ b/webrtc/modules/desktop_capture/win/window_capture_utils.cc @@ -39,22 +39,15 @@ bool GetCroppedWindowRect(HWND window, } *cropped_rect = window_rect; - WINDOWPLACEMENT window_placement; - window_placement.length = sizeof(window_placement); - if (!::GetWindowPlacement(window, &window_placement)) { + bool is_maximized = false; + if (!IsWindowMaximized(window, &is_maximized)) { return false; } // After Windows8, transparent borders will be added by OS at // left/bottom/right sides of a window. If the cropped window // doesn't remove these borders, the background will be exposed a bit. - // - // On Windows 8.1. or upper, rtc::IsWindows8OrLater(), which uses - // GetVersionEx() may not correctly return the windows version. See - // https://msdn.microsoft.com/en-us/library/windows/desktop/ms724451(v=vs.85).aspx - // So we always prefer to check |window_placement|.showCmd. - if (rtc::IsWindows8OrLater() || - window_placement.showCmd == SW_SHOWMAXIMIZED) { + if (rtc::IsWindows8OrLater() || is_maximized) { const int width = GetSystemMetrics(SM_CXSIZEFRAME); const int height = GetSystemMetrics(SM_CYSIZEFRAME); cropped_rect->Extend(-width, 0, -width, -height); @@ -125,6 +118,18 @@ bool GetDcSize(HDC hdc, DesktopSize* size) { return true; } +bool IsWindowMaximized(HWND window, bool* result) { + WINDOWPLACEMENT placement; + memset(&placement, 0, sizeof(WINDOWPLACEMENT)); + placement.length = sizeof(WINDOWPLACEMENT); + if (!::GetWindowPlacement(window, &placement)) { + return false; + } + + *result = (placement.showCmd == SW_SHOWMAXIMIZED); + return true; +} + AeroChecker::AeroChecker() : dwmapi_library_(nullptr), func_(nullptr) { // Try to load dwmapi.dll dynamically since it is not available on XP. dwmapi_library_ = LoadLibrary(L"dwmapi.dll"); diff --git a/webrtc/modules/desktop_capture/win/window_capture_utils.h b/webrtc/modules/desktop_capture/win/window_capture_utils.h index 4c19d74cea..500d58d387 100644 --- a/webrtc/modules/desktop_capture/win/window_capture_utils.h +++ b/webrtc/modules/desktop_capture/win/window_capture_utils.h @@ -26,6 +26,14 @@ bool GetWindowRect(HWND window, DesktopRect* result); // Returns true if all API calls succeeded. The returned DesktopRect is in // system coordinates, i.e. the primary monitor on the system always starts from // (0, 0). |original_rect| can be nullptr. +// +// TODO(zijiehe): Move this function to CroppingWindowCapturerWin after it has +// been removed from MouseCursorMonitorWin. +// This function should only be used by CroppingWindowCapturerWin. Instead a +// DesktopRect CropWindowRect(const DesktopRect& rect) +// should be added as a utility function to help CroppingWindowCapturerWin and +// WindowCapturerWin to crop out the borders or shadow according to their +// scenarios. But this function is too generic and easy to be misused. bool GetCroppedWindowRect(HWND window, DesktopRect* cropped_rect, DesktopRect* original_rect); @@ -45,6 +53,10 @@ int GetWindowRegionTypeWithBoundary(HWND window, DesktopRect* result); // fail. bool GetDcSize(HDC hdc, DesktopSize* size); +// Retrieves whether the |window| is maximized and stores in |result|. This +// function returns false if native APIs fail. +bool IsWindowMaximized(HWND window, bool* result); + typedef HRESULT (WINAPI *DwmIsCompositionEnabledFunc)(BOOL* enabled); class AeroChecker { public: diff --git a/webrtc/modules/desktop_capture/window_capturer_win.cc b/webrtc/modules/desktop_capture/window_capturer_win.cc index 880a719ab6..77a6ccea65 100644 --- a/webrtc/modules/desktop_capture/window_capturer_win.cc +++ b/webrtc/modules/desktop_capture/window_capturer_win.cc @@ -81,6 +81,34 @@ BOOL CALLBACK WindowsEnumerationHandler(HWND hwnd, LPARAM param) { return TRUE; } +// Retrieves the rectangle of the window rect which is drawable by either OS or +// the owner application. The returned DesktopRect is in system coordinates. +// This function returns false if native APIs fail. +// +// When |window| is maximized, its borders and shadow effect will be ignored by +// OS and leave black. So we prefer to use GetCroppedWindowRect() when capturing +// its content to avoid the black area in the final DesktopFrame. But when the +// window is in normal mode, borders and shadow should be included. +bool GetWindowDrawableRect(HWND window, + DesktopRect* drawable_rect, + DesktopRect* original_rect) { + if (!GetWindowRect(window, original_rect)) { + return false; + } + + bool is_maximized = false; + if (!IsWindowMaximized(window, &is_maximized)) { + return false; + } + + if (is_maximized) { + return GetCroppedWindowRect( + window, drawable_rect, /* original_rect */ nullptr); + } + *drawable_rect = *original_rect; + return true; +} + class WindowCapturerWin : public DesktopCapturer { public: WindowCapturerWin(); @@ -183,13 +211,10 @@ void WindowCapturerWin::CaptureFrame() { return; } - DesktopRect original_rect; DesktopRect cropped_rect; - // TODO(zijiehe): GetCroppedWindowRect() is not accurate, Windows won't draw - // the content below the |window_| with PrintWindow() or BitBlt(). See bug - // https://bugs.chromium.org/p/webrtc/issues/detail?id=8157. - if (!GetCroppedWindowRect(window_, &cropped_rect, &original_rect)) { - LOG(LS_WARNING) << "Failed to get window info: " << GetLastError(); + DesktopRect original_rect; + if (!GetWindowDrawableRect(window_, &cropped_rect, &original_rect)) { + LOG(LS_WARNING) << "Failed to get drawable window area: " << GetLastError(); callback_->OnCaptureResult(Result::ERROR_TEMPORARY, nullptr); return; }