Enhancement to Launch bug: Mirroring Across 2 and more external displays |
||||||||
Issue descriptionThis is an enhancement for launch bug: 776442. Under current implementation whenever user plugs in Display A and chooses to Mirror; whenever he/she plugs in second Display B, all displays go into Extended Mode. Instead, please ensure that once user chose to Mirror to one display and connects additional displays, Mirroring is applies across all displays.
,
Nov 15 2017
,
Nov 16 2017
This was by design because we assumed that what you want depends on physical display settings. You may want to use mirroring when presenting, but most likely want to use extended when you come back to your desk. If we want to do this, then we may want to change the settings UI to control the mirroring even without external display so that you can set which mode you want when the display is connected.
,
Nov 17 2017
This implementation mostly covers kiosk use cases- where mirroring will be applied across all display right away. For desktop use case: my rational is that when user has mirroring set to one external display, and plugs in another, one would assume they would like to continue mirroring. Going back to default settings of extended display will add extra step to the user. I don't think we need UI control to mirror when no external display is detected - that would prove to be confusing. happy to chat.
,
Nov 22 2017
The current CL does more than what you described. The mirroring is now device settings, not per displays. That is, 1) you connect display A. 2) switch to mirror 3) disconnect display A 4) connect display B. 5) you're in mirror mode Just want to make sure what's what you want.
,
Nov 22 2017
Yes, that's right. The mirror mode should be on if there are 2+ display and the user turns on mirror mode. Otherwise, mirror mode should be off.
,
Nov 22 2017
What I meant is that now the mirroring is per device, you can turn off it before you connect a display. You may not want to show what's on your screen when you project, for example.
,
Nov 22 2017
By the way, I'm fine if we decided not to change UI. My question was if #5 is what we want.
,
Nov 22 2017
Do you think we could show the mirror mode checkbox even when there's only one display? In the scenario in #5, the user could turn off mirror mode before 4) if the user does not want to mirror B.
,
Nov 22 2017
ovanieva@, does the scenario in #5 sound reasonable to you?
,
Nov 22 2017
what I described is not what oshima@ listed in comment 5. what i meant is: 1) you connect display A. 2) switch to mirror 3) connect display B. 4) you're in mirror mode there is no step of disconnecting display A. Scenario in Comment 9 does not make sense - mirroring option will appear only if 1 or more external displays connected. My ask for the bug is only for cases when 1 external display is mirroring. Whenever another display is added -mirroring should persist. let me know if that makes sense
,
Nov 22 2017
the desired implementation has been agreed and signed off by UI review - please let us know when we can get OK on the Cl.
,
Nov 23 2017
My question is what I described in #5 is what you want? If not, then we should discuss because that's the side effect this bug will cause.
,
Nov 27 2017
After reviewing the impact of this change earlier today, let's revert to first implementation in crbug/776442. I'm closing this bug as won't fix.
,
Nov 28 2017
Just got this bug. Olga is right, what is described in #11 makes sense to me. Oshima is right too that what is described in #5 is not what we should do. Are you saying we can't do #11 without getting #5? The problem is that I find going from mirrored to extended weird when I just attach a new devices (again, this is only concerning the 3 monitor case, for other cases, starting out with extended for new displays is reasonable)
,
Nov 28 2017
It should be possible although it has some side effects. Let's have a quick chat tomorrow morning.
,
Nov 30 2017
Reopen this bug after offline discussion: The rule for turning off/on mirror after display configuration changes (i.e. remove/add a display) is: current_mirror_mode := previous_mirror_mode AND num_of_displays > 1
,
Dec 1 2017
after multiple chats summarizing our latest agreement: User is connected to External Display A. User turns on Mirroring. User disconnects from External Display A and reconnects back to A. RESULT: Mirroring on A. User is connected to External Display A. User turns on Mirroring. User connects second External Display B. RESULT: Mirroring on A and B. User is connected to External Display A. User turns on Mirroring. User disconnects from External Display A and connects to External Display C. RESULT: Extended Mode on C. User is connected to External Display A. User turns on Mirroring. User turns off MIrroring. User connects to External Display B. RESULT: Extended Mode on B. Boot with A, which was previously mirrored, and B, which was previously extended RESULT: Mirroring on A.
,
Dec 14 2017
,
Dec 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fa8dfaa621c1701dce841f76effecd064ff3753e commit fa8dfaa621c1701dce841f76effecd064ff3753e Author: Weidong Guo <weidongg@chromium.org> Date: Sun Dec 17 20:43:27 2017 Display mirroring persistency changes Changes: 1. Change mirroring persistency rule to: If previous mirror mode is not set (e.g. no external display in previous configuration), turn on mirror mode if at least one of the external displays was in mirror mode before and turn off mirror mode otherwise. If previous mirror mode is set, mirror mode remains its previous value as long as there are more than 1 displays. For example: Case 1: Connect display A; Turn on mirror mode; Connect display B. B should be in mirror mode as well. Case 2: Connect display A; Turn on mirror mode; Remove A; Reconnect A; A should be in mirror mode. Case 3: Connect display A; Turn on mirror mode; Remove A; Connect display B. B should not be in mirror mode. Case 4: Connect display A; Turn on mirror mode; Connect display B; Remove B; Remove A; Reconnect B; B should be in mirror mode. 2. Fix a bug in MirrorWindowController: mirroring source display may be changed in mirror mode (e.g. Connect two external displays; Turn on mirror mode; Close internal display lid; Then the internal display will be replaced with one external display as the mirroring source.), reflector and mirror windows should be updated in this case. 3. Fix broken test. Bug: 785416 ,792207 Test: MultiMirroringTest.* Change-Id: Ia1da9f25df2bcc04f98ab087478a14685fff7708 Reviewed-on: https://chromium-review.googlesource.com/804647 Commit-Queue: Weidong Guo <weidongg@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Cr-Commit-Position: refs/heads/master@{#524627} [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ash/display/display_manager_unittest.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ash/display/mirror_window_controller.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ash/display/mirror_window_controller.h [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ash/display/mirror_window_controller_unittest.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ash/display/root_window_transformers_unittest.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ash/system/screen_layout_observer_unittest.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ash/system/web_notification/web_notification_tray_unittest.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ash/touch/touch_observer_hud_unittest.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/chrome/browser/chromeos/display/display_preferences.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/chrome/browser/chromeos/display/display_preferences_unittest.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/chrome/common/pref_names.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/chrome/common/pref_names.h [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ui/display/display_layout.h [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ui/display/manager/chromeos/display_change_observer.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ui/display/manager/chromeos/display_configurator.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ui/display/manager/display_layout_store.h [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ui/display/manager/display_manager.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ui/display/manager/display_manager.h [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ui/display/manager/display_manager_utilities.cc [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ui/display/manager/display_manager_utilities.h [modify] https://crrev.com/fa8dfaa621c1701dce841f76effecd064ff3753e/ui/display/types/display_constants.h
,
Dec 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ba99d1924ab00321c5faca2d61957fdebb37c89d commit ba99d1924ab00321c5faca2d61957fdebb37c89d Author: Findit <findit-for-me@appspot.gserviceaccount.com> Date: Sun Dec 17 21:47:26 2017 Revert "Display mirroring persistency changes" This reverts commit fa8dfaa621c1701dce841f76effecd064ff3753e. Reason for revert: Findit (https://goo.gl/kROfz5) identified CL at revision 524627 as the culprit for failures in the build cycles as shown on: https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2ZhOGRmYWE2MjFjMTcwMWRjZTg0MWY3NmVmZmVjZDA2NGZmMzc1M2UM Sample Failed Build: https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-dbg/3252 Original change's description: > Display mirroring persistency changes > > Changes: > 1. Change mirroring persistency rule to: > If previous mirror mode is not set (e.g. no external display in previous > configuration), turn on mirror mode if at least one of the external > displays was in mirror mode before and turn off mirror mode otherwise. > If previous mirror mode is set, mirror mode remains its previous value > as long as there are more than 1 displays. > For example: > Case 1: Connect display A; Turn on mirror mode; Connect display B. B > should be in mirror mode as well. > Case 2: Connect display A; Turn on mirror mode; Remove A; Reconnect A; A > should be in mirror mode. > Case 3: Connect display A; Turn on mirror mode; Remove A; Connect > display B. B should not be in mirror mode. > Case 4: Connect display A; Turn on mirror mode; Connect display B; > Remove B; Remove A; Reconnect B; B should be in mirror mode. > > 2. Fix a bug in MirrorWindowController: mirroring source display may be > changed in mirror mode (e.g. Connect two external displays; Turn on > mirror mode; Close internal display lid; Then the internal display will > be replaced with one external display as the mirroring source.), > reflector and mirror windows should be updated in this case. > > 3. Fix broken test. > > Bug: 785416 ,792207 > Test: MultiMirroringTest.* > Change-Id: Ia1da9f25df2bcc04f98ab087478a14685fff7708 > Reviewed-on: https://chromium-review.googlesource.com/804647 > Commit-Queue: Weidong Guo <weidongg@chromium.org> > Reviewed-by: Mitsuru Oshima <oshima@chromium.org> > Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> > Cr-Commit-Position: refs/heads/master@{#524627} Change-Id: Ie18b8dc47d031984590ed98af331fdb659b17941 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 785416 ,792207 Reviewed-on: https://chromium-review.googlesource.com/831290 Cr-Commit-Position: refs/heads/master@{#524629} [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ash/display/display_manager_unittest.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ash/display/mirror_window_controller.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ash/display/mirror_window_controller.h [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ash/display/mirror_window_controller_unittest.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ash/display/root_window_transformers_unittest.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ash/system/screen_layout_observer_unittest.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ash/system/web_notification/web_notification_tray_unittest.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ash/touch/touch_observer_hud_unittest.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/chrome/browser/chromeos/display/display_preferences.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/chrome/browser/chromeos/display/display_preferences_unittest.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/chrome/common/pref_names.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/chrome/common/pref_names.h [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ui/display/display_layout.h [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ui/display/manager/chromeos/display_change_observer.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ui/display/manager/chromeos/display_configurator.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ui/display/manager/display_layout_store.h [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ui/display/manager/display_manager.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ui/display/manager/display_manager.h [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ui/display/manager/display_manager_utilities.cc [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ui/display/manager/display_manager_utilities.h [modify] https://crrev.com/ba99d1924ab00321c5faca2d61957fdebb37c89d/ui/display/types/display_constants.h
,
Dec 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae commit 78d6fb65c66a4c6f2aecc882768a9002aaa8aeae Author: Weidong Guo <weidongg@chromium.org> Date: Mon Dec 18 01:47:56 2017 Reland "Display mirroring persistency changes" This is a reland of fa8dfaa621c1701dce841f76effecd064ff3753e Original change's description: > Display mirroring persistency changes > > Changes: > 1. Change mirroring persistency rule to: > If previous mirror mode is not set (e.g. no external display in previous > configuration), turn on mirror mode if at least one of the external > displays was in mirror mode before and turn off mirror mode otherwise. > If previous mirror mode is set, mirror mode remains its previous value > as long as there are more than 1 displays. > For example: > Case 1: Connect display A; Turn on mirror mode; Connect display B. B > should be in mirror mode as well. > Case 2: Connect display A; Turn on mirror mode; Remove A; Reconnect A; A > should be in mirror mode. > Case 3: Connect display A; Turn on mirror mode; Remove A; Connect > display B. B should not be in mirror mode. > Case 4: Connect display A; Turn on mirror mode; Connect display B; > Remove B; Remove A; Reconnect B; B should be in mirror mode. > > 2. Fix a bug in MirrorWindowController: mirroring source display may be > changed in mirror mode (e.g. Connect two external displays; Turn on > mirror mode; Close internal display lid; Then the internal display will > be replaced with one external display as the mirroring source.), > reflector and mirror windows should be updated in this case. > > 3. Fix broken test. > > Bug: 785416 ,792207 > Test: MultiMirroringTest.* > Change-Id: Ia1da9f25df2bcc04f98ab087478a14685fff7708 > Reviewed-on: https://chromium-review.googlesource.com/804647 > Commit-Queue: Weidong Guo <weidongg@chromium.org> > Reviewed-by: Mitsuru Oshima <oshima@chromium.org> > Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> > Cr-Commit-Position: refs/heads/master@{#524627} TBR=afakhry@chromium.org, oshima@chromium.org Bug: 785416 , 792207 Change-Id: I6f0c7b4169f5a0e0ab8a8f97a073cd165d21622d Reviewed-on: https://chromium-review.googlesource.com/831666 Reviewed-by: Weidong Guo <weidongg@chromium.org> Commit-Queue: Weidong Guo <weidongg@chromium.org> Cr-Commit-Position: refs/heads/master@{#524636} [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ash/display/display_manager_unittest.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ash/display/mirror_window_controller.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ash/display/mirror_window_controller.h [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ash/display/mirror_window_controller_unittest.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ash/display/root_window_transformers_unittest.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ash/system/screen_layout_observer_unittest.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ash/system/web_notification/web_notification_tray_unittest.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ash/touch/touch_observer_hud_unittest.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/chrome/browser/chromeos/display/display_preferences.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/chrome/browser/chromeos/display/display_preferences_unittest.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/chrome/common/pref_names.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/chrome/common/pref_names.h [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ui/display/display_layout.h [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ui/display/manager/chromeos/display_change_observer.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ui/display/manager/chromeos/display_configurator.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ui/display/manager/display_layout_store.h [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ui/display/manager/display_manager.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ui/display/manager/display_manager.h [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ui/display/manager/display_manager_utilities.cc [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ui/display/manager/display_manager_utilities.h [modify] https://crrev.com/78d6fb65c66a4c6f2aecc882768a9002aaa8aeae/ui/display/types/display_constants.h
,
Dec 18 2017
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by weidongg@chromium.org
, Nov 15 2017