This is similar to bug 1191965. It's less clear whether it's possible to do this with DirectComposition. There's no obvious way to take an existing surface and turn it into something that DirectComposition can use. However, it does seem to possible to do that with a SwapChain using IDCompositionDevice::CreateSurfaceFromHandle. There's also MF_MEDIA_ENGINE_PLAYBACK_VISUAL which seems used for getting DXVA stuff into a DirectComposition visual

I take this bug as to enable DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL usage of Bug 1419293. The flag usage make composition performance better.

Direct Composition could have 3 pars. [1] Enable DirectComposition on Windows if possible. [2] Separate Chrome and content compositions to different CompositionSurface [3] Render video as overlays if possible. This bug only handle [1]. It could improve performance and power consumption.

attachment 8952301 [details] [diff] [review] add capability to create child window in GPU process. It is necessary for DirectComposition. IDCompositionDevice::CreateTargetForHwnd() requests that window must belong to the calling process. https://msdn.microsoft.com/en-us/library/windows/desktop/hh437396(v=vs.85).aspx

attachment 8952312 [details] [diff] [review] adds capability to enable DirectComposition if gfx.webrender.dcomp-win.enabled pref is true and platform supports it. attachment 8952301 [details] [diff] [review] just enable DirectComposition. But with it, we could get performance improvement by using DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL. I tried to enable DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL without using DirectComposition, but it caused a problem like Bug 1435995 and disabled now.

Current DirectComposition is very basic and it mimics how ANGLE uses DirectComposition https://dxr.mozilla.org/mozilla-central/source/gfx/angle/src/libANGLE/renderer/d3d/d3d11/win32/NativeWindow11Win32.cpp#68

Comment on attachment 8952301 [details] [diff] [review] patch part 1 - Create child window in gpu process :jrmuizel, can you feedback to the patches?

Sotaro, do you know why weren't creating a child window in the gpu process before? i.e. even when not using DirectComposition wouldn't it be valuable to create a child window in that process?

(In reply to Jeff Muizelaar [:jrmuizel] from comment #15) > Sotaro, do you know why weren't creating a child window in the gpu process > before? i.e. even when not using DirectComposition wouldn't it be valuable > to create a child window in that process? I do not know the reason. But gpu process use HWND just to create swap chain and to get window size when d3d is used for composite. In these cases, we do not need to create child window in gpu process. I do not know if there could be another reason to create the child window other than for DirectComposition. Chromium also seems to create child window only for DirectComposition in gpu process. https://cs.chromium.org/chromium/src/gpu/ipc/service/direct_composition_surface_win.h?l=100 https://cs.chromium.org/chromium/src/gpu/ipc/service/child_window_win.cc?l=156

Comment on attachment 8952312 [details] [diff] [review] patch part 2 - Add capatiblity to enable DComp Review of attachment 8952312 [details] [diff] [review]: ----------------------------------------------------------------- I haven't looked to closely but this seems fine.

Comment on attachment 8952301 [details] [diff] [review] patch part 1 - Create child window in gpu process Review of attachment 8952301 [details] [diff] [review]: ----------------------------------------------------------------- I've looked even less closely at this, but the basic idea seems sound.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #15) > even when not using DirectComposition wouldn't it be valuable to create a child window in that process? I noticed that it seems valuable to use child window on gpu process to prevent jiggle during resizing. The some jiggling still exists during resizing with WebRender. When child window was used, the jiggling becomes really smaller.

The patch has a problem of handling multiple windows.

Fixed multiple windows bug.

Clean up.

Some clean ups.

Fixed CompositorBridgeParent::DeallocPWebRenderBridgeParent().

Rebased.

Update pref.

Fixed build failure on linux.

Rebased.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=99657856a1bd3aeab7a846cf0a851ee7bc8be79b https://treeherder.mozilla.org/#/jobs?repo=try&revision=0443728ad4c6c0529746f4300e383dd5eeed5473

Fixed debug build.

Comment on attachment 8962705 [details] [diff] [review] patch part 2 - Add capatiblity to enable DComp Review of attachment 8962705 [details] [diff] [review]: ----------------------------------------------------------------- Bas is probably a better reviewer for these than me.

Comment on attachment 8962645 [details] [diff] [review] patch part 1 - Create child window in gpu process Review of attachment 8962645 [details] [diff] [review]: ----------------------------------------------------------------- Bumping to Bas

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c4db980ad028b1b28fe93d1302c82635954d7625 https://treeherder.mozilla.org/#/jobs?repo=try&revision=02cb8bec5e5a5b0021e932f41c5c5424dfe66920

Comment on attachment 8962645 [details] [diff] [review] patch part 1 - Create child window in gpu process Review of attachment 8962645 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/windows/WinCompositorWindowThread.cpp @@ +62,5 @@ > + RefPtr<WinCompositorWindowThread>(sWinCompositorWindowThread.get()), > + &WinCompositorWindowThread::ShutDownTask, > + &task); > + sWinCompositorWindowThread->Loop()->PostTask(runnable.forget()); > + task.Wait(); It's not clear to me how this actually shuts the compositor thread down? As far as I can tell all this function does is ensure all messages currently queued have been processed.

