desktopCapture: scale the cursor image according to screen scale factor on OSX

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 73d5e028577225796cd38e35e580810cac02ba8b..0c08646a667424d38ef9420e543f931dd048da92 100644
--- a/webrtc/modules/desktop_capture/mouse_cursor_monitor_mac.mm
+++ b/webrtc/modules/desktop_capture/mouse_cursor_monitor_mac.mm
@@ -30,21 +30,28 @@
 namespace webrtc {
 
 namespace {
-// Paint the image, so that we can get a bitmap representation compatible with
-// current context. For example, in the retina display, we are going to get an
-// image with same visual size but underlying pixel size conforms to the retina
-// setting.
-NSImage* PaintInCurrentContext(NSImage* source) {
-  NSSize size = [source size];
-  NSImage* new_image = [[NSImage alloc] initWithSize:size];
-  [new_image lockFocus];
-  NSRect frame = NSMakeRect(0, 0, size.width, size.height);
-  [source drawInRect:frame
-            fromRect:frame
-           operation:NSCompositeCopy
-            fraction:1.0];
-  [new_image unlockFocus];
-  return new_image;
+CGImageRef ResizeCGImage(CGImageRef image, int width, int height) {

[erikchen, 2017/05/26 23:09:04]

please use scoped_cftyperefs everywhere. It prevents memory leaks. :)

[braveyao1, 2017/05/30 17:08:19]

scoped_cftyperefs is not available in WebRTC yet.

+  // create context, keeping original image properties
+  CGColorSpaceRef colorspace = CGImageGetColorSpace(image);
+  CGContextRef context =
+      CGBitmapContextCreate(NULL,
+                            width,
+                            height,
+                            CGImageGetBitsPerComponent(image),
+                            width * 4,
+                            colorspace,
+                            CGImageGetBitmapInfo(image));
+  CGColorSpaceRelease(colorspace);

[erikchen, 2017/05/30 17:27:46]

Please read through
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Me...

and fix your CL to correctly manage references to objects. :)
[e.g. this release is incorrect. Please go through and try to fix your own CL
before asking me to take a look].

[braveyao1, 2017/05/30 21:34:07]

Done.

It's weird that releasing the color space here doesn't cause any problem. I
searched around and found the discussion that the release here is not necessary.

Also found the discussion that the CGImageRef returned by this method should be
manually released. Tried a fix below. Please have a check.

+
+  if (context == NULL) return image;
+
+  // draw image to context (resizing it)
+  CGContextDrawImage(context, CGRectMake(0, 0, width, height), image);
+  // extract resulting image from context
+  CGImageRef imgRef = CGBitmapContextCreateImage(context);
+  CGContextRelease(context);
+
+  return imgRef;
 }
 }  // namespace
 
@@ -72,7 +79,7 @@ class MouseCursorMonitorMac : public MouseCursorMonitor {
   ScreenId screen_id_;
   Callback* callback_;
   Mode mode_;
-  std::unique_ptr<MouseCursor> last_cursor_;
+  NSImage* last_cursor_;

[erikchen, 2017/05/26 23:09:04]

use a scoped_nsobject. You're not retaining this object.

Also, it would be far more efficient to retain the NSCursor and check for
equality there.

[braveyao1, 2017/05/30 17:08:19]

scoped_nsobject is not available in WebRTC yet.

"retain the NSCursor and check for equality" doesn't work. I encountered the
same problem as in [1] and switched to NSImage TIFFRepresentation comparison.

[1]:
https://stackoverflow.com/questions/8198380/how-to-get-current-type-of-mouse-...

   rtc::scoped_refptr<FullScreenChromeWindowDetector>
       full_screen_chrome_window_detector_;
 };
@@ -253,10 +260,9 @@ void MouseCursorMonitorMac::CaptureImage(float scale) {
   NSImage* nsimage = [nscursor image];
   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);
+  if([[nsimage TIFFRepresentation] isEqual: [last_cursor_ TIFFRepresentation]])
+    return;
+  last_cursor_ = nsimage;
 
   DesktopSize size(round(nssize.width * scale),
                    round(nssize.height * scale));  // Pixel size
@@ -271,6 +277,12 @@ void MouseCursorMonitorMac::CaptureImage(float scale) {
   if (!cg_image)
     return;
 
+  // Before 10.12, OSX may report 1X cursor on Retina screen. (See
+  // crbug.com/632995.) After 10.12, OSX may report 2X cursor on non-Retina
+  // screen. (See crbug.com/671436.) So scaling the cursor if needed.
+  if (CGImageGetWidth(cg_image) != static_cast<size_t>(size.width()))
+    cg_image = ResizeCGImage(cg_image, size.width(), size.height());
+
   if (CGImageGetBitsPerPixel(cg_image) != DesktopFrame::kBytesPerPixel * 8 ||
       CGImageGetWidth(cg_image) != static_cast<size_t>(size.width()) ||
       CGImageGetBitsPerComponent(cg_image) != 8) {
@@ -285,16 +297,6 @@ void MouseCursorMonitorMac::CaptureImage(float scale) {
   const uint8_t* src_data =
       reinterpret_cast<const uint8_t*>(CFDataGetBytePtr(image_data_ref));
 
-  // Compare the cursor with the previous one.
-  if (last_cursor_.get() &&
-      last_cursor_->image()->size().equals(size) &&
-      last_cursor_->hotspot().equals(hotspot) &&
-      memcmp(last_cursor_->image()->data(), src_data,
-             last_cursor_->image()->stride() * size.height()) == 0) {
-    CFRelease(image_data_ref);
-    return;
-  }
-
   // Create a MouseCursor that describes the cursor and pass it to
   // the client.
   std::unique_ptr<DesktopFrame> image(
@@ -307,7 +309,6 @@ void MouseCursorMonitorMac::CaptureImage(float scale) {
 
   std::unique_ptr<MouseCursor> cursor(
       new MouseCursor(image.release(), hotspot));
-  last_cursor_.reset(MouseCursor::CopyOf(*cursor));
 
   callback_->OnMouseCursor(cursor.release());
 }
@@ -317,10 +318,11 @@ MouseCursorMonitor* MouseCursorMonitor::CreateForWindow(
   return new MouseCursorMonitorMac(options, window, kInvalidScreenId);
 }
 
+// ScreenCapture already contains current cursor in the captured frame on OSX.
 MouseCursorMonitor* MouseCursorMonitor::CreateForScreen(
     const DesktopCaptureOptions& options,
     ScreenId screen) {
-  return new MouseCursorMonitorMac(options, kCGNullWindowID, screen);
+  return nil;
 }
 
 }  // namespace webrtc