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

Issue 772576 link

Starred by 5 users
Status: Assigned
Owner:
ccameron@chromium.org
Cc:
danakj@chromium.org
█ fsam...@chromium.org
rjkroege@chromium.org
vmi...@chromium.org
piman@chromium.org
kylec...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 805774
issue 817830
Blocking:
issue 730193
issue 801419



Sign in to add a comment

viz: Make Mac rendering work with VizDisplayCompositor

Project Member Reported by ccameron@chromium.org, Oct 6 2017 
How things currently work:

A0. Init:

  content::GpuOutputSurfaceMac
    - constructed in GpuProcessTransportFactory::EstablishedGpuChannel
    - its gfx::AcceleratedWidget is set from the ui::Compositor

A1. We get the following callback from the GPU process after swap:

  content::GpuOutputSurfaceMac::OnGpuSwapBuffersCompleted
    - receives the gpu::GpuProcessHostedCALayerTreeParamsMac
    - looks up the ui::AcceleratedWidgetMac from content::GpuOutputSurfaceMac::widget_
      - the widget_ member was passed in GpuProcessTransportFactory::EstablishedGpuChannel
    - the ui::AcceleratedWidgetMac updates its NSView (which it got from RenderWidgetHostViewMac)

This breaks because content::GpuOutputSurfaceMac will live inside the GPU process, not where the relevant NSViews are. Plus, this is sort of hacky.

How things will work:

B0. Init:

  content::GpuOutputSurfaceMac
    - no longer is provided gfx::AcceleratedWidget

B1. We get the following callback from the GPU process after swap:

  content::GpuOutputSurfaceMac::OnGpuSwapBuffersCompleted (calls superclass):
  viz::DisplayOutputSurface::OnGpuSwapBuffersCompleted

B2. Calls the following method with the SwapResult and Mac-specific params:

  viz::DisplayOutputSurface::DidReceiveSwapBuffersAck

B3. This calls into its client_ member's method (passing the Mac-specific swap-ack params):

  viz::OutputSurfaceClient::DidReceiveSwapBuffersAck (abstract, subclass is:)
  viz::Display::DidReceiveSwapBuffersAck

B4. Call a new method added to viz::DisplayClient (passing the Mac-specific swap-ack params):

  viz::DisplayClient::DidReceiveSwapBuffersAck (abstract, relevant subclass is:)
  viz::DirectLayerTreeFrameSink::DidReceiveSwapBuffersAck

B5. Call a new method on the client_ member, cc::LayerTreeFrameSinkClient (passing the Mac-specific swap-ack params):

  cc::LayerTreeFrameSinkClient::DidReceiveSwapBuffersAck (abstract, relevant subclass is:)
  cc::LayerTreeHostImpl::DidReceiveSwapBuffersAck

B6. Call a new method on the client_ member, cc::LayerTreeHostImplClient (passing the Mac-specific swap-ack params):

  cc::LayerTreeHostImplClient::DidReceiveSwapBuffersAckOnImplThread (abstract, relevant subclass is:)
  cc::SingleThreadProxy::DidReceiveSwapBuffersAckOnImplThread

B7. Call a new method on the layer_tree_host_ member

  cc::LayerTreeHost::DidReceiveSwapBuffersAckOnImplThread (passing the Mac-specific swap-ack params):

B8. Call a new method on the client_ member (passing the Mac-specific swap-ack params):

  cc::LayerTreeHostClient::DidReceiveSwapBuffersAckOnImplThread (abstract, relevant subclass is:)
  ui::Compositor::DidReceiveSwapBuffersAckOnImplThread

B9. Now we're back at the ui::Compositor

  We have access to the gfx::AcceleratedWidget here, so we can do what was previously done in
  content::GpuOutputSurfaceMac::OnGpuSwapBuffersCompleted here. The ui::Compositor doesn't have
  its own client interface, so we should probably do this by setting a callback on ui::Compositor
  for it to call when this call comes in.

Comment 1 by ccameron@chromium.org, Oct 6 2017

Cc: danakj@chromium.org fsam...@chromium.org
Components: Internals>Viz
Owner: ccameron@chromium.org

Comment 2 by laforge@google.com, Nov 8 2017

Components: -Internals>Viz Internals>Services>Viz
Migrating from Internals>Viz to Internals>Services>Viz.

Comment 3 by ccameron@chromium.org, Nov 21 2017

Cc: vmi...@chromium.org
I will be starting this soon.

Comment 4 by ccameron@chromium.org, Nov 27 2017 

There's another wrinkle in here -- Mac is using GpuSurfaceTracker and does not have GPU_SURFACE_HANDLE_IS_ACCELERATED_WINDOW defined. That may need to be cleaned up first.

Comment 5 by piman@chromium.org, Nov 28 2017

Cc: piman@chromium.org

Comment 6 by ccameron@chromium.org, Nov 28 2017 

In B4, DirectLayerTreeFrameSink should read RootCompositorFrameSinkImpl.

I was originally going to take the following path in B4-B5
- ... -> RootCompositorFrameSinkImpl ->
- CompositorFrameSinkClient ->
- ClientLayerTreeFrameSink ->
- LayerTreeHostImpl (as LayerTreeFrameSinkClient) -> ...

But ... adding the "got display parameters" interface to all CompositorFrameSinkClients is overkill because we only expect this to come from the RootCompositorFrameSink. We'll be adding a DisplayPrivateClient to allow this callback without having to send it everywhere.

One other thing to mention is that a lot of this plumbing is running parallel to DidPresentCompositorFrame. We are indeed going to keep these as two different calls, since they mean different things and have different parameters.
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 1 2017 

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

commit 212266c053809540bf245ed9247cde2b39fdab57
Author: Christopher Cameron <ccameron@chromium.org>
Date: Fri Dec 01 20:07:43 2017

viz/mac: Remove GpuSurfaceTracker, redefine gfx::AcceleratedWidget

GpuSurfaceTracker has been removed from all other desktop platforms (in
crrev.com/389975) and is not used on Mac. Remove it.

Include Mac in GPU_SURFACE_HANDLE_IS_ACCELERATED_WINDOW with other
desktop platforms.

Define gfx::AcceleratedWidget to be uint64_t instead of NSView*. It
never actually was NSView*, but was rather a sequence number used to
look up a ui::AcceleratedWidgetMac.

Fix a number of locations that were dangerously mixing NSView* and
gfx::AcceleratedWidget (now that they are incompatible types, this is
a compile error).

