ScreenCapturerMac doesn't capture whole desktop correctly (kFullDesktopScreenId) |
|||||||||||
Issue descriptionWhat steps will reproduce the problem? (1) Connect to Chrome Remote Desktop host running on Mac with multiple monitors What is the expected result? All monitors are shown on the client What happens instead? Screen content is not updated correctly The same issue can be reproduced by commenting SelectSource() call in desktop_capture_device.cc (here https://codesearch.chromium.org/chromium/src/content/browser/media/capture/desktop_capture_device.cc?l=388 ). With that change the desktop capturer should capture whole desktop. See my comments on https://codereview.webrtc.org/2496413002 I'd like to get this fixed for M-58. Erik, please let me know if you don't have time to work on this fix or if you need help reproducing it.
,
Mar 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/31bbee73a029376ca5a6f74e3d5bf224053a6208 commit 31bbee73a029376ca5a6f74e3d5bf224053a6208 Author: erikchen <erikchen@chromium.org> Date: Thu Mar 09 21:19:03 2017 mac: Fix screen capture for whole-desktop capture. DisplayStream refresh rects are in display coordinates. When the whole screen is being captured, the coordinates passed to the ScreenCapturerHelper need to be in screen coordinates. This CL translates display coordinates to screen coordinates for whole screen capture. BUG= chromium:699672 Review-Url: https://codereview.webrtc.org/2740823002 Cr-Commit-Position: refs/heads/master@{#17153} [modify] https://crrev.com/31bbee73a029376ca5a6f74e3d5bf224053a6208/webrtc/modules/desktop_capture/screen_capturer_mac.mm
,
Mar 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/31bbee73a029376ca5a6f74e3d5bf224053a6208 commit 31bbee73a029376ca5a6f74e3d5bf224053a6208 Author: erikchen <erikchen@chromium.org> Date: Thu Mar 09 21:19:03 2017 mac: Fix screen capture for whole-desktop capture. DisplayStream refresh rects are in display coordinates. When the whole screen is being captured, the coordinates passed to the ScreenCapturerHelper need to be in screen coordinates. This CL translates display coordinates to screen coordinates for whole screen capture. BUG= chromium:699672 Review-Url: https://codereview.webrtc.org/2740823002 Cr-Commit-Position: refs/heads/master@{#17153} [modify] https://crrev.com/31bbee73a029376ca5a6f74e3d5bf224053a6208/webrtc/modules/desktop_capture/screen_capturer_mac.mm
,
Mar 10 2017
I'm not familiar with merging webrtc changes to branches - can someone take care of that part?
,
Mar 10 2017
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), bhthompson@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 10 2017
,
Mar 12 2017
Please merge your change to M58 branch 3029 before 5:00 PM PT, Monday (03/13/17) so we can take it in for next week dev release. Thank you!
,
Mar 14 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 14 2017
,
Mar 14 2017
Your change is approved for M58. Please ensure whether this fix is verified in canary. If yes, please merge ASAP so that it will be picked up for Beta promotion RC cut on 03/15 at 5.00 PM PST.
,
Mar 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/5810d27a9b13fe581282acc33350525be43e6f95 commit 5810d27a9b13fe581282acc33350525be43e6f95 Author: Sergey Ulanov <sergeyu@chromium.org> Date: Wed Mar 15 18:40:11 2017 mac: Fix screen capture for whole-desktop capture. DisplayStream refresh rects are in display coordinates. When the whole screen is being captured, the coordinates passed to the ScreenCapturerHelper need to be in screen coordinates. This CL translates display coordinates to screen coordinates for whole screen capture. BUG= chromium:699672 Review-Url: https://codereview.webrtc.org/2740823002 Cr-Commit-Position: refs/heads/master@{#17153} (cherry picked from commit 31bbee73a029376ca5a6f74e3d5bf224053a6208) Review-Url: https://codereview.webrtc.org/2755663004 . Cr-Commit-Position: refs/branch-heads/58@{#4} Cr-Branched-From: f31969a584bcafe9406c214a9d4c3afb49d19650-refs/heads/master@{#16937} [modify] https://crrev.com/5810d27a9b13fe581282acc33350525be43e6f95/webrtc/modules/desktop_capture/screen_capturer_mac.mm
,
Mar 15 2017
,
Mar 16 2017
Per comment #11, this is already merged to M58. Hence, removing "Merge-Approved-58" label.
,
Mar 22 2017
When connecting to a Mac mini with two equal sized monitors, this is fixed. The mini is non-corp. When connecting to a corp MBP with a second monitor connected through Displayport with curtain mode on, one screen is fine but the other is black. I do not have access to another corp Mac, so cannot say if this is because the second monitor is not the same resolution, if its because its corp, or if its because curtain mode is on. I'll disable curtain mode and update the bug with the results.
,
Mar 22 2017
When connecting to the MBP with curtain mode off, both displays show up, so it is mostly correct. However, there is corruption below the area of the first monitor (the smaller one) which fills in the gap to match the size of the second monitor.
,
Mar 23 2017
,
Mar 23 2017
AJ, can you please attach a screenshot?
,
Mar 24 2017
After connecting from a different client, I can no longer repro what I described above. Instead, the client only shows the primary monitor. I wrote bug 704781 for this.
,
Mar 28 2017
,
Mar 30 2017
Verified Fixed in 58.0.3029.28 |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by erikc...@chromium.org
, Mar 8 2017