New issue
Advanced search Search tips

Issue 922242 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 17
Components:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 1
Type: Bug



Sign in to add a comment

enable VizDisplayCompositor on Fuchsia

Project Member Reported by spang@chromium.org, Jan 15

Issue description

VizDisplayCompositor was disabled on Fuchsia ea5fccd162ef ("[viz][fuchsia] Disable OOP-D by default on Fuchsia") because it crashes if it tries to use software output.

Software output currently assumes it is run in the browser on the UI thread and makes direct accesses to the browser's scenic session.

Switching it to create resources in a child Scenic session and attach them to the View in the browser will fix the issue. This is what the vulkan path does.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 16 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/55c99f217b541330d13f5c9fc462f5b78d400b2f

commit 55c99f217b541330d13f5c9fc462f5b78d400b2f
Author: Michael Spang <spang@chromium.org>
Date: Wed Jan 16 22:31:28 2019

ozone: scenic: Use IO message loop for VizCompositorThread

Like Windows, we need to be able to respond to messages from the platform
about display surfaces from any thread that creates them.

Without one it blows up with the following:

ASSERT FAILED at (../../third_party/fuchsia-sdk/sdk/pkg/fidl_cpp/internal/message_reader.cc:104): dispatcher_ != nullptr
either |dispatcher| must be non-null, or |async_get_default_dispatcher| must be configured to return a non-null value

Bug:  922242 
Test: run_content_shell --enable-features=VizDisplayCompositor on fuchsia

Change-Id: Id58efa6164c9840a27964afba1585d002152ff4d
Reviewed-on: https://chromium-review.googlesource.com/c/1413657
Commit-Queue: Michael Spang <spang@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623409}
[modify] https://crrev.com/55c99f217b541330d13f5c9fc462f5b78d400b2f/components/viz/service/main/viz_compositor_thread_runner.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 16 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2663e3d1564571285c270842849fb3204c097dcc

commit 2663e3d1564571285c270842849fb3204c097dcc
Author: Michael Spang <spang@chromium.org>
Date: Wed Jan 16 23:28:51 2019

ozone: scenic: Make software output use ScenicSurface

Software output currently directly accesses ScenicWindow internals from
the browser UI thread and this is not compatible with Viz display
compositor, which needs the software output device to work out of
process.

To work out of process, we need to a new scenic session altogether.
Refactor ScenicWindowCanvas to rely on ScenicSurface, which is our
rendering mode agnostic object for managing an isolated scenic session
and common resources. Hold the ScenicSurface on SoftwareOutputDeviceOzone
(at least for now).

ScenicSurface calls a mojo endpoint to attach itself to the parent view
in the browser. For the browser compositor (non OOP-D) path, we can bind
another copy of that endpoint on the UI thread. Both ends are on the same
thread but using mojo unifies the browser & GPU process cases.

This still only works in the browser; for the Viz software path we have
to worry about the fact that the OOP-D thread is not the same as the GPU
thread, but the GPU thread is currently the only thread with access to
the (thread-hostile) mojo proxies that surfaces need.

Bug:  922242 
Test: run_content_shell on fuchsia

Change-Id: Ieebfe1e152d0de0e49af89879f51b4bccf16096a
Reviewed-on: https://chromium-review.googlesource.com/c/1413656
Commit-Queue: Michael Spang <spang@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623444}
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/components/viz/service/display_embedder/gpu_display_provider.cc
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/components/viz/service/display_embedder/software_output_device_ozone.cc
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/components/viz/service/display_embedder/software_output_device_ozone.h
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/components/viz/service/display_embedder/software_output_device_ozone_unittest.cc
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/ui/ozone/platform/scenic/ozone_platform_scenic.cc
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/ui/ozone/platform/scenic/scenic_gpu_host.cc
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/ui/ozone/platform/scenic/scenic_gpu_host.h
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/ui/ozone/platform/scenic/scenic_gpu_service.cc
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/ui/ozone/platform/scenic/scenic_gpu_service.h
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/ui/ozone/platform/scenic/scenic_surface.cc
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/ui/ozone/platform/scenic/scenic_surface.h
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/ui/ozone/platform/scenic/scenic_surface_factory.cc
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/ui/ozone/platform/scenic/scenic_surface_factory.h
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/ui/ozone/platform/scenic/scenic_window_canvas.cc
[modify] https://crrev.com/2663e3d1564571285c270842849fb3204c097dcc/ui/ozone/platform/scenic/scenic_window_canvas.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 17 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6cae8f663715cfc50ec37bff06df0cd04c6193fb