Bug: 772576
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I29212236627c512ed6a7fe55a8cf3e613e4e593a
Reviewed-on: https://chromium-review.googlesource.com/792295
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521030}
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/components/viz/service/display_embedder/software_output_device_mac_unittest.mm
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/content/browser/accessibility/browser_accessibility_manager_mac.mm
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/gpu/gles2_conform_support/egl/display.cc
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/gpu/ipc/common/BUILD.gn
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/gpu/ipc/common/surface_handle.h
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/ui/accelerated_widget_mac/accelerated_widget_mac.h
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/ui/accelerated_widget_mac/accelerated_widget_mac.mm
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/ui/compositor/test/test_compositor_host_mac.mm
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/ui/gfx/mojo/struct_traits_unittest.cc
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/ui/gfx/native_widget_types.h
[modify] https://crrev.com/212266c053809540bf245ed9247cde2b39fdab57/ui/views/cocoa/bridged_native_widget.mm
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 3 2017 

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

commit 4bc282bbe092ab36b1206a09926ba09775669ea4
Author: Christopher Cameron <ccameron@chromium.org>
Date: Sun Dec 03 11:24:05 2017

viz/mac: Prepare the ground for CoreAnimation plumbing

The parameters for sending a CALayer tree from the GPU process to the
browser process will have to be shuffled a bit for viz. Before doing
this, clean up the existing path.

Current relevant sequence of events:
  * gpu::ImageTransportSurfaceOverlayMac populates
    gpu::SwapBuffersCompleteParams in the GPU process.
      * Structure includes gfx::SwapResponse,
        gpu::TextureInUseResponses, and CALayer parameters.
  * gpu::CommandBuffer un-and-re-packs this structure as
    GpuCommandBufferMsg_SwapBuffersCompleted_Params to send via IPC.
  * gpu::CommandBufferProxyImpl::OnSwapBuffersCompleted then
    un-and-re-packs that structure as two separate arguments
      * gfx::SwapResponse
        * Is consumed on all platforms
      * gpu::GpuProcessHostedCALayerTreeParamsMac
        * Is consumed by GpuOutputSurfaceMac only, passed by pointer
        * Includes gpu::TextureInUseResponses as a member
  * These arguments ultimately resurface in a function named
    OnGpuSwapBuffersCompleted in viz::DisplayOutputSurface or
    content::GpuBrowserCompositorOutputSurface (or subclasses thereof).

The reason for this patch is that, in viz, we will need to split up
gpu::GpuProcessHostedCALayerTreeParamsMac, because different
parts go to different places. In particular:
  * The actual CALayer-related parameters need to go up to the browser
  * The TextureInUseResponses parameter just goes to the GLRenderer
Since we're doing surgery in the neighborhood, some of the
un-and-re-packing can be done away with.

The changes made are:
  * Add a new gfx::CALayerParams structure
      * In gfx because it will be passed into ui code on the browser
        side.
      * Not ifdef-ed out on non-macOS (though is largely depopulated)
        to avoid ifdef-spaghetti (an is_empty parameter will be used
        for early-out)
      * Keep the IOSurface as a mach port much longer, because viz
        will need to re-pass it to the browser side (via mojo).
  * Populate gpu::SwapBuffersCompleteParams with all three
    separately-destined components:
      * gfx::SwapResponse (as before)
      * gpu::TextureInUseResponses (as before)
          * This is not ifdef-ed out even though it is macOS-only
            avoid preprocessor-spaghetti and to help testing.
      * gpu::CALayerParams (new)
          * This was previously just inline
  * Move gpu::SwapBuffersCompleteParams from service into common
      * This is passed without any inspection or modification from
        gpu::ImageTransportSurfaceOverlayMac to *GpuOutputSurfaceMac,
        where its sub-parts finally diverge.
      * This lets us remove the un-and-re-pack of parameters.
  * Delete the redundant structures:
      * GpuCommandBufferMsg_SwapBuffersCompleted_Params
      * gpu::GpuProcessHostedCALayerTreeParamsMac

Bug: 772576
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I8fd06898a4b1f8432b563c0bc63975787b05cb46
Reviewed-on: https://chromium-review.googlesource.com/798642
Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521226}
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/components/viz/service/display_embedder/gl_output_surface.cc
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/components/viz/service/display_embedder/gl_output_surface.h
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/content/browser/compositor/gpu_browser_compositor_output_surface.cc
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/content/browser/compositor/gpu_browser_compositor_output_surface.h
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/content/browser/compositor/gpu_output_surface_mac.h
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/content/browser/compositor/gpu_output_surface_mac.mm
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/content/browser/compositor/gpu_surfaceless_browser_compositor_output_surface.cc
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/content/browser/compositor/gpu_surfaceless_browser_compositor_output_surface.h
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_schedule_ca_layer.txt
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/command_buffer/common/BUILD.gn
[rename] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/command_buffer/common/swap_buffers_complete_params.cc
[add] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/command_buffer/common/swap_buffers_complete_params.h
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/client/BUILD.gn
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/client/command_buffer_proxy_impl.h
[delete] https://crrev.com/69666610041ca9e0293f12fd5f7383025b79ee41/gpu/ipc/client/gpu_process_hosted_ca_layer_tree_params.cc
[delete] https://crrev.com/69666610041ca9e0293f12fd5f7383025b79ee41/gpu/ipc/client/gpu_process_hosted_ca_layer_tree_params.h
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/common/gpu_param_traits_macros.h
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/in_process_command_buffer.h
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/service/BUILD.gn
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/service/direct_composition_surface_win_unittest.cc
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/service/gpu_command_buffer_stub.cc
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/service/image_transport_surface_delegate.h
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/service/image_transport_surface_overlay_mac.mm
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/gpu/ipc/service/pass_through_image_transport_surface.cc
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/ui/gfx/BUILD.gn
[add] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/ui/gfx/ca_layer_params.cc
[add] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/ui/gfx/ca_layer_params.h
[modify] https://crrev.com/4bc282bbe092ab36b1206a09926ba09775669ea4/ui/gfx/ipc/gfx_param_traits_macros.h
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 6 2017 

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

commit b7158c7bb1262e1e5767a6441aa3ac53f26b109a
Author: Christopher Cameron <ccameron@chromium.org>
Date: Wed Dec 06 06:13:38 2017

viz/mac: Add mojo support for CALayerParams

This is currently passed GPU to browser via IPC. Add support for mojo.

In practice, none of the paramters are read on non-macOS platforms.
Remove the ifdef guards from everything but the mach port, because
of preprocessor guards aren't present in mojo.

