Bug Fix: Mac Retina Screen Capture's Mouse Cursor Too Small

webrtc/modules/desktop_capture/mouse_cursor_monitor_mac.mm

Index: webrtc/modules/desktop_capture/mouse_cursor_monitor_mac.mm
diff --git a/webrtc/modules/desktop_capture/mouse_cursor_monitor_mac.mm b/webrtc/modules/desktop_capture/mouse_cursor_monitor_mac.mm
index 5d6a9b0089c6ddbff18ce7db4b6083b65eab70ba..e07abdcef2c05b971de83279c0785bd5089bd1a2 100644
--- a/webrtc/modules/desktop_capture/mouse_cursor_monitor_mac.mm
+++ b/webrtc/modules/desktop_capture/mouse_cursor_monitor_mac.mm
@@ -30,6 +30,21 @@
 
 namespace webrtc {
 
+namespace {
+NSImage* PaintInCurrentContext(NSImage* source, NSSize new_size) {

[Sergey Ulanov, 2016/09/20 22:53:22]

Add a comment to explain why we need this function.

[qiangchen, 2016/09/21 16:38:31]

Done.

+  NSImage* new_image = [[[NSImage alloc] initWithSize:new_size] autorelease];
+  [source setSize:new_size];
+  [new_image lockFocus];
+  NSRect frame = NSMakeRect(0, 0, new_size.width, new_size.height);
+  [source drawInRect:frame
+            fromRect:frame
+           operation:NSCompositeCopy
+            fraction:1.0];
+  [new_image unlockFocus];
+  return new_image;
+}
+}

[Sergey Ulanov, 2016/09/20 22:53:22]

// namespace

[qiangchen, 2016/09/21 16:38:32]

Done.

+
 class MouseCursorMonitorMac : public MouseCursorMonitor {
  public:
   MouseCursorMonitorMac(const DesktopCaptureOptions& options,
@@ -47,7 +62,7 @@ class MouseCursorMonitorMac : public MouseCursorMonitor {
   void DisplaysReconfigured(CGDirectDisplayID display,
                             CGDisplayChangeSummaryFlags flags);
 
-  void CaptureImage();
+  void CaptureImage(float scale);
 
   rtc::scoped_refptr<DesktopConfigurationMonitor> configuration_monitor_;
   CGWindowID window_id_;
@@ -91,11 +106,6 @@ void MouseCursorMonitorMac::Init(Callback* callback, Mode mode) {
 void MouseCursorMonitorMac::Capture() {
   assert(callback_);
 
-  CaptureImage();
-
-  if (mode_ != SHAPE_AND_POSITION)
-    return;
-
   CursorState state = INSIDE;
 
   CGEventRef event = CGEventCreate(NULL);
@@ -113,12 +123,18 @@ void MouseCursorMonitorMac::Capture() {
   // Find the dpi to physical pixel scale for the screen where the mouse cursor
   // is.
   for (MacDisplayConfigurations::iterator it = configuration.displays.begin();
-      it != configuration.displays.end(); ++it) {
+       it != configuration.displays.end(); ++it) {
     if (it->bounds.Contains(position)) {
       scale = it->dip_to_pixel_scale;
       break;
     }
   }
+
+  CaptureImage(scale);
+
+  if (mode_ != SHAPE_AND_POSITION)
+    return;
+
   // If we are capturing cursor for a specific window then we need to figure out
   // if the current mouse position is covered by another window and also adjust
   // |position| to make it relative to the window origin.
@@ -228,24 +244,32 @@ void MouseCursorMonitorMac::Capture() {
   callback_->OnMouseCursorPosition(state, position);
 }
 
-void MouseCursorMonitorMac::CaptureImage() {
+void MouseCursorMonitorMac::CaptureImage(float scale) {
   NSCursor* nscursor = [NSCursor currentSystemCursor];
 
   NSImage* nsimage = [nscursor image];
-  NSSize nssize = [nsimage size];
-  DesktopSize size(nssize.width, nssize.height);
+  NSSize nssize = [nsimage size];  // DIP size
+
+  // For retina screen, we need to paint the cursor in current graphic context
+  // to get retina representation.
+  if (scale != 1.0)
+    nsimage = PaintInCurrentContext(nsimage, nssize);

[Sergey Ulanov, 2016/09/20 22:53:22]

Do we need to nssize here? the function can just call [nscursor image]. Also
It's not clear how this accomplishes anything: you pass current size as new
size.

[qiangchen, 2016/09/21 16:38:31]

Done.

Originally, I thought we need to plug in nssize*2 to scale it. By testing, nope.

+
+  DesktopSize size(round(nssize.width * scale),
+                   round(nssize.height * scale));  // Pixel size
   NSPoint nshotspot = [nscursor hotSpot];
   DesktopVector hotspot(
-      std::max(0, std::min(size.width(), static_cast<int>(nshotspot.x))),
-      std::max(0, std::min(size.height(), static_cast<int>(nshotspot.y))));
+      std::max(0,
+               std::min(size.width(), static_cast<int>(nshotspot.x * scale))),
+      std::max(0,
+               std::min(size.height(), static_cast<int>(nshotspot.y * scale))));
   CGImageRef cg_image =
       [nsimage CGImageForProposedRect:NULL context:nil hints:nil];
   if (!cg_image)
     return;
 
   if (CGImageGetBitsPerPixel(cg_image) != DesktopFrame::kBytesPerPixel * 8 ||
-      CGImageGetBytesPerRow(cg_image) !=
-          static_cast<size_t>(DesktopFrame::kBytesPerPixel * size.width()) ||
+      CGImageGetWidth(cg_image) != static_cast<size_t>(size.width()) ||
       CGImageGetBitsPerComponent(cg_image) != 8) {
     return;
   }
@@ -272,8 +296,17 @@ void MouseCursorMonitorMac::CaptureImage() {
   // the client.
   std::unique_ptr<DesktopFrame> image(
       new BasicDesktopFrame(DesktopSize(size.width(), size.height())));
-  memcpy(image->data(), src_data,
-         size.width() * size.height() * DesktopFrame::kBytesPerPixel);
+
+  int src_stride = CGImageGetBytesPerRow(cg_image);
+  int dst_stride = size.width() * DesktopFrame::kBytesPerPixel;
+  if (src_stride == dst_stride) {
+    memcpy(image->data(), src_data, size.height() * src_stride);
+  } else {
+    for (int y = 0; y < size.height(); y++) {

[Sergey Ulanov, 2016/09/20 22:53:22]

Use CopyPixelsFrom() here?

[qiangchen, 2016/09/21 16:38:32]

Done.

+      memcpy(image->data() + y * dst_stride, src_data + y * src_stride,
+             dst_stride);
+    }
+  }
 
   CFRelease(image_data_ref);