[Desktop Capture] wrong cursor position during Window sharing on Linux |
|||||||||||||
Issue descriptionOS: Linux What steps will reproduce the problem? (1) Build ToT Chromium (2) Launch Chromium and share an application window, i.e. Files (3) Move mouse within Files window What is the expected result? Cursor position is exact same in the captured window What happens instead? Cursor position deviates a lot. Please use labels and text to provide additional information. This issue firstly emerges in chromium after revision #500761, https://chromium-review.googlesource.com/c/chromium/src/+/651147. But the real culprit should be in other previous changes. For graphics-related bugs, please copy/paste the contents of the about:gpu page at the end of this report.
,
Oct 24 2017
All default setting. (I have two monitors connected, 30"+24". Does it matter?) Are you testing with latest Chromium? Enclose the screenshot, taken with building #500761. Building with #500760 doesn't have this problem. PS: on Mac it's another symptom. I'll file another issue after final verification. You can try with Canary. On Win, Canary still works fine.
,
Oct 24 2017
Oh, good point, I thought my change was enabled for a while. I will have a look, thank you for letting me know.
,
Oct 24 2017
Forgot to mention: don't share chromium its own window, which will go through tab capture code path. Try any other window.
,
Oct 25 2017
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/173fd91b56f5655890e8a6502f909d217e3a9c03 commit 173fd91b56f5655890e8a6502f909d217e3a9c03 Author: Zijie He <zijiehe@google.com> Date: Wed Oct 25 20:41:29 2017 [Window Capture] Inaccurate cursor position during window sharing on X11 {root_x, root_y} should be used to report the absolute cursor position in MouseCursorMonitorX11. Bug: chromium:778035 Change-Id: I421005d52786a57da8e8c3901bdf4afa2843ff24 Reviewed-on: https://webrtc-review.googlesource.com/15680 Reviewed-by: Jamie Walch <jamiewalch@chromium.org> Commit-Queue: Zijie He <zijiehe@chromium.org> Cr-Commit-Position: refs/heads/master@{#20432} [modify] https://crrev.com/173fd91b56f5655890e8a6502f909d217e3a9c03/modules/desktop_capture/mouse_cursor_monitor_x11.cc
,
Oct 25 2017
This seems not an urgent problem for a merging request.
,
Oct 26 2017
Let me know if you have a different opinion about the merging.
,
Oct 27 2017
Change in #c5 needs to be merged to M63/WebRTC.
,
Oct 27 2017
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 27 2017
Before we approve merge to M63 for cl listed #5, please provide following details: Is this M63 regression and critical to merge? Is the change well baked/verified in Canary and safe to merge? Please provide any other details to justify the merge. Thank you. Please note M63 is already promoted to Beta so merge bar is very high. Thank you.
,
Oct 28 2017
,
Oct 28 2017
Yes, this is a regression of M63. We are still verifying this change on Canary.
,
Oct 28 2017
OK, Pls update the bug with Canary result.Thank you.
,
Oct 30 2017
Just to update, rechecked this issue on Ubuntu 14.04 using chrome version 64.0.3253.0 and still able to reproduce it. Cursor position is still not accurate when mouse hovered on a file when sharing the application window in the file manager. Thanks.!
,
Oct 30 2017
I also can reproduce it on my Linux box with ToT Chromium building, 64.0.3254.0 (Developer Build) (64-bit), master@{#512508}.
Linux box: HP z840 with ubuntu 14.04.1, two monitors connected, 30"+24"
Attached the screenshot. Please have a check.
,
Oct 31 2017
Just to update, rechecked this issue on Ubuntu 14.04 using chrome version 64.0.3254.0 and still able to reproduce it. Cursor position is still not accurate when mouse hovered on a file when sharing the application window in the file manager. Taking a screen shot is not capturing the cursor postion, hence attached a screen cast for the same. Request you to please take a look into it. Thanks.!
,
Oct 31 2017
,
Oct 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a511b3d8f416498d105a3a42e7c95b16ba2e4d5d commit a511b3d8f416498d105a3a42e7c95b16ba2e4d5d Author: Zijie He <zijiehe@chromium.org> Date: Tue Oct 31 22:20:34 2017 Revert "[Desktop Capturer] Use new DesktopAndCursorComposer constructor" This reverts commit cf3aebae2044c573b4b23a2fa490ff0cc3d2bfe0. Reason for revert: The new API breaks on X11 and Mac OSX. Original change's description: > [Desktop Capturer] Use new DesktopAndCursorComposer constructor > > After several changes, we finally removed the dependency of SourceId in > MouseCursorMonitor. Now DesktopAndCursorComposer provides a clearer > constructor for its clients. > > Bug: webrtc:7950 > Change-Id: I8716d736897637412e712fcc2b2d17ef0acf3eab > Reviewed-on: https://chromium-review.googlesource.com/651147 > Commit-Queue: Zijie He <zijiehe@chromium.org> > Reviewed-by: Jamie Walch <jamiewalch@chromium.org> > Reviewed-by: Weiyong Yao <braveyao@chromium.org> > Cr-Commit-Position: refs/heads/master@{#500761} TBR=jamiewalch@chromium.org,braveyao@chromium.org,zijiehe@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:7950 , chromium:778035 , chromium:778049 Change-Id: I45fbc68ae52194164828bc34207fe849850a98bf Reviewed-on: https://chromium-review.googlesource.com/746947 Reviewed-by: Weiyong Yao <braveyao@chromium.org> Reviewed-by: Zijie He <zijiehe@chromium.org> Commit-Queue: Zijie He <zijiehe@chromium.org> Cr-Commit-Position: refs/heads/master@{#512976} [modify] https://crrev.com/a511b3d8f416498d105a3a42e7c95b16ba2e4d5d/content/browser/media/capture/desktop_capture_device.cc
,
Oct 31 2017
Instead of #c5, #c19 need to be merged to the M63. It turns out to be a relative complex fix for M63. So a safer way is to block the new logic from taking effect by merging the revert in #c19. After this change, window capturer uses the same logic as M62.
,
Oct 31 2017
Thank you zijiehe@. Please update the bug once revert listed at #19 is well baked/verified in Canary and safe to merge to M63.
,
Nov 1 2017
Rechecked this issue on Ubuntu 14.04 using chrome version 64.0.3255.0 and fix is working as intended. Able to view the cursor at the same position on the shared application window. Adding TE-Verified labels for M64
,
Nov 1 2017
Ranji and I both verified the change on Linux and Mac OSX. This change should be safe to merge to M63.
,
Nov 1 2017
Approving merge for revert listed at #19 to M63 branch 3239 based on comment #22 and #23. Please merge ASAP. Thank you.
,
Nov 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9f9fc3111e7035dc8e133f1f736e014df9f399b7 commit 9f9fc3111e7035dc8e133f1f736e014df9f399b7 Author: Zijie He <zijiehe@chromium.org> Date: Wed Nov 01 17:40:52 2017 Revert "[Desktop Capturer] Use new DesktopAndCursorComposer constructor" This reverts commit cf3aebae2044c573b4b23a2fa490ff0cc3d2bfe0. Reason for revert: The new API breaks on X11 and Mac OSX. Original change's description: > [Desktop Capturer] Use new DesktopAndCursorComposer constructor > > After several changes, we finally removed the dependency of SourceId in > MouseCursorMonitor. Now DesktopAndCursorComposer provides a clearer > constructor for its clients. > > Bug: webrtc:7950 > Change-Id: I8716d736897637412e712fcc2b2d17ef0acf3eab > Reviewed-on: https://chromium-review.googlesource.com/651147 > Commit-Queue: Zijie He <zijiehe@chromium.org> > Reviewed-by: Jamie Walch <jamiewalch@chromium.org> > Reviewed-by: Weiyong Yao <braveyao@chromium.org> > Cr-Commit-Position: refs/heads/master@{#500761} TBR=jamiewalch@chromium.org,braveyao@chromium.org,zijiehe@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:7950 , chromium:778035 , chromium:778049 Change-Id: I45fbc68ae52194164828bc34207fe849850a98bf Reviewed-on: https://chromium-review.googlesource.com/746947 Reviewed-by: Weiyong Yao <braveyao@chromium.org> Reviewed-by: Zijie He <zijiehe@chromium.org> Commit-Queue: Zijie He <zijiehe@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#512976}(cherry picked from commit a511b3d8f416498d105a3a42e7c95b16ba2e4d5d) Reviewed-on: https://chromium-review.googlesource.com/749104 Cr-Commit-Position: refs/branch-heads/3239@{#334} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/9f9fc3111e7035dc8e133f1f736e014df9f399b7/content/browser/media/capture/desktop_capture_device.cc
,
Nov 1 2017
Merged. Thank you.
,
Nov 1 2017
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/868607710204d8a249290531a2efde1ef4362a0e commit 868607710204d8a249290531a2efde1ef4362a0e Author: Zijie He <zijiehe@google.com> Date: Wed Nov 01 18:17:07 2017 [Window Capturer] Inaccurate cursor position on cinnamon When cinnamon is used, it always wraps the application window with its own window. Instead of (0, 0), the DesktopRect from XWindowAttributes starts from (10, 36). So this change considers this difference when translating the DesktopRect in GetWindowRect() function. Bug: chromium:778035 Change-Id: I4944b2d1e13a4c379e114fd1749d74e95a63003b Reviewed-on: https://webrtc-review.googlesource.com/17660 Reviewed-by: Jamie Walch <jamiewalch@chromium.org> Commit-Queue: Zijie He <zijiehe@chromium.org> Cr-Commit-Position: refs/heads/master@{#20538} [modify] https://crrev.com/868607710204d8a249290531a2efde1ef4362a0e/modules/desktop_capture/x11/window_list_utils.cc
,
Nov 1 2017
After the change in #c27, this issue should be fixed. But we need to flip the switch to make the new logic work.
,
Nov 3 2017
,
Nov 7 2017
Rechecked this issue on Ubuntu 14.04 using chrome version 63.0.3239.39 and fix is working as intended. Able to view the cursor at the same position on the shared application window. Adding TE-Verified labels for M63
,
Nov 7 2017
It's expected. The new logic has not been enabled, so the behavior won't change by last CL. |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by zijiehe@chromium.org
, Oct 24 2017