Bug: 772576
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Icae234c7969ff26a337b025b6ae325e3abc92314
Reviewed-on: https://chromium-review.googlesource.com/809541
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522024}
[modify] https://crrev.com/b7158c7bb1262e1e5767a6441aa3ac53f26b109a/gpu/ipc/service/image_transport_surface_overlay_mac.mm
[modify] https://crrev.com/b7158c7bb1262e1e5767a6441aa3ac53f26b109a/ui/gfx/ca_layer_params.h
[modify] https://crrev.com/b7158c7bb1262e1e5767a6441aa3ac53f26b109a/ui/gfx/mojo/BUILD.gn
[add] https://crrev.com/b7158c7bb1262e1e5767a6441aa3ac53f26b109a/ui/gfx/mojo/ca_layer_params.mojom
[add] https://crrev.com/b7158c7bb1262e1e5767a6441aa3ac53f26b109a/ui/gfx/mojo/ca_layer_params.typemap
[add] https://crrev.com/b7158c7bb1262e1e5767a6441aa3ac53f26b109a/ui/gfx/mojo/ca_layer_params_struct_traits.cc
[add] https://crrev.com/b7158c7bb1262e1e5767a6441aa3ac53f26b109a/ui/gfx/mojo/ca_layer_params_struct_traits.h
[modify] https://crrev.com/b7158c7bb1262e1e5767a6441aa3ac53f26b109a/ui/gfx/typemaps.gni
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 7 2017 

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

commit 6e18590282e1232d6b498878758b17a0ba525968
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Dec 07 19:53:51 2017

viz/mac: Plumb CALayerParams to frame sinks

Add plumbing from:
  viz::GLOutputSurface::OnGpuSwapBuffersCompleted and
  content::GpuOutputSurfaceMac call DidReceiveCALayerParams on:
  viz::OutputSurfaceClient virtual, subclassed as:
  viz::Display calls DisplayDidReceiveCALayerParams:
  viz::DisplayClient virtual, subclassed as:
  viz::DirectLayerTreeFrameSink/RootCompositorFrameSinkImpl
The next step will be to plumb this through to the
ui::AcceleratedWidgetMac.

Add a skeleton GLOutputSurfaceMac.

R=fsamuel
TBR=boliu (webview)

Bug: 772576
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Ic937647335c3748af8f4ce7df655d0a4f218786e
Reviewed-on: https://chromium-review.googlesource.com/807544
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522514}
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/android_webview/browser/surfaces_instance.h
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/cc/test/fake_output_surface_client.h
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/BUILD.gn
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/display/display.cc
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/display/display.h
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/display/display_client.h
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/display/display_unittest.cc
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/display/output_surface_client.h
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/display_embedder/gl_output_surface.cc
[add] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/display_embedder/gl_output_surface_mac.cc
[add] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/display_embedder/gl_output_surface_mac.h
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/display_embedder/gpu_display_provider.cc
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.h
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/test/test_layer_tree_frame_sink.cc
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/components/viz/test/test_layer_tree_frame_sink.h
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/content/browser/compositor/gpu_output_surface_mac.mm
[modify] https://crrev.com/6e18590282e1232d6b498878758b17a0ba525968/content/renderer/android/synchronous_layer_tree_frame_sink.h
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 8 2017 

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

commit 210f24b4c8a63318597b9a5b67e5a471457e5aae
Author: Christopher Cameron <ccameron@chromium.org>
Date: Fri Dec 08 00:39:57 2017

Remove RenderWidgetHostView::GetAcceleratedWidgetMac

This is not called and just confuses matters.

Add more comments to clear up behaviors.

R=tapted
TBR=avi (content/ OWNERship)

Bug: 772576
Change-Id: I38d7a847b07f3f30622c679ec8486a34637d144f
Reviewed-on: https://chromium-review.googlesource.com/812211
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522646}
[modify] https://crrev.com/210f24b4c8a63318597b9a5b67e5a471457e5aae/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/210f24b4c8a63318597b9a5b67e5a471457e5aae/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/210f24b4c8a63318597b9a5b67e5a471457e5aae/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/210f24b4c8a63318597b9a5b67e5a471457e5aae/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/210f24b4c8a63318597b9a5b67e5a471457e5aae/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/210f24b4c8a63318597b9a5b67e5a471457e5aae/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/210f24b4c8a63318597b9a5b67e5a471457e5aae/content/public/browser/render_widget_host_view.h
[modify] https://crrev.com/210f24b4c8a63318597b9a5b67e5a471457e5aae/content/test/test_render_view_host.cc
[modify] https://crrev.com/210f24b4c8a63318597b9a5b67e5a471457e5aae/content/test/test_render_view_host.h
[modify] https://crrev.com/210f24b4c8a63318597b9a5b67e5a471457e5aae/ui/accelerated_widget_mac/accelerated_widget_mac.h
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 8 2017 

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

commit 8de941fbe7b9866f96fb90df4ea982134acc1400
Author: Christopher Cameron <ccameron@chromium.org>
Date: Fri Dec 08 05:54:31 2017

viz/mac: Separate CALayerFrameSink interface from AcceleratedWidgetMac

The goal here is to create a non-objective-C interface through which
viz::OutputSurface and viz::SoftwareOutputDevices can update the
embedding NSView.

Move the remote CoreAnimation API use from GpuOutputSurfaceMac to
AcceleratedWidgetMac.

Also move part of the surface suspension API to AcceleratedWidgetMac,
although the entire surface suspension scheme should be revisited due to
changing substrate.

Separate the functions that are called by viz::OutputSurface and
viz::SoftwareOutputDevice into a CALayerFrameSink API.

Remove AcceleratedWidgetMac::Get, and replace it with
CALayerEmbedder::Get. This means that locations that have the
gfx::AcceleratedWidget handle can now only access the CALayerFrameSink
interface.

Change a couple of mm files to be cc files, now that that is possible.

R=tapted
TBR=avi (content/ OWNERship)

Bug: 772576
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I77890fdab935ab8efd0fc4a8d37d05118fe9f229
Reviewed-on: https://chromium-review.googlesource.com/809780
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522720}
[modify] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/components/viz/service/BUILD.gn
[rename] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/components/viz/service/display_embedder/compositor_overlay_candidate_validator_mac.cc
[rename] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/components/viz/service/display_embedder/software_output_device_mac.cc
[modify] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/content/browser/BUILD.gn
[rename] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/content/browser/compositor/gpu_output_surface_mac.cc
[modify] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/content/browser/compositor/gpu_output_surface_mac.h
[modify] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/ui/accelerated_widget_mac/BUILD.gn
[modify] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/ui/accelerated_widget_mac/accelerated_widget_mac.h
[modify] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/ui/accelerated_widget_mac/accelerated_widget_mac.mm
[add] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/ui/accelerated_widget_mac/ca_layer_frame_sink.h
[add] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/ui/accelerated_widget_mac/ca_layer_frame_sink.mm
[modify] https://crrev.com/8de941fbe7b9866f96fb90df4ea982134acc1400/ui/views/widget/native_widget_mac_unittest.mm
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 14 2017 

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

commit baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Dec 14 23:11:27 2017

viz/mac: Plumb CALayerParams to the browser process under viz

