From 483a00c80f1360bd9b1860ab2e10c9eb78e9c948 Mon Sep 17 00:00:00 2001 From: "wez@chromium.org" Date: Fri, 16 Mar 2012 21:43:52 +0000 Subject: Use XDamageSubtract to fetch the damaged region from the X server. Previously the damaged area was accumulated based on received XDamage events, and the entire desktop then "repaired" with an XDamageSubtract() call, leading to a race-condition between damage notifications and the repair. This CL registers only for non-empty damage notifications, and atomically fetches and clears the damage region using XDamageSubtract(), to ensure that no damage is missed. BUG=116283,117485 TEST=Change the resolution of a Chromoting Me2Me host between two sizes and verify that it is always displayed correctly at the client. Review URL: http://codereview.chromium.org/9700084 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127257 0039d316-1c4b-4281-b951-d872f2087c98 --- remoting/host/capturer_linux.cc | 99 +++++++++++++++++++++++++---------------- 1 file changed, 61 insertions(+), 38 deletions(-) (limited to 'remoting') diff --git a/remoting/host/capturer_linux.cc b/remoting/host/capturer_linux.cc index 85e03ab..ec7dbf3 100644 --- a/remoting/host/capturer_linux.cc +++ b/remoting/host/capturer_linux.cc @@ -132,6 +132,7 @@ class CapturerLinux : public Capturer { Damage damage_handle_; int damage_event_base_; int damage_error_base_; + XserverRegion damage_region_; // Access to the X Server's pixel buffer. XServerPixelBuffer x_server_pixel_buffer_; @@ -166,9 +167,10 @@ CapturerLinux::CapturerLinux() gc_(NULL), root_window_(BadValue), use_damage_(false), - damage_handle_(BadValue), + damage_handle_(0), damage_event_base_(-1), damage_error_base_(-1), + damage_region_(0), current_buffer_(0), pixel_format_(media::VideoFrame::RGB32), last_buffer_(NULL) { @@ -215,25 +217,43 @@ bool CapturerLinux::Init() { } void CapturerLinux::InitXDamage() { - // Setup XDamage to report changes in the damage window. Mark the whole - // window as invalid. - if (XDamageQueryExtension(display_, &damage_event_base_, - &damage_error_base_)) { - damage_handle_ = XDamageCreate(display_, root_window_, - XDamageReportDeltaRectangles); - if (damage_handle_ == BadValue) { - LOG(ERROR) << "Unable to create damage handle."; - } else { - // TODO(lambroslambrou): Disable DAMAGE in situations where it is known - // to fail, such as when Desktop Effects are enabled, with graphics - // drivers (nVidia, ATI) that fail to report DAMAGE notifications - // properly. - use_damage_ = true; - LOG(INFO) << "Using XDamage extension."; - } - } else { - LOG(INFO) << "Server does not support XDamage."; + // Check for XFixes and XDamage extensions. If both are found then use + // XDamage to get explicit notifications of on-screen changes. + int xfixes_event_base; + int xfixes_error_base; + if (!XFixesQueryExtension(display_, &xfixes_event_base, &xfixes_error_base)) { + LOG(INFO) << "X server does not support XFixes."; + return; + } + if (!XDamageQueryExtension(display_, &damage_event_base_, + &damage_error_base_)) { + LOG(INFO) << "X server does not support XDamage."; + return; + } + + // TODO(lambroslambrou): Disable DAMAGE in situations where it is known + // to fail, such as when Desktop Effects are enabled, with graphics + // drivers (nVidia, ATI) that fail to report DAMAGE notifications + // properly. + + // Request notifications every time the screen becomes damaged. + damage_handle_ = XDamageCreate(display_, root_window_, + XDamageReportNonEmpty); + if (!damage_handle_) { + LOG(ERROR) << "Unable to initialize XDamage."; + return; + } + + // Create an XFixes server-side region to collate damage into. + damage_region_ = XFixesCreateRegion(display_, 0, 0); + if (!damage_region_) { + XDamageDestroy(display_, damage_handle_); + LOG(ERROR) << "Unable to create XFixes region."; + return; } + + use_damage_ = true; + LOG(INFO) << "Using XDamage extension."; } void CapturerLinux::ScreenConfigurationChanged() { @@ -294,34 +314,18 @@ void CapturerLinux::ProcessPendingXEvents() { // on XPending because we want to guarantee this terminates. int events_to_process = XPending(display_); XEvent e; - SkRegion invalid_region; for (int i = 0; i < events_to_process; i++) { XNextEvent(display_, &e); if (use_damage_ && (e.type == damage_event_base_ + XDamageNotify)) { XDamageNotifyEvent *event = reinterpret_cast(&e); - - // TODO(hclam): Perform more checks on the rect. - if (event->area.width <= 0 || event->area.height <= 0) - continue; - - SkIRect damage_rect = SkIRect::MakeXYWH(event->area.x, - event->area.y, - event->area.width, - event->area.height); - invalid_region.op(damage_rect, SkRegion::kUnion_Op); - DVLOG(3) << "Damage received for rect at (" - << damage_rect.fLeft << "," << damage_rect.fTop << ") size (" - << damage_rect.width() << "," << damage_rect.height() << ")"; + DCHECK(event->level == XDamageReportNonEmpty); } else if (e.type == ConfigureNotify) { ScreenConfigurationChanged(); - invalid_region.setEmpty(); } else { LOG(WARNING) << "Got unknown event type: " << e.type; } } - - helper_.InvalidateRegion(invalid_region); } CaptureData* CapturerLinux::CaptureFrame() { @@ -348,12 +352,25 @@ CaptureData* CapturerLinux::CaptureFrame() { x_server_pixel_buffer_.Synchronize(); if (use_damage_ && last_buffer_) { + // Atomically fetch and clear the damage region. + XDamageSubtract(display_, damage_handle_, None, damage_region_); + int nRects = 0; + XRectangle bounds; + XRectangle* rects = XFixesFetchRegionAndBounds(display_, damage_region_, + &nRects, &bounds); + for (int i=0; i