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

webrtc/modules/desktop_capture/mouse_cursor_monitor_mac.mm

 /*
  *  Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 12 matching lines @@
 #include "webrtc/modules/desktop_capture/desktop_capture_options.h"
 #include "webrtc/modules/desktop_capture/desktop_frame.h"
 #include "webrtc/modules/desktop_capture/mac/desktop_configuration.h"
 #include "webrtc/modules/desktop_capture/mac/desktop_configuration_monitor.h"
 #include "webrtc/modules/desktop_capture/mac/full_screen_chrome_window_detector.h"
 #include "webrtc/modules/desktop_capture/mouse_cursor.h"
 
 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 CreateScaledCGImage(CGImageRef image, int width, int height) {
+  // create context, keeping original image properties
+  CGColorSpaceRef colorspace = CGImageGetColorSpace(image);
+  CGContextRef context =
+      CGBitmapContextCreate(NULL,

[Do not use (sergeyu), 2017/06/01 00:22:00]

s/NULL/nullptr/

[braveyao1, 2017/06/01 17:50:14]

Done.

+                            width,

[Do not use (sergeyu), 2017/06/01 00:22:00]

Please run 'git cl format'. It should format this code differently, particularly
it would put multiple arguments per line.

[braveyao1, 2017/06/01 17:50:14]

Actually I tried to run "git cl format" before filing this cl. It's weird that
it formats the mm file into 100 char line length. So I have to give it up. IIRC,
it works under chromium though...
And here one argument per line is from that cl format result.

[Do not use (sergeyu), 2017/06/01 18:08:08]

That's by design. Style guide for ObjC allows 100-char lines (see
https://google.github.io/styleguide/objcguide.xml#Line_Length) and clang-format
is configured to use 100-char lines for ObjC (see .clang-format in the root of
the webrtc repo).

[braveyao1, 2017/06/01 18:17:56]

Done.

I think 'git cl format' won't be wrong. :) But
https://chromium.googlesource.com/chromium/src/+/master/styleguide/objective-...
says it should be 80 char. So confused...

+                            height,
+                            CGImageGetBitsPerComponent(image),
+                            width * 4,

[Do not use (sergeyu), 2017/06/01 00:22:00]

width * DesktopFrame::kBytesPerPixel;

[braveyao1, 2017/06/01 17:50:14]

Done.

+                            colorspace,
+                            CGImageGetBitmapInfo(image));
+
+  if (context == NULL) return nil;

[Do not use (sergeyu), 2017/06/01 00:21:59]

if (!context)
  return nil;

[braveyao1, 2017/06/01 17:50:14]

Done.

+
+  // draw image to context (resizing it)

[Do not use (sergeyu), 2017/06/01 00:21:59]

Please reformat this as follows:
  // Draw image to the context, resizing it.
Note capital D and period at the end.

[braveyao1, 2017/06/01 17:50:15]

Done.

+  CGContextDrawImage(context, CGRectMake(0, 0, width, height), image);
+  // extract resulting image from context

[Do not use (sergeyu), 2017/06/01 00:21:59]

// Extract resulting image from context.

[braveyao1, 2017/06/01 17:50:14]

Done.

+  CGImageRef imgRef = CGBitmapContextCreateImage(context);
+  CGContextRelease(context);
+
+  return imgRef;
 }
 }  // namespace
 
 class MouseCursorMonitorMac : public MouseCursorMonitor {
  public:
   MouseCursorMonitorMac(const DesktopCaptureOptions& options,
                         CGWindowID window_id,
                         ScreenId screen_id);
   ~MouseCursorMonitorMac() override;
 
   void Init(Callback* callback, Mode mode) override;
   void Capture() override;
 
  private:
   static void DisplaysReconfiguredCallback(CGDirectDisplayID display,
                                            CGDisplayChangeSummaryFlags flags,
                                            void *user_parameter);
   void DisplaysReconfigured(CGDirectDisplayID display,
                             CGDisplayChangeSummaryFlags flags);
 
   void CaptureImage(float scale);
 
   rtc::scoped_refptr<DesktopConfigurationMonitor> configuration_monitor_;
   CGWindowID window_id_;
   ScreenId screen_id_;
   Callback* callback_;
   Mode mode_;
-  std::unique_ptr<MouseCursor> last_cursor_;
+  __strong NSImage* last_cursor_;
   rtc::scoped_refptr<FullScreenChromeWindowDetector>
       full_screen_chrome_window_detector_;
 };
 
 MouseCursorMonitorMac::MouseCursorMonitorMac(
     const DesktopCaptureOptions& options,
     CGWindowID window_id,
     ScreenId screen_id)
     : configuration_monitor_(options.configuration_monitor()),
       window_id_(window_id),
@@ skipping 160 matching lines @@
   }
   callback_->OnMouseCursorPosition(state, position);
 }
 
 void MouseCursorMonitorMac::CaptureImage(float scale) {
   NSCursor* nscursor = [NSCursor currentSystemCursor];
 
   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]])

[Do not use (sergeyu), 2017/06/01 00:21:59]

Add a comment, to make it clear clear what happens here.

[braveyao1, 2017/06/01 17:50:14]

Done.

+    return;
+  last_cursor_ = nsimage;
 
   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 * 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;
 
+  // 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.
+  CGImageRef scaled_cg_image = nil;
+  if (CGImageGetWidth(cg_image) != static_cast<size_t>(size.width())) {
+    scaled_cg_image =
+        CreateScaledCGImage(cg_image, size.width(), size.height());
+    if (scaled_cg_image != nil) {
+      cg_image = scaled_cg_image;
+    }
+  }
   if (CGImageGetBitsPerPixel(cg_image) != DesktopFrame::kBytesPerPixel * 8 ||
       CGImageGetWidth(cg_image) != static_cast<size_t>(size.width()) ||
       CGImageGetBitsPerComponent(cg_image) != 8) {
+    if (scaled_cg_image != nil) CGImageRelease(scaled_cg_image);
     return;
   }
 
   CGDataProviderRef provider = CGImageGetDataProvider(cg_image);
   CFDataRef image_data_ref = CGDataProviderCopyData(provider);
-  if (image_data_ref == NULL)
+  if (image_data_ref == NULL) {
+    if (scaled_cg_image != nil) CGImageRelease(scaled_cg_image);
     return;
+  }
 
   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(
       new BasicDesktopFrame(DesktopSize(size.width(), size.height())));
 
   int src_stride = CGImageGetBytesPerRow(cg_image);
   image->CopyPixelsFrom(src_data, src_stride, DesktopRect::MakeSize(size));
 
   CFRelease(image_data_ref);
+  if (scaled_cg_image != nil) CGImageRelease(scaled_cg_image);
 
   std::unique_ptr<MouseCursor> cursor(
       new MouseCursor(image.release(), hotspot));
-  last_cursor_.reset(MouseCursor::CopyOf(*cursor));
 
   callback_->OnMouseCursor(cursor.release());
 }
 
 MouseCursorMonitor* MouseCursorMonitor::CreateForWindow(
     const DesktopCaptureOptions& options, WindowId window) {
   return new MouseCursorMonitorMac(options, window, kInvalidScreenId);
 }
 
+// ScreenCapture already contains current cursor in the captured frame on OSX.

[Do not use (sergeyu), 2017/06/01 00:21:59]

This is not the right place to fix this. This class is used in chromoting (see
https://codesearch.chromium.org/chromium/src/remoting/host/mouse_cursor_monit...
) and we want to keep it working there.

I suggest adding a separate ifdef in
content/browser/media/capture/desktop_capture_device.cc

[braveyao1, 2017/06/01 17:50:14]

OK. Then a separated cl is needed. This is under WebRTC depository.
BTW: it seems that mouse_cursor_monitor_proxy.cc can handle NULL
MouseCursorMonitor. An why we need to draw the cursor again in chromoting?

 MouseCursorMonitor* MouseCursorMonitor::CreateForScreen(
     const DesktopCaptureOptions& options,
     ScreenId screen) {
-  return new MouseCursorMonitorMac(options, kCGNullWindowID, screen);
+  return nil;
 }
 
 }  // namespace webrtc