The motivation for this change is a difference in behavior between
macOS and other platforms.

  On non-macOS platforms, once the output surface has been created,
  calling SwapBuffers on the output surface will cause content to appear
  in the window created in the browser process.

  On macOS, SwapBuffers generates parameters that then must be sent back
  to the browser process (more generally, wherever the NSWindow lives),
  in order for the content to be displayed on-screen.

  These parameters are stored in the CALayerParams structure. They are
  consumed by the ui::CALayerFrameSink interface (which, given a
  gfx::AcceleratedWidget and CALayerParams, will show the described
  content in the specified gfx::AcceleratedWidget).

Prior to this change, content::GpuOutputSurfaceMac would reach directly
over to ui::CALayerFrameSink. This doesn't work in viz, because the viz
equivalent (viz::GLOutputSurfaceMac) lives in the GPU process.

This change plumbs these CALayerParams back to the browser process, via
a new priviledged DisplayPrivateClient interface.

Note that creating a new DisplayPrivateClient interface is chosen over
repurposing the existing CompositorFrameSinkClient interface because
CompositorFrameSinkClient is not specific to an output display. The
feedback from the new DisplayPrivateClient will also be used in MacViews
in the future to ensure that NSWindows are not placed on-screen until
they have content available.

A stub DisplayPrivateClient is used by ui::ws::VisHostProxy, which will
need to be fleshed out for mus on macOS.

The DisplayPrivateClient for DirectLayerTreeFrameSink is left as nullptr
on Android (and other locations) for now. This interface should be made
non-optional in the future.

R=sadrul,tsepez,fsamuel
TBR=avi (content/)

Bug: 772576
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Id5e873cf18ef17e86dc26fd61e17705e94274015
Reviewed-on: https://chromium-review.googlesource.com/820630
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524224}
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/host/host_frame_sink_manager_unittest.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.h
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/service/frame_sinks/direct_layer_tree_frame_sink_unittest.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/test/BUILD.gn
[add] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/test/mock_display_client.cc
[add] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/test/mock_display_client.h
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/components/viz/test/test_frame_sink_manager.h
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/content/browser/BUILD.gn
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/content/browser/compositor/gpu_browser_compositor_output_surface.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/content/browser/compositor/gpu_output_surface_mac.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/content/browser/compositor/gpu_output_surface_mac.h
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/content/browser/compositor/gpu_process_transport_factory.cc
[add] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/content/browser/compositor/in_process_display_client.cc
[add] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/content/browser/compositor/in_process_display_client.h
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/content/browser/compositor/viz_process_transport_factory.h
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/services/ui/ws/platform_display_default.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/services/ui/ws/server_window.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/services/ui/ws/server_window.h
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/services/ui/ws/server_window_delegate.h
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/services/ui/ws/window_server.cc
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/services/viz/privileged/interfaces/compositing/display_private.mojom
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/services/viz/privileged/interfaces/compositing/frame_sink_manager.mojom
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/ui/accelerated_widget_mac/ca_layer_tree_coordinator.mm
[modify] https://crrev.com/baa8c41ce1764df4bd4fb9a02d13d0cdfdfeb963/ui/compositor/test/in_process_context_factory.cc
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 19 2017 

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

commit 5b399be5297356288df1a8431f9f42dd05a97490
Author: Christopher Cameron <ccameron@chromium.org>
Date: Tue Dec 19 21:11:30 2017

viz/mac: Share BufferQueue-related code with ozone

Split off from GLOutputSurfaceSurfaceless GLOutputSurfaceOzone, and
make GLOutputSurfaceMac use it.

Specify the overlay processor to GLOutputSurfaceMac.

With this in place, we finally see (the wrong) content on macOS with
viz enabled.

Bug: 772576
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I67e22cce8628e8e348039fe94344eed6033b049b
Reviewed-on: https://chromium-review.googlesource.com/832939
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525141}
[modify] https://crrev.com/5b399be5297356288df1a8431f9f42dd05a97490/components/viz/service/BUILD.gn
[add] https://crrev.com/5b399be5297356288df1a8431f9f42dd05a97490/components/viz/service/display_embedder/gl_output_surface_buffer_queue.cc
[add] https://crrev.com/5b399be5297356288df1a8431f9f42dd05a97490/components/viz/service/display_embedder/gl_output_surface_buffer_queue.h
[modify] https://crrev.com/5b399be5297356288df1a8431f9f42dd05a97490/components/viz/service/display_embedder/gl_output_surface_mac.cc
[modify] https://crrev.com/5b399be5297356288df1a8431f9f42dd05a97490/components/viz/service/display_embedder/gl_output_surface_mac.h
[modify] https://crrev.com/5b399be5297356288df1a8431f9f42dd05a97490/components/viz/service/display_embedder/gl_output_surface_ozone.cc
[modify] https://crrev.com/5b399be5297356288df1a8431f9f42dd05a97490/components/viz/service/display_embedder/gl_output_surface_ozone.h
[modify] https://crrev.com/5b399be5297356288df1a8431f9f42dd05a97490/components/viz/service/display_embedder/gpu_display_provider.cc
Project Member

Comment 16 by bugdroid1@chromium.org, Dec 20 2017 

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

commit 1eecc9e825b1b54fb7577817bb7c69f3e9d9223f
Author: Christopher Cameron <ccameron@chromium.org>
Date: Wed Dec 20 00:12:59 2017

viz/mac: Add surface id for DelegatedFrameHost

This gets web content on-screen

Resizing is very broken on macOS and will require quite some
refactoring to work.

Bug: 772576
Change-Id: Ia7e6378550736fb95220c511d35fdad1fcbd5a6c
Reviewed-on: https://chromium-review.googlesource.com/833351
Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525191}
[modify] https://crrev.com/1eecc9e825b1b54fb7577817bb7c69f3e9d9223f/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/1eecc9e825b1b54fb7577817bb7c69f3e9d9223f/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/1eecc9e825b1b54fb7577817bb7c69f3e9d9223f/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/1eecc9e825b1b54fb7577817bb7c69f3e9d9223f/content/browser/renderer_host/render_widget_host_view_mac.mm
Project Member

Comment 17 by bugdroid1@chromium.org, Dec 21 2017 

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

commit 86b894bbcac3cf12a0e233b068dba7d778d2bc3a
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Dec 21 21:31:58 2017

viz/mac: Fix some window resize bugs

Overload RendererRequestedFrameSize to query the BrowserCompositorMac
instead of retrieving the properties from the NSView directly.

This makes resize no longer crash, but leaves changes to device scale
factor unstable.

Bug: 772576
Change-Id: I3d07adc3e5e66bc74b688d3fe480b55cc2f23c87
Reviewed-on: https://chromium-review.googlesource.com/835630
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525807}
[modify] https://crrev.com/86b894bbcac3cf12a0e233b068dba7d778d2bc3a/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/86b894bbcac3cf12a0e233b068dba7d778d2bc3a/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/86b894bbcac3cf12a0e233b068dba7d778d2bc3a/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/86b894bbcac3cf12a0e233b068dba7d778d2bc3a/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/86b894bbcac3cf12a0e233b068dba7d778d2bc3a/content/browser/renderer_host/render_widget_host_view_mac.mm

