exo: SurfaceFactoryOwner is really an ExoCompositorFrameSink and should use the MojoCompositorFrameSink interface. |
||||||||||||||||||
Issue descriptionThis bug tracks a bunch of exo compositing refactors for Mus+Ash: 1. SurfaceFactoryOwner moves to its own file and becomes the "ExoCompositorFrameSink". 2. "ExoCompositorFrameSink" moves behind a mojo interface. 3. "ExoCompositorFrameSink" should not know about exo::Surface. Someday, in a slightly more distant future, all CompositorFrameSinks will be unified and we won't need another CompositorFrameSink that does almost the same thing. We first need to make them all look the same (and of course do the same thign) to do that so this is a good intermediate step.
,
Nov 10 2016
,
Nov 10 2016
,
Nov 16 2016
Background for exo folks: We would like to move the display compositor to the gpu process for Poliwog and then later Salamander to support Vulkan, centralized scheduling, centralized animation, scrolling, etc. Thus, SurfaceFactory, SurfaceManager and related classes are all moving to the Gpu process. Exo and other clients cannot access SurfaceFactory directly. They will talk to the display compositor via what's called today "MojoCompositorFrameSink". GpuCompositorFrameSink is a good example of this. As an intermediate step for exo, we should access to SurfaceFactory to an "ExoCompositorFrameSink" or some such name, put it behind a mojo interface, so that exo does not directly talk to SurfaceFactory, then unify it with other "CompositorFrameSink" implementations when they all converge.
,
Dec 1 2016
,
Dec 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3f6957ecf9d02fd3b022a9c3709e05bd51ceeeaf commit 3f6957ecf9d02fd3b022a9c3709e05bd51ceeeaf Author: fsamuel <fsamuel@chromium.org> Date: Thu Dec 01 20:08:40 2016 cc mojo: Serialize/deserialize missed field in TextureDrawQuads resource_size_in_pixels was missed in TextureDrawQuad. This was missed because unit tests used SetNew instead of SetAll and only set all set the resource_size_in_pixels. This CL adds the missing field and updates the corresponding test. BUG= 659601 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2541403002 Cr-Commit-Position: refs/heads/master@{#435686} [modify] https://crrev.com/3f6957ecf9d02fd3b022a9c3709e05bd51ceeeaf/cc/ipc/quads.mojom [modify] https://crrev.com/3f6957ecf9d02fd3b022a9c3709e05bd51ceeeaf/cc/ipc/quads_struct_traits.cc [modify] https://crrev.com/3f6957ecf9d02fd3b022a9c3709e05bd51ceeeaf/cc/ipc/quads_struct_traits.h [modify] https://crrev.com/3f6957ecf9d02fd3b022a9c3709e05bd51ceeeaf/cc/ipc/struct_traits_unittest.cc
,
Dec 1 2016
,
Dec 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7fe38563b35eee5ee8818232d1ad15530ab53fa7 commit 7fe38563b35eee5ee8818232d1ad15530ab53fa7 Author: staraz <staraz@chromium.org> Date: Sat Dec 03 01:16:52 2016 Added MojoCompositorFrameSink::EvictFrame() and MojoCompositorFrameSinkClient::WillDrawSurface() This is a first step to separate exo::SurfaceFactoryOwner into exo::CompositorFrameSink and exo::CompositorFrameSinkHolder (WIP CL: https://codereview.chromium.org/2493223002) BUG= 659601 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2544103002 Cr-Commit-Position: refs/heads/master@{#436117} [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/cc/ipc/mojo_compositor_frame_sink.mojom [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.h [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/services/ui/public/cpp/window_compositor_frame_sink.cc [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/services/ui/public/cpp/window_compositor_frame_sink.h [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/services/ui/surfaces/gpu_compositor_frame_sink.cc [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/services/ui/surfaces/gpu_compositor_frame_sink.h [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/services/ui/ws/frame_generator.cc [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/services/ui/ws/frame_generator.h [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.h [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/ui/aura/mus/window_compositor_frame_sink.cc [modify] https://crrev.com/7fe38563b35eee5ee8818232d1ad15530ab53fa7/ui/aura/mus/window_compositor_frame_sink.h
,
Dec 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/105c120724c4d7d0e8c89dfcefb20e847f2a2c4c commit 105c120724c4d7d0e8c89dfcefb20e847f2a2c4c Author: staraz <staraz@chromium.org> Date: Mon Dec 05 16:45:55 2016 Remove two NOTIMPLEMENTED() added in CL 2544103002 to reduce console spam BUG= 659601 TBR=msw@chromium.org, sadrul@chromium.org Review-Url: https://codereview.chromium.org/2548383002 Cr-Commit-Position: refs/heads/master@{#436322} [modify] https://crrev.com/105c120724c4d7d0e8c89dfcefb20e847f2a2c4c/services/ui/public/cpp/window_compositor_frame_sink.cc [modify] https://crrev.com/105c120724c4d7d0e8c89dfcefb20e847f2a2c4c/services/ui/ws/frame_generator.cc [modify] https://crrev.com/105c120724c4d7d0e8c89dfcefb20e847f2a2c4c/ui/aura/mus/window_compositor_frame_sink.cc
,
Dec 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e6b29c494898402e221db3ff490d219ca0bab9fe commit e6b29c494898402e221db3ff490d219ca0bab9fe Author: staraz <staraz@chromium.org> Date: Wed Dec 07 01:03:45 2016 Add Satisfy() and Require() to MojoCompositorFrameSink Sometimes Wayland client requires a surface before it is created by SubmitCompositorFrame() since the request and creation use different message pipes. This CL moves the implementation of Require and Satisfy to CompositorFrameSink so that they share one message pipe with SubmitCompositorFrame and things would happen in the correct order. This is a temporary fix. kylechar@'s change of surface reference (CL 2541683004) would formally address this issue. BUG= 659601 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2551083003 Cr-Commit-Position: refs/heads/master@{#436806} [modify] https://crrev.com/e6b29c494898402e221db3ff490d219ca0bab9fe/cc/ipc/mojo_compositor_frame_sink.mojom [modify] https://crrev.com/e6b29c494898402e221db3ff490d219ca0bab9fe/cc/surfaces/compositor_frame_sink_support.cc [modify] https://crrev.com/e6b29c494898402e221db3ff490d219ca0bab9fe/cc/surfaces/compositor_frame_sink_support.h [modify] https://crrev.com/e6b29c494898402e221db3ff490d219ca0bab9fe/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc [modify] https://crrev.com/e6b29c494898402e221db3ff490d219ca0bab9fe/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.h [modify] https://crrev.com/e6b29c494898402e221db3ff490d219ca0bab9fe/services/ui/surfaces/gpu_compositor_frame_sink.cc [modify] https://crrev.com/e6b29c494898402e221db3ff490d219ca0bab9fe/services/ui/surfaces/gpu_compositor_frame_sink.h
,
Dec 9 2016
,
Dec 9 2016
,
Dec 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/87d78f2736ef5cd0e45387edee49b5e9c922d6cb commit 87d78f2736ef5cd0e45387edee49b5e9c922d6cb Author: sadrul <sadrul@chromium.org> Date: Sat Dec 10 04:45:47 2016 Revert of Change exo::SurfaceFactoryOwner to exo::ExoCompositorFrameSink (patchset #54 id:1050001 of https://codereview.chromium.org/2493223002/ ) Reason for revert: Breaks test (DesktopMediaListAshTest.ScreenOnly) in chromeos-ozone builds. Original issue's description: > Moved exo::SurfaceFactoryOwner to its own file and renamed it to exo::CompositorFrameSink. > > CompositorFrameSink implements cc::mojom::MojoCompositorFrameSink. > > CompositorFrameSink is no longer a friend class of exo::Surface. > > Added exo::CompositorFrameSinkHolder class that implements cc::mojom::MojoCompositorFrameSinkClient. > > BUG= 659601 > CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel > > Review-Url: https://codereview.chromium.org/2493223002 TBR=reveman@chromium.org,fsamuel@chromium.org,jbauman@chromium.org,sky@chromium.org,staraz@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 659601 Review-Url: https://codereview.chromium.org/2566813002 Cr-Commit-Position: refs/heads/master@{#437753} [modify] https://crrev.com/87d78f2736ef5cd0e45387edee49b5e9c922d6cb/ash/test/ash_test_helper.cc [modify] https://crrev.com/87d78f2736ef5cd0e45387edee49b5e9c922d6cb/chrome/browser/media/webrtc/desktop_media_list_ash_unittest.cc [modify] https://crrev.com/87d78f2736ef5cd0e45387edee49b5e9c922d6cb/components/exo/BUILD.gn [modify] https://crrev.com/87d78f2736ef5cd0e45387edee49b5e9c922d6cb/components/exo/DEPS [delete] https://crrev.com/3d1922bf7e1a53616f82c630eb6c0958eb574386/components/exo/compositor_frame_sink.cc [delete] https://crrev.com/3d1922bf7e1a53616f82c630eb6c0958eb574386/components/exo/compositor_frame_sink.h [delete] https://crrev.com/3d1922bf7e1a53616f82c630eb6c0958eb574386/components/exo/compositor_frame_sink_holder.cc [delete] https://crrev.com/3d1922bf7e1a53616f82c630eb6c0958eb574386/components/exo/compositor_frame_sink_holder.h [modify] https://crrev.com/87d78f2736ef5cd0e45387edee49b5e9c922d6cb/components/exo/surface.cc [modify] https://crrev.com/87d78f2736ef5cd0e45387edee49b5e9c922d6cb/components/exo/surface.h [modify] https://crrev.com/87d78f2736ef5cd0e45387edee49b5e9c922d6cb/components/exo/surface_unittest.cc [modify] https://crrev.com/87d78f2736ef5cd0e45387edee49b5e9c922d6cb/components/exo/test/run_all_unittests.cc
,
Dec 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79 commit d8f4f642d72fc5fdb428f5a90ca207c2143a6f79 Author: staraz <staraz@chromium.org> Date: Sat Dec 10 23:49:20 2016 Moved exo::SurfaceFactoryOwner to its own file and renamed it to exo::CompositorFrameSink. CompositorFrameSink implements cc::mojom::MojoCompositorFrameSink. CompositorFrameSink is no longer a friend class of exo::Surface. Added exo::CompositorFrameSinkHolder class that implements cc::mojom::MojoCompositorFrameSinkClient. BUG= 659601 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2493223002 Review-Url: https://codereview.chromium.org/2493223002 Cr-Commit-Position: refs/heads/master@{#437780} [modify] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/ash/test/ash_test_helper.cc [modify] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/chrome/browser/media/webrtc/desktop_media_list_ash_unittest.cc [modify] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/components/exo/BUILD.gn [modify] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/components/exo/DEPS [add] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/components/exo/compositor_frame_sink.cc [add] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/components/exo/compositor_frame_sink.h [add] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/components/exo/compositor_frame_sink_holder.cc [add] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/components/exo/compositor_frame_sink_holder.h [modify] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/components/exo/surface.cc [modify] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/components/exo/surface.h [modify] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/components/exo/surface_unittest.cc [modify] https://crrev.com/d8f4f642d72fc5fdb428f5a90ca207c2143a6f79/components/exo/test/run_all_unittests.cc
,
Dec 12 2016
,
Feb 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2cd69789c0cc1bf7b35f6c5673ba1db6f2273cf0 commit 2cd69789c0cc1bf7b35f6c5673ba1db6f2273cf0 Author: reveman <reveman@chromium.org> Date: Wed Feb 01 20:19:44 2017 exo: Cleanup and make buffer release code more robust. Since the introduction of CompositorFrameSinkHolder it's possible for release callbacks to be lost. This is because we rely on the use count to drop to 0 but we're only guaranteed to get a callback for the last frame as we only keep one CompositorFrameSinkHolder reference. This removes the use count in favor of a simple cancelable callback. The use count was never necessary as attaching the buffer to multiple surfaces is a client behavior that results in undefined release callback behavior. Running the callback when the last attachment is released is not worse then sending it when all attachments have been released. BUG= 659601 TEST=exo_unittests --gtest_filter=BufferTest.* Review-Url: https://codereview.chromium.org/2666233002 Cr-Commit-Position: refs/heads/master@{#447593} [modify] https://crrev.com/2cd69789c0cc1bf7b35f6c5673ba1db6f2273cf0/components/exo/buffer.cc [modify] https://crrev.com/2cd69789c0cc1bf7b35f6c5673ba1db6f2273cf0/components/exo/buffer.h
,
Mar 8 2017
,
Mar 8 2017
This bug requires manual review: We are only 5 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 8 2017
What OS is this merge being requested for? And why are we requesting a merge for Pri-3 bug only a few days before stable? I'm tempted to reject it outright but I'll wait until we have an OS tag to let the platform TPM make the right call.
,
Mar 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/05b55a8cd99bb90122b92194f01b15cc849f8860 commit 05b55a8cd99bb90122b92194f01b15cc849f8860 Author: Fady Samuel <fsamuel@chromium.org> Date: Wed Mar 08 00:40:34 2017 exo: Cleanup and make buffer release code more robust. Since the introduction of CompositorFrameSinkHolder it's possible for release callbacks to be lost. This is because we rely on the use count to drop to 0 but we're only guaranteed to get a callback for the last frame as we only keep one CompositorFrameSinkHolder reference. This removes the use count in favor of a simple cancelable callback. The use count was never necessary as attaching the buffer to multiple surfaces is a client behavior that results in undefined release callback behavior. Running the callback when the last attachment is released is not worse then sending it when all attachments have been released. BUG= 659601 TEST=exo_unittests --gtest_filter=BufferTest.* Review-Url: https://codereview.chromium.org/2666233002 Cr-Commit-Position: refs/heads/master@{#447593} (cherry picked from commit 2cd69789c0cc1bf7b35f6c5673ba1db6f2273cf0) Review-Url: https://codereview.chromium.org/2736753006 . Cr-Commit-Position: refs/branch-heads/2987@{#793} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/05b55a8cd99bb90122b92194f01b15cc849f8860/components/exo/buffer.cc [modify] https://crrev.com/05b55a8cd99bb90122b92194f01b15cc849f8860/components/exo/buffer.h
,
Mar 8 2017
The last patch was blocking a P0 chrome OS bug: 697227. I've merged it into M57. We should be good now. *crosses fingers*
,
Mar 8 2017
CL listed at #20 got merged without approval (fsamuel@ ptal comment #19 by amineer@). Per comment #21 this seems to be related to Chrome OS, hence applying OS=Chrome label. Please apply any other OSes label if applicable. Thank you.
,
Mar 8 2017
govind@: Sorry for the confusion. I got approval for this merge from ketakid@ in bug 697227.
,
Mar 8 2017
Ok, no worries. Thank you for confirmation fsamuel@.
,
Apr 17 2017
,
May 30 2017
,
Jun 13 2017
,
Aug 1 2017
,
Oct 14 2017
,
Feb 26 2018
|
||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||
Comment 1 by fsam...@chromium.org
, Oct 26 2016