Allow switching DirectCompositionSurfaceWin between drawing modes.

Allow switching DirectCompositionSurfaceWin between drawing modes.

There are two ways to put the main page content into a DirectComposition
surface. IDXGISwapChain has the advantage that it can be put into an
overlay, saving power, but updates to it don't happen in sync with
updates to the layer tree. This results in weird-looking lag when
scrolling and makes transitions between layer structures look bad.

IDCompositionSurface does have its contents synced with tree updates,
but can't be put in an overlay. This wastes power on pages with canvas
or software-decoded video that update often but can't be put in
independent layers.

To get the best of both worlds, add a new SetEnableDCLayersCHROMIUM GL
command that can switch the backbuffers between the two modes. It will
be enabled when an overlay is promoted for the first time and is
disabled 60 frames after the last frame where an overlay is used.
This hysteresis helps prevent unnecessary switches, which are expensive
because they require reallocating the buffer, redrawing the entire
frame, and (when switching to IDXGISwapChains) waiting for the contents
to finish rendering on the GPU before swapping.

BUG=678800
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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

Review-Url: https://codereview.chromium.org/2743663006
Cr-Commit-Position: refs/heads/master@{#458664}
Committed: https://chromium.googlesource.com/chromium/src/+/456e9b8f3bf0301b3d026953f6d23d1d626a07e6

[jbauman, 2017-03-11 01:27:44]

Description was changed from

==========
Allow switching DirectCompositionSurfaceWin between drawing modes.

There are two ways to put the main page content into a DirectComposition
surface. IDXGISwapChain has the advantage that it can be put into an
overlay, saving power, but updates to it don't happen in sync with
updates to the layer tree. This results in weird-looking lag when
scrolling and makes transitions between layer structures look bad.

IDCompositionSurface does have its contents synced with tree updates,
but can't be put in an overlay. This wastes power on pages with canvas
or software-decoded video that update often but can't be put in
independent layers.

To get the best of both worlds, add a new SetEnableDCLayersCHROMIUM GL
command that can switch the backbuffers between the two modes. It will
be enabled when an overlay is promoted for the first time and is
disabled 60 frames after the last frame where an overlay is used.
This hysteresis helps prevent unnecessary switches, which are expensive
because they require reallocating the buffer, redrawing the entire
frame, and (when switching to IDXGISwapChains) waiting for the contents
to finish rendering on the GPU before swapping.

BUG=678800
==========

to

==========
Allow switching DirectCompositionSurfaceWin between drawing modes.

There are two ways to put the main page content into a DirectComposition
surface. IDXGISwapChain has the advantage that it can be put into an
overlay, saving power, but updates to it don't happen in sync with
updates to the layer tree. This results in weird-looking lag when
scrolling and makes transitions between layer structures look bad.

IDCompositionSurface does have its contents synced with tree updates,
but can't be put in an overlay. This wastes power on pages with canvas
or software-decoded video that update often but can't be put in
independent layers.

To get the best of both worlds, add a new SetEnableDCLayersCHROMIUM GL
command that can switch the backbuffers between the two modes. It will
be enabled when an overlay is promoted for the first time and is
disabled 60 frames after the last frame where an overlay is used.
This hysteresis helps prevent unnecessary switches, which are expensive
because they require reallocating the buffer, redrawing the entire
frame, and (when switching to IDXGISwapChains) waiting for the contents
to finish rendering on the GPU before swapping.

BUG=678800
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
==========

[jbauman, 2017-03-11 01:27:45]

Description was changed from

==========
Allow switching DirectCompositionSurfaceWin between drawing modes.

There are two ways to put the main page content into a DirectComposition
surface. IDXGISwapChain has the advantage that it can be put into an
overlay, saving power, but updates to it don't happen in sync with
updates to the layer tree. This results in weird-looking lag when
scrolling and makes transitions between layer structures look bad.

IDCompositionSurface does have its contents synced with tree updates,
but can't be put in an overlay. This wastes power on pages with canvas
or software-decoded video that update often but can't be put in
independent layers.

To get the best of both worlds, add a new SetEnableDCLayersCHROMIUM GL
command that can switch the backbuffers between the two modes. It will
be enabled when an overlay is promoted for the first time and is
disabled 60 frames after the last frame where an overlay is used.
This hysteresis helps prevent unnecessary switches, which are expensive
because they require reallocating the buffer, redrawing the entire
frame, and (when switching to IDXGISwapChains) waiting for the contents
to finish rendering on the GPU before swapping.

BUG=678800
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
==========

to

==========
Allow switching DirectCompositionSurfaceWin between drawing modes.

There are two ways to put the main page content into a DirectComposition
surface. IDXGISwapChain has the advantage that it can be put into an
overlay, saving power, but updates to it don't happen in sync with
updates to the layer tree. This results in weird-looking lag when
scrolling and makes transitions between layer structures look bad.

IDCompositionSurface does have its contents synced with tree updates,
but can't be put in an overlay. This wastes power on pages with canvas
or software-decoded video that update often but can't be put in
independent layers.

To get the best of both worlds, add a new SetEnableDCLayersCHROMIUM GL
command that can switch the backbuffers between the two modes. It will
be enabled when an overlay is promoted for the first time and is
disabled 60 frames after the last frame where an overlay is used.
This hysteresis helps prevent unnecessary switches, which are expensive
because they require reallocating the buffer, redrawing the entire
frame, and (when switching to IDXGISwapChains) waiting for the contents
to finish rendering on the GPU before swapping.

BUG=678800
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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
==========

[jbauman, 2017-03-11 01:27:51]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-11 01:28:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-11 01:36:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-11 01:36:01]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jbauman, 2017-03-13 20:56:55]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-13 20:57:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-13 21:22:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-13 21:22:46]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[jbauman, 2017-03-13 22:22:28]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-13 22:23:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-13 22:50:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-13 22:50:17]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[jbauman, 2017-03-14 00:14:52]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-14 00:15:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-14 00:35:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-14 00:35:27]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jbauman, 2017-03-14 01:41:58]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-14 01:42:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-14 02:36:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-14 02:36:49]