Comment 18 by ccameron@chromium.org, Dec 21 2017 

As of the patch in #17, --enable-viz on Mac will show content on screen and not be too crashy.

The remaining issues that I know of are as follows:
1. Allocating the backbuffer in BufferQueue fails
  - ironically, only the CoreAnimation renderer path works
  - I suspect that whatever the issue is, it also affects ozone
2. Device scale factor transitions frequently crash
  - this is super-racy on all platforms and needs to be re-examained
  - WebContentsView*::GetScreenInfo is copy-pasta on all platforms and needs to go
3. Resize isn't lock-step
  - We somehow need to make the browser compositor not draw anything until the renderer
    provides a correctly-sized frame
4. Resize sometimes flickers
  - We may need to think more about the lifetime our remote layer CAContexts
5. Suspend for recycle behavior is broken
  - This is still NOTIMPLEMENTED
  - I have some thoughts on how to solve this an  issue 4  together
  - TLDR is keep the CALayerTree around in the browser and the CAContext around in
    the gpu proc.
6. Lots of console spam about IOSurface mach ports
  - probably not a big deal, but haven't looked into it

So it's still a fair chunk of work until this is like-ship-able. And those are just the /known/ issues.

Comment 19 by ccameron@chromium.org, Dec 21 2017

Blocking: 601863

Comment 20 by fsam...@chromium.org, Dec 21 2017

Cc: rjkroege@chromium.org
1. +rjkreoge@

2. All that matters is whenever device scale factor changes, we allocate a new LocalSurfaceId. RenderWidgetHostImpl::WasResized will grab all the synchronized properties from RenderWidgetHostView.

3. The ResizeHelper needs to be updated to wait for OnFirstSurfaceActivation (making sure the ResizeParams do arrive to the renderer too).

4. Hmm, are you seeing any missing surface errors in SurfaceAggregator?

Comment 21 by ccameron@chromium.org, Dec 21 2017 

WRT the device scale factor stuff, the main issue is that
- We need (LocalSurfaceId, SizeDIPs, DSF) to be updated atomically
  - SizeDIPs and DSF are static properties of the LocalSurfaceId
- IMO we should query (LocalSurfaceId, SizeDIPs, DSF) atomically together from
  the same thing that ultimately created the LocalSurfaceId.
- Currently we don't quite do that -- in particular, in RWHImpl::GetResizeParams...
  - we pull LocalSurfaceId and SizeDIPS from the RWHV
  - we pull DSF from ScreenInfo (which comes from WebContentsView)
- Because we pull these properties from two different places, we can end up with
  synchronization and ordering bugs between the two places
  - Aura appears to do in the right order, though it's not obvious
  - Mac has a few ways for DSF to be updated, which makes this harder

It may be that the root issue is that ResizeParams is sending two pieces of information
A. parameters for how the renderer-compositor should create is current frame
B. capabilities info to send to WebScreenInfo to expose to the web
When the screen properties change, we want to update B. But we only want to update A when the WindowTreeHost has digested all of these changes.

> 3. The ResizeHelper needs to be updated to wait for OnFirstSurfaceActivation (making sure the ResizeParams do arrive to the renderer too).

Nice -- that should be fairly easy!

> 4. Hmm, are you seeing any missing surface errors in SurfaceAggregator?

Not yet -- IIUC this is a macOS-specific badness...

Comment 22 by kylec...@chromium.org, Jan 2 2018

Cc: kylec...@chromium.org

Comment 23 by ccameron@chromium.org, Jan 12 2018

Blocking: 801419

Comment 24 by ccameron@chromium.org, Jan 26 2018

Blockedon: 805774
Project Member

Comment 25 by bugdroid1@chromium.org, Jan 28 2018 

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

commit 7ece806c6957e937a5070172b51ebf03746cc68d
Author: Christopher Cameron <ccameron@chromium.org>
Date: Sun Jan 28 07:58:51 2018

viz/mac: Add plumbing for GL rendering on mac

Plumb the flag --disable-mac-overlays.

Update CALayer parameters to make the IOSurfaces CAContextID mutually
exclusive (otherwise we get warned that the IOSurface mach port is
invalid, when it is expected to be invalid).

R=dcheng, fsamuel
TBR=avi (content/)

Bug: 772576
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Iaa2f2fdfb6dbb4135bd989f4c2af1ab6be892a1a
Reviewed-on: https://chromium-review.googlesource.com/882316
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532269}
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/components/viz/common/display/renderer_settings.h
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/components/viz/host/renderer_settings_creation.cc
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/components/viz/service/display_embedder/gl_output_surface_mac.cc
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/components/viz/service/display_embedder/gl_output_surface_mac.h
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/components/viz/service/display_embedder/gpu_display_provider.cc
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/services/viz/privileged/interfaces/compositing/renderer_settings.mojom
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/services/viz/privileged/interfaces/compositing/renderer_settings_struct_traits.cc
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/services/viz/privileged/interfaces/compositing/renderer_settings_struct_traits.h
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/ui/gfx/mojo/ca_layer_params.mojom
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/ui/gfx/mojo/ca_layer_params_struct_traits.cc
[modify] https://crrev.com/7ece806c6957e937a5070172b51ebf03746cc68d/ui/gfx/mojo/ca_layer_params_struct_traits.h
Project Member

Comment 26 by bugdroid1@chromium.org, Jan 28 2018 

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

commit 9656d1f10bdf71d5a72fe9bcdb26664a6a361ddc
Author: Christopher Cameron <ccameron@chromium.org>
Date: Sun Jan 28 09:23:19 2018

viz/mac: Fix InProcessGpuMemoryBufferManager client id

InProcessCommandBuffer::CreateImageOnGpuThread assumes that its client
id is zero, but InProcessGpuMemoryBufferManager uses the constant one.

Put the constant in a InProcessCommandBuffer to keep them the same.

Bug: 772576
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I18f48dff252af04a438e09382277f0a5c7208d44
Reviewed-on: https://chromium-review.googlesource.com/882442
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532271}
[modify] https://crrev.com/9656d1f10bdf71d5a72fe9bcdb26664a6a361ddc/components/viz/service/display_embedder/in_process_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/9656d1f10bdf71d5a72fe9bcdb26664a6a361ddc/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/9656d1f10bdf71d5a72fe9bcdb26664a6a361ddc/gpu/ipc/in_process_command_buffer.h
[modify] https://crrev.com/9656d1f10bdf71d5a72fe9bcdb26664a6a361ddc/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.cc

Comment 27 by ccameron@chromium.org, Jan 30 2018 

