Fix ScreenCapturerMac handling of secondary configurations.

media/video/capture/screen/mac/desktop_configuration.mm

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/video/capture/screen/mac/desktop_configuration.h"
 
 #include <Cocoa/Cocoa.h>
 
 #include "base/logging.h"
 #include "skia/ext/skia_utils_mac.h"
 
 #if !defined(MAC_OS_X_VERSION_10_7) || \
     MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
 
 @interface NSScreen (LionAPI)
 - (CGFloat)backingScaleFactor;
 - (NSRect)convertRectToBacking:(NSRect)aRect;
 @end
 
 #endif  // 10.7
 
 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 top- to bottom-relative with respect to
+// the specified |reference| coordinates.

[Jamie, 2013/02/11 22:12:44]

It's not clear to me from this comment what this function does. Can you clarify
it please?

[Wez, 2013/02/11 22:56:11]

Done.

+void InvertRectYOrigin(const SkIRect& reference, SkIRect* rect) {
+  rect->setXYWH(rect->x(), reference.bottom() - rect->bottom(),
+                rect->width(), rect->height());
+}
+
+MacDisplayConfiguration GetConfigurationForScreen(NSScreen* screen) {
   MacDisplayConfiguration display_config;
 
   // Fetch the NSScreenNumber, which is also the CGDirectDisplayID.
   NSDictionary* device_description = [screen deviceDescription];
   display_config.id = static_cast<CGDirectDisplayID>(
       [[device_description objectForKey:@"NSScreenNumber"] intValue]);
 
   // Determine the display's logical & physical dimensions.
   NSRect ns_bounds = [screen frame];
   display_config.bounds = NSRectToSkIRect(ns_bounds);
 
   // If the host is running Mac OS X 10.7+ or later, query the scaling factor
   // between logical and physical (aka "backing") pixels, otherwise assume 1:1.
   if ([screen respondsToSelector:@selector(backingScaleFactor)] &&
-      [screen respondsToSelector:@selector(convertRectToBacking:)]) {
+    [screen respondsToSelector:@selector(convertRectToBacking:)]) {

[Sergey Ulanov, 2013/02/11 22:57:20]

nit: this line was indented correctly.

[Wez, 2013/02/11 23:12:03]

Done.

     display_config.dip_to_pixel_scale = [screen backingScaleFactor];
     NSRect ns_pixel_bounds = [screen convertRectToBacking: ns_bounds];
     display_config.pixel_bounds = NSRectToSkIRect(ns_pixel_bounds);
   } else {
     display_config.pixel_bounds = display_config.bounds;
   }
 
   return display_config;
 }
 
+} // namespace
+
+MacDisplayConfiguration::MacDisplayConfiguration()
+  : id(0),

[Sergey Ulanov, 2013/02/11 22:57:20]

nit: this line should be indented 4 spaces and the 3 following lines - 6.

[Wez, 2013/02/11 23:12:03]

Done.

+    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) {
 }
 
 MacDesktopConfiguration::~MacDesktopConfiguration() {
 }
 
 // static
-MacDesktopConfiguration MacDesktopConfiguration::GetCurrent() {
+MacDesktopConfiguration MacDesktopConfiguration::GetCurrent(Origin origin) {
   MacDesktopConfiguration desktop_config;
 
   NSArray* screens = [NSScreen screens];
   CHECK(screens != NULL);
 
   // Iterator over the monitors, adding the primary monitor and monitors whose
   // DPI match that of the primary monitor.
   for (NSUInteger i = 0; i < [screens count]; ++i) {
     MacDisplayConfiguration display_config
         = GetConfigurationForScreen([screens objectAtIndex: i]);
 
     // Handling mixed-DPI is hard, so we only return displays that match the
     // "primary" display's DPI.  The primary display is always the first in the
     // list returned by [NSScreen screens].
     if (i == 0) {
       desktop_config.dip_to_pixel_scale = display_config.dip_to_pixel_scale;
     } else if (desktop_config.dip_to_pixel_scale !=
                display_config.dip_to_pixel_scale) {
       continue;
     }
 
+    // Mac uses Cartesian (bottom-left origin) coordinates, so if the caller
+    // wants inverse Cartesian (top-left origin), secondary monitors need
+    // their coordinates adjusting.

[Jamie, 2013/02/11 22:12:44]

Why only secondary monitors?

[Wez, 2013/02/11 22:56:11]

The coordinate space is primary-monitor-relative.  Clarified.

+    if (i > 0 && origin == TopLeftOrigin) {
+      InvertRectYOrigin(desktop_config.displays[0].bounds,

[Sergey Ulanov, 2013/02/11 22:57:20]

This doesn't look right to me. why invert bounds rect only for the first
display?
Shouldn't this be "display_config.bounds"?

[Wez, 2013/02/11 23:12:03]

The first parameter is the coordinate scheme about which to invert, the second
is the in/out rect to invert.

[Sergey Ulanov, 2013/02/11 23:16:02]

Ah, I see. Thanks for clarifying.

+                        &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);
 
     // Update the desktop bounds to account for this display.
     desktop_config.bounds.join(display_config.bounds);
     desktop_config.pixel_bounds.join(display_config.pixel_bounds);
   }
 
   return desktop_config;
 }
 
 }  // namespace media