commit 6cae8f663715cfc50ec37bff06df0cd04c6193fb
Author: Michael Spang <spang@chromium.org>
Date: Thu Jan 17 02:25:04 2019

ozone: scenic: Make software output work with VizDisplayCompositor

This makes software output work from Viz on Fuchsia by allowing creation
of surfaces from non "main" threads. Previously we assumed that the
thread that ran our initializer was the only one creating any surfaces,
which were the browser UI and Viz GPU threads. This assumption does not
hold in Viz display compositor's software path.

Since we use mojo and mojo proxy objects are thread hostile, a couple of
operations are now posted to the thread that runs the initializer ("main"
thread).

  - Creation of sessions via the master scenic FIDL endpoint
  - Messaging to the browser to attach scenic surfaces to their parents

These are infrequent operations so it shouldn't be a performance concern.

Locking is used to protect the surface map. Surfaces are still thread
hostile, but software surfaces can now be created from any thread as long
as they are used and destroyed on the same thread.

Bug:  922242 
Test: run_content_shell --enable-features=VizDisplayCompositor on fuchsia

Change-Id: I7acba0d3cc03ac27825616bec40d86f682d4bf62
Reviewed-on: https://chromium-review.googlesource.com/c/1413658
Commit-Queue: Michael Spang <spang@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623522}
[modify] https://crrev.com/6cae8f663715cfc50ec37bff06df0cd04c6193fb/ui/ozone/platform/scenic/scenic_surface.cc
[modify] https://crrev.com/6cae8f663715cfc50ec37bff06df0cd04c6193fb/ui/ozone/platform/scenic/scenic_surface.h
[modify] https://crrev.com/6cae8f663715cfc50ec37bff06df0cd04c6193fb/ui/ozone/platform/scenic/scenic_surface_factory.cc
[modify] https://crrev.com/6cae8f663715cfc50ec37bff06df0cd04c6193fb/ui/ozone/platform/scenic/scenic_surface_factory.h
[modify] https://crrev.com/6cae8f663715cfc50ec37bff06df0cd04c6193fb/ui/ozone/platform/scenic/vulkan_implementation_scenic.cc

Comment 4 by spang@chromium.org, Jan 17 (6 days ago)

Status: Fixed (was: Started)

Comment 5 by spang@chromium.org, Jan 17 (5 days ago)

Status: Started (was: Fixed)
I guess we should turn it back on :)
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 17 (5 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b94793dee2f9cca69c41f7f87f6fb4fef648bb3e

commit b94793dee2f9cca69c41f7f87f6fb4fef648bb3e
Author: Michael Spang <spang@chromium.org>
Date: Thu Jan 17 20:07:47 2019

viz: Re-enable VizDisplayCompositor on Fuchsia

This was disabled because software output was crashing. That works now,
so re-enable it.

Bug:  922242 
Test: run_content_shell on fuchsia

Change-Id: I946e7c728002e66603af4d28bdf655f834686875

Reviewed-on: https://chromium-review.googlesource.com/c/1418434
Commit-Queue: Michael Spang <spang@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623811}
[modify] https://crrev.com/b94793dee2f9cca69c41f7f87f6fb4fef648bb3e/components/viz/common/features.cc

Comment 7 by spang@chromium.org, Jan 17 (5 days ago)

Status: Fixed (was: Started)

Sign in to add a comment