From b75d01690fce9be7115060b22db33398d1536928 Mon Sep 17 00:00:00 2001 From: Lambros Lambrou Date: Tue, 2 Nov 2021 23:25:15 -0700 Subject: [PATCH] Fix a couple of monitor-offset bugs in ScreenCapturerX11. UpdateMonitors() crops the selected RANDR monitor to the root window, in case X returns a monitor that lies outside it. But it wasn't enough. SelectSource() alters the selection directly and doesn't call UpdateMonitors(), so it also needs to crop. This fixes the case where a virtual monitor is added, the screen resolution is reduced, then the new monitor is selected (which now extends outside the reduced screen size). This CL also fixes an issue where the ScreenCapturerHelper would sometimes expand a damage-region outside the DesktopFrame boundary. This occurred because the helper's size was set to the full pixel-buffer, so it didn't crop correctly to the monitor's rect. This CL sets the helper's correct size, and removes some unnecessary cropping now that the helper will do it correctly. Bug: chromium:1266179 Change-Id: I8eb8f3302701be4f393934c0899f41def3512853 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/237120 Commit-Queue: Joe Downing Reviewed-by: Joe Downing Cr-Commit-Position: refs/heads/main@{#35304} --- .../linux/screen_capturer_x11.cc | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/modules/desktop_capture/linux/screen_capturer_x11.cc b/modules/desktop_capture/linux/screen_capturer_x11.cc index e05f7bfea7..c601a2afb8 100644 --- a/modules/desktop_capture/linux/screen_capturer_x11.cc +++ b/modules/desktop_capture/linux/screen_capturer_x11.cc @@ -325,6 +325,12 @@ bool ScreenCapturerX11::SelectSource(SourceId id) { selected_monitor_name_ = m.name; selected_monitor_rect_ = DesktopRect::MakeXYWH(m.x, m.y, m.width, m.height); + const auto& pixel_buffer_rect = x_server_pixel_buffer_.window_rect(); + if (!pixel_buffer_rect.ContainsRect(selected_monitor_rect_)) { + RTC_LOG(LS_WARNING) + << "Cropping selected monitor rect to fit the pixel-buffer."; + selected_monitor_rect_.IntersectWith(pixel_buffer_rect); + } return true; } } @@ -358,8 +364,10 @@ std::unique_ptr ScreenCapturerX11::CaptureScreen() { RTC_DCHECK(selected_monitor_rect_.top_left().equals(frame->top_left())); // Pass the screen size to the helper, so it can clip the invalid region if it - // expands that region to a grid. - helper_.set_size_most_recent(x_server_pixel_buffer_.window_size()); + // expands that region to a grid. Note that the helper operates in the + // DesktopFrame coordinate system where the top-left pixel is (0, 0), even for + // a monitor with non-zero offset relative to `x_server_pixel_buffer_`. + helper_.set_size_most_recent(frame->size()); // In the DAMAGE case, ensure the frame is up-to-date with the previous frame // if any. If there isn't a previous frame, that means a screen-resolution @@ -382,15 +390,13 @@ std::unique_ptr ScreenCapturerX11::CaptureScreen() { auto damage_rect = DesktopRect::MakeXYWH(rects[i].x, rects[i].y, rects[i].width, rects[i].height); - // Damage regions are in the same coordinate-system as - // ```selected_monitor_rect_```, but may fall outside of it. - damage_rect.IntersectWith(selected_monitor_rect_); - if (!damage_rect.is_empty()) { - // Convert to DesktopFrame coordinates where the top-left is - // always (0, 0), before adding to the frame's update_region. - damage_rect.Translate(-frame->top_left()); - updated_region->AddRect(damage_rect); - } + // Damage-regions are relative to `x_server_pixel_buffer`, so convert the + // region to DesktopFrame coordinates where the top-left is always (0, 0), + // before adding to the frame's updated_region. `helper_` also operates in + // DesktopFrame coordinates, and it will take care of cropping away any + // damage-regions that lie outside the selected monitor. + damage_rect.Translate(-frame->top_left()); + updated_region->AddRect(damage_rect); } XFree(rects); helper_.InvalidateRegion(*updated_region);