New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 771178 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 798857



Sign in to add a comment

Hookup DisplayConfigurator when running on desktop

Project Member Reported by osh...@chromium.org, Oct 3 2017

Issue description

Currently, DisplayConfigurator is bypassed when running chrome for cros on desktop. (Display configuration is done inside display manager in this environment)

With ozone x11, we should be able to use DisplayConfigurator to configure the displays on desktop environment, which in turn will increase the coverage in unit tests.

 

Comment 1 by ovanieva@google.com, Oct 10 2017

Owner: kylec...@chromium.org
Status: Assigned (was: Available)
Owner: ----
Status: Available (was: Assigned)
Sorry, I won't be able to work on this in the foreseeable future.
@kylechar - is there anyone in the team we could assign it to?
My team primarily workings on GPU code and servicification. I don't think anyone else on it has much background in Chrome OS display management code unfortunately.

I wrote the FakeDisplayDelegate code for mus+ash that accomplishes using DisplayConfigurator on desktop. I have a pretty good idea how to adapt that to regular Chrome OS. I'll write up a list of tasks needed to make it work but otherwise I can't prioritize this now.
Owner: kylec...@chromium.org
assigning to you for the list of tasks and we will take it from here. 
Status: Assigned (was: Available)
Owner: ovanieva@chromium.org
Heres how to turn on the display configuration code when running on desktop:
1. Set DisplayManager::configure_displays_ and DisplayConfigurator::configure_display_ to true.
2. Make sure ash::Shell::Init() creates display::DisplayChangeObserver and ash::ShutdownObserver. There is some configuration that goes along with this here:
https://cs.chromium.org/chromium/src/ash/shell.cc?l=933&rcl=fae9dfcfcafb3763d8832c1b490f0c18ca43fe07
3. Don't pass in the --ash-host-window-bounds flag.
4. Off device a FakeDisplayDelegate is created by Ozone. display::DisplayManager needs a display::mojom::DevDisplayControllerPtr injected into it for add/remove display accelerators. This happens already for mus/mash. ash::Shell::Init() would need to do this in cash too. The one complication is where the mojo::Binding<display::mojom::DevDisplayController> should live. That could be moved into display::FakeDisplayDelegate in cash and mus/mash cases.

I think at that point everything should be connected and almost work in classic ash. Here is what needs to be fixed:
1. DrmNativeDisplayDelegate is a bit weird. Calls to GetDisplays() and Configure() return synchronously during ash startup so that ash::Shell::Init() doesn't block. FakeDisplayDelegate doesn't do this and I think ash::Shell::Init() would either hang or do something weird. FakeDisplayDelegate should do the same thing as DrmNativeDisplayDelegate and be synchronous until startup has finished.
2. FakeDisplayDelegate checks the --screen-config flag instead of --ash-host-window-bounds. The two flags are similar but --screen-config doesn't provide any way to position the PlatformWindows (eg. XWindows) on the desktop. This functionality should be added to FakeDisplayDelegate.
3. ash_unittests and browser_tests need to be modified now that they use DisplayConfigurator. Tests inject ManagedDisplayInfos directly into DisplayManager now using DisplayManagerTestApi which won't work. DisplayManagerTestApi should use display::mojom::DevDisplayController to change the display state in NativeDisplayDelegate. The display state changes will bubble back up through NativeDisplayDelegate->DisplayConfigurator->DisplayChangeObserver->DisplayManager. Some tests will need to be async. 

Assigning back to ovaniena for triage.
Cc: -afakhry@chromium.org ovanieva@chromium.org
Owner: afakhry@chromium.org
Thanks, Kyle. I'll add this to my backlog.

Comment 9 by msw@chromium.org, Dec 21 2017

Cc: afakhry@chromium.org derat@chromium.org sky@chromium.org rjkroege@chromium.org msw@chromium.org sadrul@chromium.org marc...@chromium.org osh...@chromium.org
 Issue 796282  has been merged into this issue.
Blocking: 798857

Sign in to add a comment