Status update from #18:

There are patches landed or in flight which should cover most of these

> 1. Allocating the backbuffer in BufferQueue fails

Fixed, but I still see blank frames sometimes in the non-CoreAnimation path -- needs more examining.

> 2. Device scale factor transitions frequently crash

This doesn't crash anymore, but I still feel the logic harder to verify than I'd like.

> 3. Resize isn't lock-step

Fixed.

> 4. Resize sometimes flickers

Appears fixed.

> 5. Suspend for recycle behavior is broken

Still NOTIMPLEMENTED, needs examining.

> 6. Lots of console spam about IOSurface mach ports

Fixed.
Project Member

Comment 28 by bugdroid1@chromium.org, Feb 1 2018 

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

commit e64540177d347e0b1d5c334decd29fc815f0fa84
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Feb 01 23:17:09 2018

viz/mac: Update SetShowPrimarySurface after navigation

After navigation, the surface id changes, but no call is made to
SetShowPrimarySurface, to tell the ui::Compositor to draw this
surface. Add a call to DelegatedFrameHost::WasResized to cause
this to happen.

Bug: 772576
Change-Id: I33e1fd509e3666f07a4f9a86aa600dc54adac733
Reviewed-on: https://chromium-review.googlesource.com/894197
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533847}
[modify] https://crrev.com/e64540177d347e0b1d5c334decd29fc815f0fa84/content/browser/renderer_host/browser_compositor_view_mac.mm

Comment 29 by kylec...@chromium.org, Feb 7 2018

Blocking: 787097
Project Member

Comment 30 by bugdroid1@chromium.org, Feb 8 2018 

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

commit e086c175bea7923d7e85e7ae1fa581a3ce7225e8
Author: kylechar <kylechar@chromium.org>
Date: Thu Feb 08 18:36:20 2018

viz: Send vsync parameters from browser to gpu.

On macOS the vsync signal is received in the browser process. When
running with VizDisplayCompositor send IPC with vsync paramaters to the
BeginFrameSource in the GPU process.

Bug:  772524 , 772576
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I22d2036bdfb6589ba3342fa4972b670d105376ec
Reviewed-on: https://chromium-review.googlesource.com/907597
Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535451}
[modify] https://crrev.com/e086c175bea7923d7e85e7ae1fa581a3ce7225e8/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/e086c175bea7923d7e85e7ae1fa581a3ce7225e8/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h
[modify] https://crrev.com/e086c175bea7923d7e85e7ae1fa581a3ce7225e8/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/e086c175bea7923d7e85e7ae1fa581a3ce7225e8/services/viz/privileged/interfaces/compositing/display_private.mojom

Comment 31 by kylec...@chromium.org, Feb 8 2018

Summary: viz: Make Mac rendering work with VizDisplayCompositor (was: viz: Make Mac rendering work in viz)

Comment 32 by kylec...@chromium.org, Feb 9 2018

Blocking: -601863
Project Member

Comment 33 by bugdroid1@chromium.org, Feb 13 2018 

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

commit 0158ac8bd270408aa9f3b44e19524c5f8d54cb97
Author: Christopher Cameron <ccameron@chromium.org>
Date: Tue Feb 13 19:59:45 2018

viz/mac: Make resize awesome

Resize uses a nested run loop on macOS. The TaskRunner for that nested
loop is passed everywhere, but needed to be hooked up in a couple of
places for viz.

With this patch NSWindow resize is done in smooth (well, if we can
raster fast enough) lock-step with web content.

R=fsamuel
TBR=tapted

Bug: 772576
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I7ea42757b15d1d0b4d8f9f9cf3a37c1a6dd21e2a
Reviewed-on: https://chromium-review.googlesource.com/909788
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536430}
[modify] https://crrev.com/0158ac8bd270408aa9f3b44e19524c5f8d54cb97/chrome/browser/ui/browser_browsertest.cc
[modify] https://crrev.com/0158ac8bd270408aa9f3b44e19524c5f8d54cb97/components/viz/client/client_layer_tree_frame_sink.cc
[modify] https://crrev.com/0158ac8bd270408aa9f3b44e19524c5f8d54cb97/content/browser/compositor/in_process_display_client.cc
[modify] https://crrev.com/0158ac8bd270408aa9f3b44e19524c5f8d54cb97/content/browser/compositor/in_process_display_client.h
[modify] https://crrev.com/0158ac8bd270408aa9f3b44e19524c5f8d54cb97/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/0158ac8bd270408aa9f3b44e19524c5f8d54cb97/content/browser/renderer_host/render_widget_host_impl.cc

Comment 34 by ccameron@chromium.org, Feb 16 2018 

With r537240 landed, I think we can enable this again.
Project Member

Comment 35 by bugdroid1@chromium.org, Feb 19 2018 

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

commit 5a89487894c5c141e32bd716d72ce4e0f63800d4
Author: Christopher Cameron <ccameron@chromium.org>
Date: Mon Feb 19 09:03:23 2018

mac: Don't pause for frames with no frame sink

This should not have been removed in https://crrev.com/536430

Move tracking from RenderWidgetHostImpl (where it is tracked only in
non-viz mode) to BrowserCompositorView, where it is tracked in both
modes.

Bug: 772576,  812505 ,  812156 
Change-Id: I006c1c78f2dc148602f08253745993f8a57a7e16
Reviewed-on: https://chromium-review.googlesource.com/923379
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537604}
[modify] https://crrev.com/5a89487894c5c141e32bd716d72ce4e0f63800d4/content/browser/renderer_host/browser_compositor_view_mac.mm
Project Member

Comment 36 by bugdroid1@chromium.org, Feb 21 2018 

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

commit 3f04bdbcb9a94506f9099dce92a15334911d19e6
Author: Christopher Cameron <ccameron@chromium.org>
Date: Wed Feb 21 02:48:25 2018

content: Use a consistent device scale factor

There are lots of places that want to get the device scale factor for
a RenderWidgetHostView.

When we want this, we will variously call...
- ui::GetScaleFactorForNativeView
- content::GetScaleFactorForView
- content::RenderWidgetHostView::GetScreenInfo()'s device_scale_factor

When transitioning scale factors, race conditions can result in
content::RenderWidgetHostView::GetScreenInfo returning different values
than the first two of these, causing (eventually) surface invariant
violations.

To fix this, do the following
- Add a method RenderWidgetHostView::GetDeviceScaleFactor, which calls
  GetScreenInfo and returns its device_scale_factor.
- Change content::GetScaleFactorForView to return
  RenderWidgetHostView::GetDeviceScaleFactor (and handle nullptr).
- Remove calls to ui::GetScaleFactorForNativeView from all of
  src/content/...  and most of src/chrome/..., and replace them with
  an appropriate substitute.

