Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2889063002:
Linux desktopCapture: fix the cursor position issue in Window sharing (Closed)
|
Created:
3 years, 7 months ago by braveyao1 Modified:
3 years, 7 months ago Reviewers:
Sergey Ulanov CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionLinux desktopCapture: fix the cursor position issue in Window sharing
On Linux, during Windwo sharing, the cursore capture may happen in the parent
window of the target. And the parent window may have some decorations added by
window manager(Chrome windows don't have those decorations.), so the relative
cursor position to the parent window with decorations may differ to its child
target window. The offset includes the height of caption bar and the around
shadow and border.
This problem only happens with Window sharing on Linux.
The fix is to translate the coordinates from the parent window to the coordinates space of the target window.
BUG=chromium:723889
Review-Url: https://codereview.webrtc.org/2889063002
Cr-Commit-Position: refs/heads/master@{#18243}
Committed: https://chromium.googlesource.com/external/webrtc/+/d019667c001679b9c73b63735f2c2fca92c530d6
Patch Set 1 #
Total comments: 10
Patch Set 2 : re-do the cl with applying XTranslateCoordinates() method #Messages
Total messages: 30 (22 generated)
Description was changed from ========== Linux desktopCapture: fix the cursor position issue in Window sharing BUG= ========== to ========== Linux desktopCapture: fix the cursor position issue in Window sharing On Linux, during Windwo sharing, the cursore capture may happen in the parent window of the target. And the parent window may have some decorations added by window manager(Chrome windows don't have those decorations.), so the relative cursor position to the parent window with decorations may differ to its child target window. The offset includes the height of caption bar and the around shadow and border. This problem only happens with Window sharing on Linux. The fix is to find out the offset between the target window and its parent, and compensate it to the captured cursor position. BUG=723889 ==========
The CQ bit was checked by braveyao@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #1 (id:1) has been deleted
The CQ bit was checked by braveyao@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
braveyao@webrtc.org changed reviewers: + sergeyu@chromium.org
Hi sergeyu@, please take a look at this.
https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc (right): https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc:65: return DesktopSize(0, 0); DesktopSize(); Don't need the zeros https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc:72: // As the comments to |GetTopLevelWindow()| above states, in window capture, nit: || around name of a function are not necessary. Parentheses at the end make it clear that this is a reference to a function https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc:80: Window children) { s/children/child/ 'children' is plural https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc:89: int x_offset = (parent_size.width() - children_size.width()) / 2; 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. https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc:90: int y_offset = parent_size.height() - children_size.height() - x_offset; 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.
The CQ bit was checked by braveyao@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Thanks sooooo much for the recommendation of XTranslateCoordinates() method. The cl is much more simple now. PTAL. https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc (right): https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc:65: return DesktopSize(0, 0); On 2017/05/19 20:36:03, Sergey Ulanov wrote: > DesktopSize(); Don't need the zeros Done. https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc:72: // As the comments to |GetTopLevelWindow()| above states, in window capture, On 2017/05/19 20:36:03, Sergey Ulanov wrote: > nit: || around name of a function are not necessary. Parentheses at the end make > it clear that this is a reference to a function Done. https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc:80: Window children) { On 2017/05/19 20:36:03, Sergey Ulanov wrote: > s/children/child/ > 'children' is plural Done. https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc:89: int x_offset = (parent_size.width() - children_size.width()) / 2; On 2017/05/19 20:36:03, Sergey Ulanov wrote: > 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. Done. https://codereview.webrtc.org/2889063002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/mouse_cursor_monitor_x11.cc:90: int y_offset = parent_size.height() - children_size.height() - x_offset; On 2017/05/19 20:36:03, Sergey Ulanov wrote: > 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. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by braveyao@webrtc.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Description was changed from ========== Linux desktopCapture: fix the cursor position issue in Window sharing On Linux, during Windwo sharing, the cursore capture may happen in the parent window of the target. And the parent window may have some decorations added by window manager(Chrome windows don't have those decorations.), so the relative cursor position to the parent window with decorations may differ to its child target window. The offset includes the height of caption bar and the around shadow and border. This problem only happens with Window sharing on Linux. The fix is to find out the offset between the target window and its parent, and compensate it to the captured cursor position. BUG=723889 ========== to ========== Linux desktopCapture: fix the cursor position issue in Window sharing On Linux, during Windwo sharing, the cursore capture may happen in the parent window of the target. And the parent window may have some decorations added by window manager(Chrome windows don't have those decorations.), so the relative cursor position to the parent window with decorations may differ to its child target window. The offset includes the height of caption bar and the around shadow and border. This problem only happens with Window sharing on Linux. The fix is to translate the coordinates from the parent window to the coordinates space of the target window. BUG=723889 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_x64_clang_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_clang_dbg/build...)
The CQ bit was checked by braveyao@webrtc.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495556392423970,
"parent_rev": "c1b5ea959ed058c7b44c437f1cbbbccf6d9daf6c", "commit_rev":
"d019667c001679b9c73b63735f2c2fca92c530d6"}
Message was sent while issue was closed.
Description was changed from ========== Linux desktopCapture: fix the cursor position issue in Window sharing On Linux, during Windwo sharing, the cursore capture may happen in the parent window of the target. And the parent window may have some decorations added by window manager(Chrome windows don't have those decorations.), so the relative cursor position to the parent window with decorations may differ to its child target window. The offset includes the height of caption bar and the around shadow and border. This problem only happens with Window sharing on Linux. The fix is to translate the coordinates from the parent window to the coordinates space of the target window. BUG=723889 ========== to ========== Linux desktopCapture: fix the cursor position issue in Window sharing On Linux, during Windwo sharing, the cursore capture may happen in the parent window of the target. And the parent window may have some decorations added by window manager(Chrome windows don't have those decorations.), so the relative cursor position to the parent window with decorations may differ to its child target window. The offset includes the height of caption bar and the around shadow and border. This problem only happens with Window sharing on Linux. The fix is to translate the coordinates from the parent window to the coordinates space of the target window. BUG=723889 Review-Url: https://codereview.webrtc.org/2889063002 Cr-Commit-Position: refs/heads/master@{#18243} Committed: https://chromium.googlesource.com/external/webrtc/+/d019667c001679b9c73b63735... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as https://chromium.googlesource.com/external/webrtc/+/d019667c001679b9c73b63735...
Message was sent while issue was closed.
Description was changed from ========== Linux desktopCapture: fix the cursor position issue in Window sharing On Linux, during Windwo sharing, the cursore capture may happen in the parent window of the target. And the parent window may have some decorations added by window manager(Chrome windows don't have those decorations.), so the relative cursor position to the parent window with decorations may differ to its child target window. The offset includes the height of caption bar and the around shadow and border. This problem only happens with Window sharing on Linux. The fix is to translate the coordinates from the parent window to the coordinates space of the target window. BUG=723889 Review-Url: https://codereview.webrtc.org/2889063002 Cr-Commit-Position: refs/heads/master@{#18243} Committed: https://chromium.googlesource.com/external/webrtc/+/d019667c001679b9c73b63735... ========== to ========== Linux desktopCapture: fix the cursor position issue in Window sharing On Linux, during Windwo sharing, the cursore capture may happen in the parent window of the target. And the parent window may have some decorations added by window manager(Chrome windows don't have those decorations.), so the relative cursor position to the parent window with decorations may differ to its child target window. The offset includes the height of caption bar and the around shadow and border. This problem only happens with Window sharing on Linux. The fix is to translate the coordinates from the parent window to the coordinates space of the target window. BUG=chromium:723889 Review-Url: https://codereview.webrtc.org/2889063002 Cr-Commit-Position: refs/heads/master@{#18243} Committed: https://chromium.googlesource.com/external/webrtc/+/d019667c001679b9c73b63735... ========== |
