Fix ScreenCapturerMac handling of secondary display configurations.

- Fix bugs in translating capture regions into display coordinates.
- Fix MacDeskopConfiguration to support inverse-Cartesian coordinates.

BUG=175261,174090


Review URL: https://chromiumcodereview.appspot.com/12221103

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@181885 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 46 insertions, 24 deletions

diff --git a/media/video/capture/screen/mac/desktop_configuration.h b/media/video/capture/screen/mac/desktop_configuration.h
index c3c65b3..3d03f39 100644
--- a/media/video/capture/screen/mac/desktop_configuration.h
+++ b/media/video/capture/screen/mac/desktop_configuration.h
@@ -20,10 +20,6 @@ namespace media {
 struct MEDIA_EXPORT MacDisplayConfiguration {
   MacDisplayConfiguration();
 
-  // Returns the current configuration of the specified display.
-  MEDIA_EXPORT static MacDisplayConfiguration ForDisplay(
-      CGDirectDisplayID display_id);
-
   // Cocoa identifier for this display.
   CGDirectDisplayID id;
 
@@ -41,11 +37,16 @@ typedef std::vector<MacDisplayConfiguration> MacDisplayConfigurations;
 
 // Describes the configuration of the whole desktop.
 struct MEDIA_EXPORT MacDesktopConfiguration {
+  // Used to request bottom-up or top-down coordinates.
+  enum Origin { BottomLeftOrigin, TopLeftOrigin };
+
   MacDesktopConfiguration();
   ~MacDesktopConfiguration();
 
-  // Returns the current configuration of the desktop.
-  MEDIA_EXPORT static MacDesktopConfiguration GetCurrent();
+  // Returns the desktop & display configurations in Cocoa-style "bottom-up"
+  // (the origin is the bottom-left of the primary monitor, and coordinates
+  // increase as you move up the screen) or Carbon-style "top-down" coordinates.
+  MEDIA_EXPORT static MacDesktopConfiguration GetCurrent(Origin origin);
 
   // Bounds of the desktop in Density-Independent Pixels (DIPs).
   SkIRect bounds;
diff --git a/media/video/capture/screen/mac/desktop_configuration.mm b/media/video/capture/screen/mac/desktop_configuration.mm
index 064c4ef..fbece48 100644
--- a/media/video/capture/screen/mac/desktop_configuration.mm
+++ b/media/video/capture/screen/mac/desktop_configuration.mm
@@ -21,20 +21,23 @@
 
 namespace media {
 
-MacDisplayConfiguration::MacDisplayConfiguration()
-  : id(0),
-    bounds(SkIRect::MakeEmpty()),
-    pixel_bounds(SkIRect::MakeEmpty()),
-    dip_to_pixel_scale(1.0f) {
-}
+namespace {
 
-static SkIRect NSRectToSkIRect(const NSRect& ns_rect) {
+SkIRect NSRectToSkIRect(const NSRect& ns_rect) {
   SkIRect result;
   gfx::CGRectToSkRect(NSRectToCGRect(ns_rect)).roundOut(&result);
   return result;
 }
 
-static MacDisplayConfiguration GetConfigurationForScreen(NSScreen* screen) {
+// Inverts the position of |rect| from bottom-up coordinates to top-down,
+// relative to |bounds|.
+void InvertRectYOrigin(const SkIRect& bounds, SkIRect* rect) {
+  DCHECK_EQ(0, bounds.top());
+  rect->setXYWH(rect->x(), bounds.bottom() - rect->bottom(),
+                rect->width(), rect->height());
+}
+
+MacDisplayConfiguration GetConfigurationForScreen(NSScreen* screen) {
   MacDisplayConfiguration display_config;
 
   // Fetch the NSScreenNumber, which is also the CGDirectDisplayID.
@@ -60,17 +63,26 @@ static MacDisplayConfiguration GetConfigurationForScreen(NSScreen* screen) {
   return display_config;
 }
 
+} // namespace
+
+MacDisplayConfiguration::MacDisplayConfiguration()
+    : id(0),
+      bounds(SkIRect::MakeEmpty()),
+      pixel_bounds(SkIRect::MakeEmpty()),
+      dip_to_pixel_scale(1.0f) {
+}
+
 MacDesktopConfiguration::MacDesktopConfiguration()
-  : bounds(SkIRect::MakeEmpty()),
-    pixel_bounds(SkIRect::MakeEmpty()),
-    dip_to_pixel_scale(1.0f) {
+    : bounds(SkIRect::MakeEmpty()),
+      pixel_bounds(SkIRect::MakeEmpty()),
+      dip_to_pixel_scale(1.0f) {
 }
 
 MacDesktopConfiguration::~MacDesktopConfiguration() {
 }
 
 // static
-MacDesktopConfiguration MacDesktopConfiguration::GetCurrent() {
+MacDesktopConfiguration MacDesktopConfiguration::GetCurrent(Origin origin) {
   MacDesktopConfiguration desktop_config;
 
   NSArray* screens = [NSScreen screens];
@@ -92,6 +104,16 @@ MacDesktopConfiguration MacDesktopConfiguration::GetCurrent() {
       continue;
     }
 
+    // Cocoa uses bottom-up coordinates, so if the caller wants top-down then
+    // we need to invert the positions of secondary monitors relative to the
+    // primary one (the primary monitor's position is (0,0) in both systems).
+    if (i > 0 && origin == TopLeftOrigin) {
+      InvertRectYOrigin(desktop_config.displays[0].bounds,
+                        &display_config.bounds);
+      InvertRectYOrigin(desktop_config.displays[0].pixel_bounds,
+                        &display_config.pixel_bounds);
+    }
+
     // Add the display to the configuration.
     desktop_config.displays.push_back(display_config);
 
diff --git a/media/video/capture/screen/screen_capturer_mac.mm b/media/video/capture/screen/screen_capturer_mac.mm
index 27aea6c..a1d5b07 100644
--- a/media/video/capture/screen/screen_capturer_mac.mm
+++ b/media/video/capture/screen/screen_capturer_mac.mm
@@ -586,8 +586,7 @@ void ScreenCapturerMac::CgBlitPreLion(const ScreenCaptureFrame& buffer,
       continue;
 
     // Translate the region to be copied into display-relative coordinates.
-    copy_region.translate(-desktop_config_.pixel_bounds.left(),
-                          -desktop_config_.pixel_bounds.top());
+    copy_region.translate(-display_bounds.left(), -display_bounds.top());
 
     // Calculate where in the output buffer the display's origin is.
     uint8* out_ptr = buffer.pixels() +
@@ -607,7 +606,7 @@ void ScreenCapturerMac::CgBlitPreLion(const ScreenCaptureFrame& buffer,
 }
 
 void ScreenCapturerMac::CgBlitPostLion(const ScreenCaptureFrame& buffer,
-                                           const SkRegion& region) {
+                                       const SkRegion& region) {
   const int buffer_height = buffer.dimensions().height();
 
   // Copy the entire contents of the previous capture buffer, to capture over.
@@ -633,8 +632,7 @@ void ScreenCapturerMac::CgBlitPostLion(const ScreenCaptureFrame& buffer,
       continue;
 
     // Translate the region to be copied into display-relative coordinates.
-    copy_region.translate(-desktop_config_.pixel_bounds.left(),
-                          -desktop_config_.pixel_bounds.top());
+    copy_region.translate(-display_bounds.left(), -display_bounds.top());
 
     // Create an image containing a snapshot of the display.
     base::mac::ScopedCFTypeRef<CGImageRef> image(
@@ -678,7 +676,8 @@ void ScreenCapturerMac::ScreenConfigurationChanged() {
   helper_.ClearInvalidRegion();
 
   // Refresh the cached desktop configuration.
-  desktop_config_ = MacDesktopConfiguration::GetCurrent();
+  desktop_config_ = MacDesktopConfiguration::GetCurrent(
+      MacDesktopConfiguration::TopLeftOrigin);
 
   // Re-mark the entire desktop as dirty.
   helper_.InvalidateScreen(