While we're in the neighborhood, unify layout and layout_mac (don't
know why they were different, but can't imagine that was intentional).

R=piman
TBR=sky

Bug: 772576
Change-Id: I911df7767ef25a6d4bebf2acabd300356080bb87
Reviewed-on: https://chromium-review.googlesource.com/924696
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538007}
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/chrome/browser/extensions/api/downloads/downloads_api.cc
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/chrome/browser/thumbnails/thumbnail_tab_helper.cc
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/content/browser/media/capture/web_contents_video_capture_device.cc
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/content/browser/renderer_host/dip_util.cc
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/content/browser/renderer_host/dip_util.h
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/content/browser/renderer_host/display_util.cc
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/content/public/browser/render_widget_host_view.h
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/ui/base/BUILD.gn
[modify] https://crrev.com/3f04bdbcb9a94506f9099dce92a15334911d19e6/ui/base/layout.cc
[delete] https://crrev.com/9cca25a09c19d84ae9be5b25eadff5f4faf72141/ui/base/layout_mac.mm
Project Member

Comment 37 by bugdroid1@chromium.org, Feb 22 2018 

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

commit 3505eac7be4fc60480e934c160d3c9c867930ebb
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Feb 22 10:00:24 2018

viz/mac: Don't crash at startup and shutdown

At startup...
  Remove the DCHECK that the current surface be offscreen. This check
was added in crrev.com/262594, which removed support for the onscreen
surfaces from macOS. At that time, onscreen surfaces had been backed by
NSOpenGLContexts (and might have even used NSOpenGLViews for surfaces)
and weren't to be trusted for virtualization. All macOS contexts are now
CGLContexts and the offscreen/onscreen distinction of surfaces is
largely irrelevant.

At shutdown...
  In the very similar function, GLES2DecoderImpl::Destroy, the order
of destroying the GLSurface and GLContext are reversed, and there exists
the comment "Destroy the surface before the context, some surface
destructors make GL calls." The same thing needs to be done here, for
the same reason. Also ensure that the context is re-made current in case
tearing down the decoder changed the current context.

Bug: 772576
Change-Id: I2f29e235e1c2cd587f78a7b24d21451fbda4f1a9
Reviewed-on: https://chromium-review.googlesource.com/929894
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538387}
[modify] https://crrev.com/3505eac7be4fc60480e934c160d3c9c867930ebb/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/3505eac7be4fc60480e934c160d3c9c867930ebb/ui/gl/init/gl_factory_mac.cc

Comment 38 by ccameron@chromium.org, Mar 2 2018

Blockedon: 817830
Project Member

Comment 39 by bugdroid1@chromium.org, Mar 2 2018 

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

commit 97027229740c6be2712c4902376fd7e1fbaa6fd4
Author: kylechar <kylechar@chromium.org>
Date: Fri Mar 02 22:08:33 2018

Cleanup GpuDisplayProvider.

Move everything software compositing related into one function.

Bug: 772576
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I110c9dd06b5e9a90a6667501aa2945c383a670ca
Reviewed-on: https://chromium-review.googlesource.com/943723
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540653}
[modify] https://crrev.com/97027229740c6be2712c4902376fd7e1fbaa6fd4/components/viz/service/display_embedder/gpu_display_provider.cc
[modify] https://crrev.com/97027229740c6be2712c4902376fd7e1fbaa6fd4/components/viz/service/display_embedder/gpu_display_provider.h

Comment 40 by kylec...@chromium.org, Apr 23 2018

Blocking: -787097 730193
Project Member

Comment 41 by bugdroid1@chromium.org, Apr 27 2018 

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

commit 17d950a2b6cf3ffce02772d89c3ab701241e0d89
Author: Christopher Cameron <ccameron@chromium.org>
Date: Fri Apr 27 13:32:26 2018

viz/mac: Remove the concept of suspending a ui::Compositor

Because we re-use the same ui::Compositor for several different
RenderWidgetHostViewMacs, we can encounter a situation where the
following sequence of events happens
1. RWHVMac A draws frame A
2. The GLRenderer does the SwapBuffers for frame A
3. RWHVMac A is hidden, and its ui::Compositor/AcceleratedWidgetMac
   are put up for recycling
4. RWHVMac B is shown (in a new window or a new tab), and takes the
   AcceleratedWidgetMac that was recycled by RWHVMac A
5. The CALayerParams from the SwapBuffers gets to the browser, and
   causes the frame from RWHVMac A to appear in RWHVMac B, which is
   incorrect.
This problem has a long history, and many of its original constraints
no longer apply.

In viz this becomes more complicated because the timing of Step 2
becomes much more unpredictable (it can happen as late as after Step 4).

Remove the methods which are used to skip drawing the framein Step 5
above, and replace them with a direct call Suspend on the
AcceleratedWidgetMac, telling it to stop showing new frames until the
ui::Compositor does a commit (which it will do because of the new
SurfaceId that it will have).

With respect to viz
- The old interfaces did not solve the problem for viz (when the
  GLRenderer is in the GPU process), but added lots of complexity.
- The new simpler scheme also does not solve this problem completely
  for viz.
- It is likely that the concept of ui::Compositor recycling will be
  removed entirely before viz ships, making this a moot point.

R=fsamuel
TBR=yfriedman (content/renderer/android), boliu (android_webview/)

Bug: 772576
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ia41712f0730947e6f0426aeea5064c818a16f122
Reviewed-on: https://chromium-review.googlesource.com/1031654
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554370}
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/android_webview/browser/parent_output_surface.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/android_webview/browser/parent_output_surface.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/cc/test/pixel_test_output_surface.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/cc/test/pixel_test_output_surface.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/display/display.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/display/gl_renderer_unittest.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/display/output_surface.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/display/overlay_unittest.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/display_embedder/gl_output_surface.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/display_embedder/gl_output_surface.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/display_embedder/skia_output_surface_impl.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/display_embedder/skia_output_surface_impl.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/display_embedder/software_output_device_mac.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/display_embedder/software_output_surface.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/display_embedder/software_output_surface.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/service/frame_sinks/direct_layer_tree_frame_sink_unittest.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/test/fake_output_surface.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/components/viz/test/fake_output_surface.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/browser_compositor_output_surface.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/gpu_browser_compositor_output_surface.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/gpu_browser_compositor_output_surface.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/gpu_output_surface_mac.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/gpu_output_surface_mac.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/image_transport_factory.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/offscreen_browser_compositor_output_surface.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/offscreen_browser_compositor_output_surface.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/reflector_impl_unittest.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/software_browser_compositor_output_surface.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/software_browser_compositor_output_surface.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/test/test_image_transport_factory.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/viz_process_transport_factory.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/vulkan_browser_compositor_output_surface.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/compositor/vulkan_browser_compositor_output_surface.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/content/renderer/android/synchronous_layer_tree_frame_sink.cc
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/ui/accelerated_widget_mac/accelerated_widget_mac.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/ui/accelerated_widget_mac/accelerated_widget_mac.mm
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/ui/accelerated_widget_mac/ca_layer_frame_sink.h
[modify] https://crrev.com/17d950a2b6cf3ffce02772d89c3ab701241e0d89/ui/compositor/test/in_process_context_factory.cc
Project Member