Dry run: This issue passed the CQ dry run.

[jbauman, 2017-03-14 21:28:21]

jbauman@chromium.org changed reviewers:
+ sunnyps@chromium.org

[jbauman, 2017-03-17 02:12:15]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 02:12:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-17 04:00:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-17 04:00:58]

Dry run: This issue passed the CQ dry run.

[sunnyps, 2017-03-20 23:38:57]

https://codereview.chromium.org/2743663006/diff/80001/cc/output/gl_renderer_u...
File cc/output/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/2743663006/diff/80001/cc/output/gl_renderer_u...
cc/output/gl_renderer_unittest.cc:1910: explicit
PartialSwapMockGLES2Interface(bool support_set_draw_rectangle)
nit: support_dc_layers?

https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
File gpu/ipc/service/direct_composition_surface_win.cc (right):

https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
gpu/ipc/service/direct_composition_surface_win.cc:180:
swap_chain_->Present1(first_swap_ ? 0 : 1, 0, &params);
This will present a frame if first_swap_ = false and ReleaseDrawTexture is
called due to a resize. Do we want that?

https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
gpu/ipc/service/direct_composition_surface_win.cc:277: int height) {
Why do we even support PostSubBuffer for this surface?

https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
File gpu/ipc/service/direct_composition_surface_win_unittest.cc (right):

https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
gpu/ipc/service/direct_composition_surface_win_unittest.cc:71: new
DirectCompositionSurfaceWin(delegate.AsWeakPtr(),
nit: surface1

https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
gpu/ipc/service/direct_composition_surface_win_unittest.cc:76:
scoped_refptr<gl::GLContext> context =
nit: context1

https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
gpu/ipc/service/direct_composition_surface_win_unittest.cc:173: // surface.
nit: should we expose swap_chain_ and check that it exists?

[jbauman, 2017-03-20 23:59:19]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 00:00:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jbauman, 2017-03-21 00:05:33]

PTAL.

On 2017/03/20 23:38:57, sunnyps wrote:
>
https://codereview.chromium.org/2743663006/diff/80001/cc/output/gl_renderer_u...
> File cc/output/gl_renderer_unittest.cc (right):
> 
>
https://codereview.chromium.org/2743663006/diff/80001/cc/output/gl_renderer_u...
> cc/output/gl_renderer_unittest.cc:1910: explicit
> PartialSwapMockGLES2Interface(bool support_set_draw_rectangle)
> nit: support_dc_layers?
> 
Done.
>
https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
> File gpu/ipc/service/direct_composition_surface_win.cc (right):
> 
>
https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
> gpu/ipc/service/direct_composition_surface_win.cc:180:
> swap_chain_->Present1(first_swap_ ? 0 : 1, 0, &params);
> This will present a frame if first_swap_ = false and ReleaseDrawTexture is
> called due to a resize. Do we want that?
> 
Not really. Fixed.


>
https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
> gpu/ipc/service/direct_composition_surface_win.cc:277: int height) {
> Why do we even support PostSubBuffer for this surface?

Mainly we want cc/ to understand that this supports partial swap, and so we
support this so everything's unified. I've now simplified it by having it call
into SwapBuffers.
> 
>
https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
> File gpu/ipc/service/direct_composition_surface_win_unittest.cc (right):
> 
>
https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
> gpu/ipc/service/direct_composition_surface_win_unittest.cc:71: new
> DirectCompositionSurfaceWin(delegate.AsWeakPtr(),
> nit: surface1
> 
Done.

>
https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
> gpu/ipc/service/direct_composition_surface_win_unittest.cc:76:
> scoped_refptr<gl::GLContext> context =
> nit: context1
>
Done.
 
>
https://codereview.chromium.org/2743663006/diff/80001/gpu/ipc/service/direct_...
> gpu/ipc/service/direct_composition_surface_win_unittest.cc:173: // surface.
> nit: should we expose swap_chain_ and check that it exists?

Done.

[sunnyps, 2017-03-21 00:19:00]

lgtm % nit

https://codereview.chromium.org/2743663006/diff/100001/gpu/ipc/service/direct...
File gpu/ipc/service/direct_composition_surface_win.h (right):

https://codereview.chromium.org/2743663006/diff/100001/gpu/ipc/service/direct...
gpu/ipc/service/direct_composition_surface_win.h:59: const
base::win::ScopedComPtr<IDXGISwapChain1> swap_chain() const {
nit: operator-> on ScopedComPtr is const so the first const declaration here
doesn't do anything.

nit: can you rename this to swap_chain_for_testing?

[jbauman, 2017-03-21 00:43:33]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 00:44:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jbauman, 2017-03-21 00:45:17]

jbauman@chromium.org changed reviewers:
+ danakj@chromium.org, dcheng@chromium.org

[jbauman, 2017-03-21 00:45:18]

danakj@: cc/ OWNERS review
dcheng@: gpu\ipc\common\gpu_command_buffer_traits_multi.h IPC review

Thanks.

[commit-bot: I haz the power, 2017-03-21 02:27:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-21 02:27:23]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-03-21 07:10:48]

ipc lgtm

i don't really know what to do about the name; the style guide says to avoid
abbreviations, and dc is a bit vague here (it could easily stand for 'device
context', among other things).

at the same time, has_direct_composition_layers is much longer...

https://codereview.chromium.org/2743663006/diff/120001/gpu/ipc/common/gpu_com...
File gpu/ipc/common/gpu_command_buffer_traits_multi.h (right):

https://codereview.chromium.org/2743663006/diff/120001/gpu/ipc/common/gpu_com...
gpu/ipc/common/gpu_command_buffer_traits_multi.h:130:
IPC_STRUCT_TRAITS_MEMBER(dc_layers)
FWIW, my main thought is that it's not super obvious that dc maps to "direct
composition".

[danakj, 2017-03-21 15:54:05]

https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
File cc/output/direct_renderer.cc (right):

https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
cc/output/direct_renderer.cc:29: static gfx::Transform
OrthoProjectionMatrix(float left,
nit: whitespace after namespace {

https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
cc/output/direct_renderer.cc:69: const int
kNumberOfFramesBeforeDisablingDCLayers = 60;
constexpr

https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
cc/output/direct_renderer.cc:325: if
(!current_frame()->dc_layer_overlay_list.empty()) {
Couple suggestions to make this a bit easier to read IMO.

- Save off a was_using_dc_layers
- Then write the if/else logic to set using_dc_layers_. It doesn't need to nest
an inner check on the current state now.
- Then if using_dc_layers_ != was_using_dc_layers change the root_damage_rect
(only need to write that once then).

https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
cc/output/direct_renderer.cc:333: } else {
else if

https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
cc/output/direct_renderer.cc:626: if (supports_dc_layers_) {
I am confused how DC layers and chromecast's SetDrawRectangle are becoming the
same thing tbh. I thought DC layers was about overlays, but the SetDrawRectangle
is about bounds for the root surface's composited content.. Can this be
explained in some code comments somewhere maybe?

https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
File cc/output/direct_renderer.h (right):

https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
cc/output/direct_renderer.h:195: bool using_dc_layers_ = false;
nit: group bools for better packing

[jbauman, 2017-03-21 19:42:46]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 19:43:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jbauman, 2017-03-21 20:37:44]

On 2017/03/21 15:54:05, danakj wrote:
>
https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
> File cc/output/direct_renderer.cc (right):
> 
>
https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
> cc/output/direct_renderer.cc:29: static gfx::Transform
> OrthoProjectionMatrix(float left,
> nit: whitespace after namespace {

Done.
> 
>
https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
> cc/output/direct_renderer.cc:69: const int
> kNumberOfFramesBeforeDisablingDCLayers = 60;
> constexpr
> 
Done.

>
https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
> cc/output/direct_renderer.cc:325: if
> (!current_frame()->dc_layer_overlay_list.empty()) {
> Couple suggestions to make this a bit easier to read IMO.
> 
> - Save off a was_using_dc_layers
> - Then write the if/else logic to set using_dc_layers_. It doesn't need to
nest
> an inner check on the current state now.
> - Then if using_dc_layers_ != was_using_dc_layers change the root_damage_rect
> (only need to write that once then).
> 
Done.

>
https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
> cc/output/direct_renderer.cc:333: } else {
> else if
> 
>
https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
> cc/output/direct_renderer.cc:626: if (supports_dc_layers_) {
> I am confused how DC layers and chromecast's SetDrawRectangle are becoming the
> same thing tbh. I thought DC layers was about overlays, but the
SetDrawRectangle
> is about bounds for the root surface's composited content.. Can this be
> explained in some code comments somewhere maybe?
> 
SetDrawRectangle doesn't have anything to do with Chromecast (Chromecast uses
SwapBuffersWithBounds). The draw rectangle is essentially the same as the damage
rectangle, except the client must guarantee that it's drawn into during the
frame.

>
https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
> File cc/output/direct_renderer.h (right):
> 
>
https://codereview.chromium.org/2743663006/diff/120001/cc/output/direct_rende...
> cc/output/direct_renderer.h:195: bool using_dc_layers_ = false;
> nit: group bools for better packing

Done.

[commit-bot: I haz the power, 2017-03-21 21:09:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-21 21:09:40]

Dry run: Try jobs failed on following builders:
  win_optional_gpu_tests_rel on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_optional_gpu_...)

[danakj, 2017-03-21 21:36:12]

Oh! My bad about the chromecast thing.

Looks like this comment needs update since the capability name changed?
https://cs.chromium.org/chromium/src/cc/output/output_surface.h?rcl=6add17253...

LGTM otherwise

[jbauman, 2017-03-21 22:11:45]

The CQ bit was checked by jbauman@chromium.org

[jbauman, 2017-03-21 22:11:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
sunnyps@chromium.org, dcheng@chromium.org, danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/2743663006/#ps160001
(title: "change comments")

[commit-bot: I haz the power, 2017-03-21 22:12:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-21 23:12:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-21 23:12:43]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jbauman, 2017-03-22 00:53:54]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2017-03-22 00:54:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 01:46:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-22 01:46:40]

Failed to apply patch for gpu/ipc/service/direct_composition_surface_win.cc:
While running git apply --index -p1;
  error: patch failed: gpu/ipc/service/direct_composition_surface_win.cc:5
  error: gpu/ipc/service/direct_composition_surface_win.cc: patch does not apply

Patch:       gpu/ipc/service/direct_composition_surface_win.cc
Index: gpu/ipc/service/direct_composition_surface_win.cc
diff --git a/gpu/ipc/service/direct_composition_surface_win.cc
b/gpu/ipc/service/direct_composition_surface_win.cc
index
9454f787361fe6ff1eca8a0f557a58a889e2b946..d2e017c6bc1c1465d5d8308c479a8668aca8d45b
100644
--- a/gpu/ipc/service/direct_composition_surface_win.cc
+++ b/gpu/ipc/service/direct_composition_surface_win.cc
@@ -5,6 +5,7 @@
 #include "gpu/ipc/service/direct_composition_surface_win.h"
 
 #include "base/optional.h"
+#include "base/synchronization/waitable_event.h"
 #include "gpu/ipc/service/gpu_channel_manager.h"
 #include "gpu/ipc/service/gpu_channel_manager_delegate.h"
 #include "ui/gfx/native_widget_types.h"
@@ -115,32 +116,83 @@ bool
DirectCompositionSurfaceWin::Initialize(gl::GLSurfaceFormat format) {
       eglCreatePbufferSurface(display, GetConfig(), &pbuffer_attribs[0]);
   CHECK(!!default_surface_);
 
-  InitializeSurface();
-
   return true;
 }
 
-void DirectCompositionSurfaceWin::InitializeSurface() {
-  ScopedReleaseCurrent release_current(this);
-  ReleaseDrawTexture();
+void DirectCompositionSurfaceWin::ReleaseCurrentSurface() {
+  ReleaseDrawTexture(true);
   dcomp_surface_.Release();
-  HRESULT hr = dcomp_device_->CreateSurface(
-      size_.width(), size_.height(), DXGI_FORMAT_B8G8R8A8_UNORM,
-      DXGI_ALPHA_MODE_PREMULTIPLIED, dcomp_surface_.Receive());
-  has_been_rendered_to_ = false;
+  swap_chain_.Release();
+}
 
-  CHECK(SUCCEEDED(hr));
+void DirectCompositionSurfaceWin::InitializeSurface() {
+  DCHECK(!dcomp_surface_);
+  DCHECK(!swap_chain_);
+  if (enable_dc_layers_) {
+    HRESULT hr = dcomp_device_->CreateSurface(
+        size_.width(), size_.height(), DXGI_FORMAT_B8G8R8A8_UNORM,
+        DXGI_ALPHA_MODE_PREMULTIPLIED, dcomp_surface_.Receive());
+    has_been_rendered_to_ = false;
+    CHECK(SUCCEEDED(hr));
+  } else {
+    base::win::ScopedComPtr<IDXGIDevice> dxgi_device;
+    d3d11_device_.QueryInterface(dxgi_device.Receive());
+    base::win::ScopedComPtr<IDXGIAdapter> dxgi_adapter;
+    dxgi_device->GetAdapter(dxgi_adapter.Receive());
+    base::win::ScopedComPtr<IDXGIFactory2> dxgi_factory;
+    dxgi_adapter->GetParent(IID_PPV_ARGS(dxgi_factory.Receive()));
+
+    DXGI_SWAP_CHAIN_DESC1 desc = {};
+    desc.Width = size_.width();
+    desc.Height = size_.height();
+    desc.Format = DXGI_FORMAT_B8G8R8A8_UNORM;
+    desc.Stereo = FALSE;
+    desc.SampleDesc.Count = 1;
+    desc.BufferCount = 2;
+    desc.BufferUsage = DXGI_USAGE_RENDER_TARGET_OUTPUT;
+    desc.Scaling = DXGI_SCALING_STRETCH;
+    desc.SwapEffect = DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL;
+    desc.AlphaMode = DXGI_ALPHA_MODE_PREMULTIPLIED;
+    desc.Flags = 0;
+    HRESULT hr = dxgi_factory->CreateSwapChainForComposition(
+        d3d11_device_.get(), &desc, nullptr, swap_chain_.Receive());
+    has_been_rendered_to_ = false;
+    first_swap_ = true;
+    CHECK(SUCCEEDED(hr));
+  }
 }
 
-void DirectCompositionSurfaceWin::ReleaseDrawTexture() {
+void DirectCompositionSurfaceWin::ReleaseDrawTexture(bool will_discard) {
   if (real_surface_) {
     eglDestroySurface(GetDisplay(), real_surface_);
     real_surface_ = nullptr;
   }
   if (draw_texture_) {
     draw_texture_.Release();
-    HRESULT hr = dcomp_surface_->EndDraw();
-    CHECK(SUCCEEDED(hr));
+    if (dcomp_surface_) {
+      HRESULT hr = dcomp_surface_->EndDraw();
+      CHECK(SUCCEEDED(hr));
+    } else if (!will_discard) {
+      DXGI_PRESENT_PARAMETERS params = {};
+      RECT dirty_rect = swap_rect_.ToRECT();
+      params.DirtyRectsCount = 1;
+      params.pDirtyRects = &dirty_rect;
+      swap_chain_->Present1(first_swap_ ? 0 : 1, 0, &params);
+      if (first_swap_) {
+        // Wait for the GPU to finish executing its commands before
+        // committing the DirectComposition tree, or else the swapchain
+        // may flicker black when it's first presented.
+        base::win::ScopedComPtr<IDXGIDevice2> dxgi_device2;
+        HRESULT hr = d3d11_device_.QueryInterface(dxgi_device2.Receive());
+        DCHECK(SUCCEEDED(hr));
+        base::WaitableEvent event(
+            base::WaitableEvent::ResetPolicy::AUTOMATIC,
+            base::WaitableEvent::InitialState::NOT_SIGNALED);
+        dxgi_device2->EnqueueSetEvent(event.handle());
+        event.Wait();
+        first_swap_ = false;
+      }
+    }
   }
   if (dcomp_surface_ == g_current_surface)
     g_current_surface = nullptr;
@@ -161,8 +213,11 @@ void DirectCompositionSurfaceWin::Destroy() {
     }
     real_surface_ = nullptr;
   }
-  if (dcomp_surface_ == g_current_surface)
+  if (dcomp_surface_ && (dcomp_surface_ == g_current_surface)) {
+    HRESULT hr = dcomp_surface_->EndDraw();
+    CHECK(SUCCEEDED(hr));
     g_current_surface = nullptr;
+  }
   draw_texture_.Release();
   dcomp_surface_.Release();
 }
@@ -192,7 +247,9 @@ bool DirectCompositionSurfaceWin::Resize(const gfx::Size&
size,
     return false;
   }
   size_ = size;
-  InitializeSurface();
+  ScopedReleaseCurrent release_current(this);
+  // New surface will be initialized in SetDrawRectangle.
+  ReleaseCurrentSurface();
 
   return true;
 }
@@ -200,19 +257,16 @@ bool DirectCompositionSurfaceWin::Resize(const gfx::Size&
size,
 gfx::SwapResult DirectCompositionSurfaceWin::SwapBuffers() {
   {
     ScopedReleaseCurrent release_current(this);
-    ReleaseDrawTexture();
-    visual_->SetContent(dcomp_surface_.get());
+    ReleaseDrawTexture(false);
+    DCHECK(dcomp_surface_ || swap_chain_);
+    if (dcomp_surface_)
+      visual_->SetContent(dcomp_surface_.get());
+    else
+      visual_->SetContent(swap_chain_.get());
 
     CommitAndClearPendingOverlays();
     dcomp_device_->Commit();
   }
-  // Force the driver to finish drawing before clearing the contents to
-  // transparent, to reduce or eliminate the period of time where the contents
-  // have flashed black.
-  if (first_swap_) {
-    glFinish();
-    first_swap_ = false;
-  }
   child_window_.ClearInvalidContents();
   return gfx::SwapResult::SWAP_ACK;
 }
@@ -221,13 +275,9 @@ gfx::SwapResult
DirectCompositionSurfaceWin::PostSubBuffer(int x,
                                                            int y,
                                                            int width,
                                                            int height) {
-  ScopedReleaseCurrent release_current(this);
-  ReleaseDrawTexture();
-  visual_->SetContent(dcomp_surface_.get());
-  CommitAndClearPendingOverlays();
-  dcomp_device_->Commit();
-  child_window_.ClearInvalidContents();
-  return gfx::SwapResult::SWAP_ACK;
+  // The arguments are ignored because SetDrawRectangle specified the area to
+  // be swapped.
+  return SwapBuffers();
 }
 
 gfx::VSyncProvider* DirectCompositionSurfaceWin::GetVSyncProvider() {
@@ -245,6 +295,11 @@ bool DirectCompositionSurfaceWin::ScheduleOverlayPlane(
   return true;
 }
 
+bool DirectCompositionSurfaceWin::SetEnableDCLayers(bool enable) {
+  enable_dc_layers_ = enable;
+  return true;
+}
+
 bool DirectCompositionSurfaceWin::CommitAndClearPendingOverlays() {
   pending_overlays_.clear();
   return true;
@@ -274,13 +329,23 @@ bool
DirectCompositionSurfaceWin::OnMakeCurrent(gl::GLContext* context) {
   return true;
 }
 
-bool DirectCompositionSurfaceWin::SupportsSetDrawRectangle() const {
+bool DirectCompositionSurfaceWin::SupportsDCLayers() const {
   return true;
 }
 
 bool DirectCompositionSurfaceWin::SetDrawRectangle(const gfx::Rect& rectangle)
{
   if (draw_texture_)
     return false;
+
+  DCHECK(!real_surface_);
+  ScopedReleaseCurrent release_current(this);
+
+  if ((enable_dc_layers_ && !dcomp_surface_) ||
+      (!enable_dc_layers_ && !swap_chain_)) {
+    ReleaseCurrentSurface();
+    InitializeSurface();
+  }
+
   if (!gfx::Rect(size_).Contains(rectangle)) {
     DLOG(ERROR) << "Draw rectangle must be contained within size of surface";
     return false;
@@ -290,20 +355,24 @@ bool DirectCompositionSurfaceWin::SetDrawRectangle(const
gfx::Rect& rectangle) {
     return false;
   }
 
-  DCHECK(!real_surface_);
   CHECK(!g_current_surface);
-  ScopedReleaseCurrent release_current(this);
 
   RECT rect = rectangle.ToRECT();
-  POINT update_offset;
-
-  HRESULT hr = dcomp_surface_->BeginDraw(
-      &rect, IID_PPV_ARGS(draw_texture_.Receive()), &update_offset);
-  CHECK(SUCCEEDED(hr));
+  if (dcomp_surface_) {
+    POINT update_offset;
+    HRESULT hr = dcomp_surface_->BeginDraw(
+        &rect, IID_PPV_ARGS(draw_texture_.Receive()), &update_offset);
+    draw_offset_ = gfx::Point(update_offset) - gfx::Rect(rect).origin();
+    CHECK(SUCCEEDED(hr));
+  } else {
+    HRESULT hr =
+        swap_chain_->GetBuffer(0, IID_PPV_ARGS(draw_texture_.Receive()));
+    swap_rect_ = rectangle;
+    draw_offset_ = gfx::Vector2d();
+    CHECK(SUCCEEDED(hr));
+  }
   has_been_rendered_to_ = true;
 
-  draw_offset_ = gfx::Point(update_offset) - gfx::Rect(rect).origin();
-
   g_current_surface = dcomp_surface_.get();
 
   std::vector<EGLint> pbuffer_attribs{

[jbauman, 2017-03-22 01:55:42]

The CQ bit was checked by jbauman@chromium.org

[jbauman, 2017-03-22 01:55:43]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
sunnyps@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2743663006/#ps180001
(title: "rebase")

[commit-bot: I haz the power, 2017-03-22 01:56:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 03:33:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-22 03:33:27]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jbauman, 2017-03-22 07:02:30]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2017-03-22 07:03:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 07:25:21]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1490166150613510,
"parent_rev": "cb6ebed7c921e667425703651130450eab318721", "commit_rev":
"456e9b8f3bf0301b3d026953f6d23d1d626a07e6"}

[commit-bot: I haz the power, 2017-03-22 07:26:05]

Description was changed from

==========
Allow switching DirectCompositionSurfaceWin between drawing modes.

There are two ways to put the main page content into a DirectComposition
surface. IDXGISwapChain has the advantage that it can be put into an
overlay, saving power, but updates to it don't happen in sync with
updates to the layer tree. This results in weird-looking lag when
scrolling and makes transitions between layer structures look bad.

IDCompositionSurface does have its contents synced with tree updates,
but can't be put in an overlay. This wastes power on pages with canvas
or software-decoded video that update often but can't be put in
independent layers.

To get the best of both worlds, add a new SetEnableDCLayersCHROMIUM GL
command that can switch the backbuffers between the two modes. It will
be enabled when an overlay is promoted for the first time and is
disabled 60 frames after the last frame where an overlay is used.
This hysteresis helps prevent unnecessary switches, which are expensive
because they require reallocating the buffer, redrawing the entire
frame, and (when switching to IDXGISwapChains) waiting for the contents
to finish rendering on the GPU before swapping.

BUG=678800
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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
==========

to

==========
Allow switching DirectCompositionSurfaceWin between drawing modes.

There are two ways to put the main page content into a DirectComposition
surface. IDXGISwapChain has the advantage that it can be put into an
overlay, saving power, but updates to it don't happen in sync with
updates to the layer tree. This results in weird-looking lag when
scrolling and makes transitions between layer structures look bad.

IDCompositionSurface does have its contents synced with tree updates,
but can't be put in an overlay. This wastes power on pages with canvas
or software-decoded video that update often but can't be put in
independent layers.

To get the best of both worlds, add a new SetEnableDCLayersCHROMIUM GL
command that can switch the backbuffers between the two modes. It will
be enabled when an overlay is promoted for the first time and is
disabled 60 frames after the last frame where an overlay is used.
This hysteresis helps prevent unnecessary switches, which are expensive
because they require reallocating the buffer, redrawing the entire
frame, and (when switching to IDXGISwapChains) waiting for the contents
to finish rendering on the GPU before swapping.

BUG=678800
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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

Review-Url: https://codereview.chromium.org/2743663006
Cr-Commit-Position: refs/heads/master@{#458664}
Committed:
https://chromium.googlesource.com/chromium/src/+/456e9b8f3bf0301b3d026953f6d2...
==========

[commit-bot: I haz the power, 2017-03-22 07:26:07]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/456e9b8f3bf0301b3d026953f6d2...