Linux desktopCapture: fix the cursor position issue in Window sharing

webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc

Index: webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc
diff --git a/webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc b/webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc
index 4870bae7ff0baade0c476f670d074ee36f0b295b..f1894c53b60c51924c5dd29661597a4ec0c483ef 100644
--- a/webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc
+++ b/webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc
@@ -22,6 +22,8 @@
 #include "webrtc/modules/desktop_capture/mouse_cursor.h"
 #include "webrtc/modules/desktop_capture/x11/x_error_trap.h"
 
+namespace webrtc {
+
 namespace {
 
 // WindowCapturer returns window IDs of X11 windows with WM_STATE attribute.
@@ -54,14 +56,50 @@ Window GetTopLevelWindow(Display* display, Window window) {
   return window;
 }
 
-}  // namespace
+DesktopSize GetWindowSize(Display* display, Window window) {
+  XWindowAttributes attributes;
+  {
+    XErrorTrap error_trap(display);
+    if (!XGetWindowAttributes(display, window, &attributes) ||
+        error_trap.GetLastErrorAndDisable() != 0) {
+      return DesktopSize(0, 0);

[Sergey Ulanov, 2017/05/19 20:36:03]

DesktopSize(); Don't need the zeros

[braveyao1, 2017/05/19 22:38:59]

Done.

+    }
+  }
 
-namespace webrtc {
+  return DesktopSize(attributes.width, attributes.height);
+}
+
+// As the comments to |GetTopLevelWindow()| above states, in window capture,

[Sergey Ulanov, 2017/05/19 20:36:03]

nit: || around name of a function are not necessary. Parentheses at the end make
it clear that this is a reference to a function

[braveyao1, 2017/05/19 22:38:59]

Done.

+// the captured frame is in the coordinates of the input |window|, while the
+// captured cursor position is in the coordinates of its |parent| window.
+// And these two coordinates are not always same. The |parent| window may have
+// decorations added, including the caption bar on top of the window and the
+// shadow and border around them. The offset needs to be compensated later.
+DesktopVector GetCursorPostionOffset(Display* display,
+                                     Window parent,
+                                     Window children) {

[Sergey Ulanov, 2017/05/19 20:36:03]

s/children/child/
'children' is plural

[braveyao1, 2017/05/19 22:38:59]

Done.

+  DesktopSize parent_size = GetWindowSize(display, parent);
+  DesktopSize children_size = GetWindowSize(display, children);
+
+  if (parent_size.is_empty() || children_size.is_empty()) {
+    return DesktopVector(0, 0);
+  }
+  // The |x_offset| is the thickness of the shadow and border added around.
+  // The |y_offset| doesn't include the shadow and border added at the bottom.
+  int x_offset = (parent_size.width() - children_size.width()) / 2;

[Sergey Ulanov, 2017/05/19 20:36:03]

This assumes that the border has the same width on left and right.  I don't
think it's safe to make this assumption. Is it not possible to directly get
position the child window relative to parent? XTranslateCoordinates() may be
useful here.

[braveyao1, 2017/05/19 22:38:59]

Done.

+  int y_offset = parent_size.height() - children_size.height() - x_offset;

[Sergey Ulanov, 2017/05/19 20:36:03]

This also assumes that border at the bottom is the same as on left and right. I
don't think this is a safe assumption.

[braveyao1, 2017/05/19 22:38:59]

Done.

+
+  return DesktopVector(x_offset, y_offset);
+}
+
+}  // namespace
 
 class MouseCursorMonitorX11 : public MouseCursorMonitor,
                               public SharedXDisplay::XEventHandler {
  public:
-  MouseCursorMonitorX11(const DesktopCaptureOptions& options, Window window);
+  MouseCursorMonitorX11(const DesktopCaptureOptions& options,
+                        Window window,
+                        DesktopVector offset);
   ~MouseCursorMonitorX11() override;
 
   void Init(Callback* callback, Mode mode) override;
@@ -80,6 +118,7 @@ class MouseCursorMonitorX11 : public MouseCursorMonitor,
   Callback* callback_;
   Mode mode_;
   Window window_;
+  DesktopVector offset_;
 
   bool have_xfixes_;
   int xfixes_event_base_;
@@ -90,11 +129,13 @@ class MouseCursorMonitorX11 : public MouseCursorMonitor,
 
 MouseCursorMonitorX11::MouseCursorMonitorX11(
     const DesktopCaptureOptions& options,
-    Window window)
+    Window window,
+    DesktopVector offset)
     : x_display_(options.x_display()),
       callback_(NULL),
       mode_(SHAPE_AND_POSITION),
       window_(window),
+      offset_(offset),
       have_xfixes_(false),
       xfixes_event_base_(-1),
       xfixes_error_base_(-1) {
@@ -185,8 +226,8 @@ void MouseCursorMonitorX11::Capture() {
           (window_ == root_window || child_window != None) ? INSIDE : OUTSIDE;
     }
 
-    callback_->OnMouseCursorPosition(state,
-                                     webrtc::DesktopVector(win_x, win_y));
+    callback_->OnMouseCursorPosition(
+        state, DesktopVector(win_x, win_y).subtract(offset_));
   }
 }
 
@@ -238,10 +279,13 @@ MouseCursorMonitor* MouseCursorMonitor::CreateForWindow(
     const DesktopCaptureOptions& options, WindowId window) {
   if (!options.x_display())
     return NULL;
-  window = GetTopLevelWindow(options.x_display()->display(), window);
-  if (window == None)
+  Window parent = GetTopLevelWindow(options.x_display()->display(), window);
+  if (parent == None)
     return NULL;
-  return new MouseCursorMonitorX11(options, window);
+
+  DesktopVector offset =
+      GetCursorPostionOffset(options.x_display()->display(), parent, window);
+  return new MouseCursorMonitorX11(options, parent, offset);
 }
 
 MouseCursorMonitor* MouseCursorMonitor::CreateForScreen(
@@ -249,8 +293,10 @@ MouseCursorMonitor* MouseCursorMonitor::CreateForScreen(
     ScreenId screen) {
   if (!options.x_display())
     return NULL;
+
   return new MouseCursorMonitorX11(
-      options, DefaultRootWindow(options.x_display()->display()));
+      options, DefaultRootWindow(options.x_display()->display()),
+      DesktopVector(0, 0));
 }
 
 }  // namespace webrtc