Comment 42 by bugdroid1@chromium.org, May 5 2018 

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

commit acc082799b19bdda6fd257f96cefd662fddccdab
Author: Christopher Cameron <ccameron@chromium.org>
Date: Sat May 05 15:08:37 2018

viz/mac: Enable software compositing

There exists plumbing to send gfx::CALayerParams (the parameters for
displaying content on macOS) from the viz::Display to the browser
process via that viz::DisplayClient interface.

In GPU compositing, this comes in via viz::Display's implementation of
viz::OutputSurfaceClient::DidReceiveCALayerParams.

Make viz::Display also implement a new interface,
viz::SoftwareOutputDeviceClient, and add a method to this interface,
SoftwareDeviceUpdatedCALayerParams, which the SoftwareOutputDeviceMac
calls when a frame is drawn. Route this to the same paths that the
GPU compositing path takes to send it to the browser process.

Delete the now-unneeded gfx::AcceleratedWidgetMac interfaces in
SoftwareOutputDeviceMac.

Bug:  730660 , 772576
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I03a21c5b1c1e5333aec94b5a0d1e88811fbbaf34
Reviewed-on: https://chromium-review.googlesource.com/1042312
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556332}
[modify] https://crrev.com/acc082799b19bdda6fd257f96cefd662fddccdab/components/viz/service/display/display.cc
[modify] https://crrev.com/acc082799b19bdda6fd257f96cefd662fddccdab/components/viz/service/display/display.h
[modify] https://crrev.com/acc082799b19bdda6fd257f96cefd662fddccdab/components/viz/service/display/software_output_device.cc
[modify] https://crrev.com/acc082799b19bdda6fd257f96cefd662fddccdab/components/viz/service/display/software_output_device.h
[add] https://crrev.com/acc082799b19bdda6fd257f96cefd662fddccdab/components/viz/service/display/software_output_device_client.h
[modify] https://crrev.com/acc082799b19bdda6fd257f96cefd662fddccdab/components/viz/service/display_embedder/gpu_display_provider.cc
[modify] https://crrev.com/acc082799b19bdda6fd257f96cefd662fddccdab/components/viz/service/display_embedder/software_output_device_mac.cc
[modify] https://crrev.com/acc082799b19bdda6fd257f96cefd662fddccdab/components/viz/service/display_embedder/software_output_device_mac.h
[modify] https://crrev.com/acc082799b19bdda6fd257f96cefd662fddccdab/components/viz/service/display_embedder/software_output_device_mac_unittest.mm
[modify] https://crrev.com/acc082799b19bdda6fd257f96cefd662fddccdab/content/browser/compositor/gpu_process_transport_factory.cc
Project Member

Comment 43 by bugdroid1@chromium.org, May 5 2018 

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

commit 0868b9fba1a5345f77eaddee52bd93d693fbe629
Author: Christopher Cameron <ccameron@chromium.org>
Date: Sat May 05 19:10:35 2018

viz/mac: Fix surface invariants violation by recycled compositors

RecyclableCompositorMac owns a ui::Compositor, but the surface id
generation was owned by BrowserCompositorMac.

A RecyclableCompositorMac will be recycled by many different
BrowserCompositorMac, and as a result, it may have a non-monotonic-
increasing surface id.

Scope the surface id information to RecyclableCompositorMac, and
throw some of the related logic into a helper function.

Bug: 772576,  817827 
Change-Id: Ic4299afa5670c96b51260917a3fafb2b692b3465
Reviewed-on: https://chromium-review.googlesource.com/1046045
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556342}
[modify] https://crrev.com/0868b9fba1a5345f77eaddee52bd93d693fbe629/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/0868b9fba1a5345f77eaddee52bd93d693fbe629/content/browser/renderer_host/browser_compositor_view_mac.mm
Project Member

Comment 44 by bugdroid1@chromium.org, May 9 2018 

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

commit 78ad7b11608330e2b3d7344b4ab5493b641b6c1a
Author: Christopher Cameron <ccameron@chromium.org>
Date: Wed May 09 18:25:01 2018

viz/mac: Add an explicit GLSurface clean-up call

Add an explicit method on GLSurface that is to be called while the
GLContext is still current, but before the GLSurface is destroyed.

A step in this direction was taken in https://crrev.com/538387, where
we made the GLContext current again after its decoder had been
destroyed, but that path has proved unstable. In particular, the
GLSurface internals on macOS are created while the decoder is active,
and so destroying them after the decoder is destroyed is precarious.

Bug: 772576,  817830 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I3ff1c395c4e9d7ac0b0f1ab54400cde28bdbe298
Reviewed-on: https://chromium-review.googlesource.com/1046046
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557250}
[modify] https://crrev.com/78ad7b11608330e2b3d7344b4ab5493b641b6c1a/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/78ad7b11608330e2b3d7344b4ab5493b641b6c1a/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/78ad7b11608330e2b3d7344b4ab5493b641b6c1a/gpu/ipc/service/image_transport_surface_overlay_mac.h
[modify] https://crrev.com/78ad7b11608330e2b3d7344b4ab5493b641b6c1a/gpu/ipc/service/image_transport_surface_overlay_mac.mm
[modify] https://crrev.com/78ad7b11608330e2b3d7344b4ab5493b641b6c1a/ui/gl/gl_surface.cc
[modify] https://crrev.com/78ad7b11608330e2b3d7344b4ab5493b641b6c1a/ui/gl/gl_surface.h

Comment 45 by ccameron@chromium.org, Jun 5 2018 

This has regressed severely in the last couple of weeks -- tracking new issues in  issue 849478  -- most looks fairly simple
Project Member

Comment 46 by bugdroid1@chromium.org, Jul 23 

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

commit e8df742a7fd06d904c08bdd027dd4fa8b90a065f
Author: Jochen Eisinger <jochen@chromium.org>
Date: Mon Jul 23 19:25:41 2018

Reenable BrowserTest.WindowOpenClose3 on Mac

We no longer should wait for frames in tests, so this should pass again

R=ccameron@chromium.org
BUG=772576,753691

Change-Id: I9ff676c05fff3fa2d79e1a22edb4ec25ef5e5806
Reviewed-on: https://chromium-review.googlesource.com/1146191
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577219}
[modify] https://crrev.com/e8df742a7fd06d904c08bdd027dd4fa8b90a065f/chrome/browser/ui/browser_browsertest.cc
►

Sign in to add a comment