Comment on attachment 8962705 [details] [diff] [review] patch part 2 - Add capatiblity to enable DComp Review of attachment 8962705 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/ipc/CompositorBridgeParent.cpp @@ +1752,5 @@ > MOZ_ASSERT(!mCompositorScheduler); > MOZ_ASSERT(mWidget); > > #ifdef XP_WIN > + if (DeviceManagerDx::Get()->CanUseDComp() && mWidget) { How does this now ensure we only do this in the GPU process? ::: gfx/webrender_bindings/RenderCompositor.h @@ +7,5 @@ > #ifndef MOZILLA_GFX_RENDERCOMPOSITOR_H > #define MOZILLA_GFX_RENDERCOMPOSITOR_H > > #include "mozilla/RefPtr.h" > +#include "mozilla/UniquePtr.h" I assume this was just a fix for unified builds somehow?

(In reply to Bas Schouten (:bas.schouten) from comment #34) > > It's not clear to me how this actually shuts the compositor thread down? As > far as I can tell all this function does is ensure all messages currently > queued have been processed. Compositor thread was shut down by CompositorThreadHolder::Shutdown(). It is called before RenderThread::ShutDown().

(In reply to Bas Schouten (:bas.schouten) from comment #35) > Comment on attachment 8962705 [details] [diff] [review] > patch part 2 - Add capatiblity to enable DComp > > Review of attachment 8962705 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/layers/ipc/CompositorBridgeParent.cpp > @@ +1752,5 @@ > > MOZ_ASSERT(!mCompositorScheduler); > > MOZ_ASSERT(mWidget); > > > > #ifdef XP_WIN > > + if (DeviceManagerDx::Get()->CanUseDComp() && mWidget) { > > How does this now ensure we only do this in the GPU process? DCompositionDevice is created only in "GPU process + WebRender enabled" for now. > > ::: gfx/webrender_bindings/RenderCompositor.h > @@ +7,5 @@ > > #ifndef MOZILLA_GFX_RENDERCOMPOSITOR_H > > #define MOZILLA_GFX_RENDERCOMPOSITOR_H > > > > #include "mozilla/RefPtr.h" > > +#include "mozilla/UniquePtr.h" > > I assume this was just a fix for unified builds somehow? Yes.

Pushed by sikeda@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/19142efa84a5 part 1 - Create child window in gpu process r=bas https://hg.mozilla.org/integration/mozilla-inbound/rev/d328d8eda8e6 part 2 - Add capatiblity to enable DComp r=bas

(In reply to Sotaro Ikeda [:sotaro] from comment #36) > (In reply to Bas Schouten (:bas.schouten) from comment #34) > > > > It's not clear to me how this actually shuts the compositor thread down? As > > far as I can tell all this function does is ensure all messages currently > > queued have been processed. > > Compositor thread was shut down by CompositorThreadHolder::Shutdown(). It is > called before RenderThread::ShutDown(). Oh, I misunderstood your question. WinCompositorWindowThread was shut down by clearing sWinCompositorWindowThread. It mimics how RenderThread is shut down. > static StaticRefPtr<WinCompositorWindowThread> sWinCompositorWindowThread;

https://hg.mozilla.org/mozilla-central/rev/19142efa84a5 https://hg.mozilla.org/mozilla-central/rev/d328d8eda8e6