DirectX capturer flickers on the second monitor

DirectX capturer flickers on the second monitor

In DxgiOutputDuplicator, we need to convert between a monitor based coordinate
and a entire screen based coordinate. i.e. Copying an updated area from a
monitor (an output in DirectX API) to the entire screen frame (DesktopFrame).
But DxgiOutputDuplicator always assumes the coordinate is based on screen frame.
So we only need to convert a rectange in updated_region to monitor based
coordinate when copying data from texture_. But in last_frame_, the data are
always based on screen coordinate.

So fixes are both required in line 167 and line 180. In the previous one, we do
not need to convert the DesktopRect, which is already screen based, into screen
based coordinate. In the late one, we do not need to convert the DesktopRect at
all. So after these two changes, DxgiOutputDuplicator::TargetRect() function can
be removed.

Flickers of DirectX capturer can happen on any devices, but a virtual machine
can easily reproduce it. While on a regular high performance machine, it's
harder, but not totally impossible, to reproduce the issue.

BUG=314516
Committed: https://crrev.com/8bc9326a0bc6bb2f07fbb1ef60dae1d8cddd2b4a
Cr-Commit-Position: refs/heads/master@{#15075}

[Hzj_jie, 2016-11-12 03:04:22]

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

[commit-bot: I haz the power, 2016-11-12 03:04:26]

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

[Hzj_jie, 2016-11-12 03:18:56]

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

[commit-bot: I haz the power, 2016-11-12 03:19:06]

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

[Hzj_jie, 2016-11-12 03:19:17]

Patchset #1 (id:1) has been deleted

[Hzj_jie, 2016-11-12 03:38:14]

Description was changed from

==========
DirectX capturer flickers on the second monitor

BUG=314516
==========

to

==========
DirectX capturer flickers on the second monitor

Sorry, I made a mistake before. The flicker can actually happen on any devices,
but a virtual machine can easily reproduce it. While on a regular high
performance machine, it's harder, but not totally impossible, to reproduce.

The reason of the flicker is because of wrongly copied areas.

BUG=314516
==========

[Hzj_jie, 2016-11-12 03:38:14]

zijiehe@chromium.org changed reviewers:
+ jamiewalch@chromium.org, sergeyu@chromium.org

[commit-bot: I haz the power, 2016-11-12 03:44:57]

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

[commit-bot: I haz the power, 2016-11-12 03:44:58]

Dry run: This issue passed the CQ dry run.

[Jamie, 2016-11-14 18:20:14]

The CL description is not very clear. When you say "before", are you referring
to another CL (in which case link to it) or to one of the conversations we had
about VMs vs. physical machines (in which case I don't think it's worth
mentioning--later readers of this won't have the context to understand it).

It would also be helpful to explain what caused the bug. Trusting OS-supplied
dirty rects that are not always correct? A bug in the original clipping logic?
It's not clear from the context exactly what the problem is, so it's hard to
assess whether or not the CL fixes it.

[Hzj_jie, 2016-11-14 19:38:01]

Description was changed from

==========
DirectX capturer flickers on the second monitor

Sorry, I made a mistake before. The flicker can actually happen on any devices,
but a virtual machine can easily reproduce it. While on a regular high
performance machine, it's harder, but not totally impossible, to reproduce.

The reason of the flicker is because of wrongly copied areas.

BUG=314516
==========

to

==========
DirectX capturer flickers on the second monitor

In DxgiOutputDuplicator, we need to convert between a monitor based coordinate
and a entire screen based coordinate. i.e. Copying an updated area from a
monitor (an output in DirectX API) to the entire screen frame (DesktopFrame).
But DxgiOutputDuplicator always assumes the coordinate is based on screen frame.
So we only need to convert a rectange in updated_region to monitor based
coordinate when copying data from texture_. But in last_frame_, the data are
always based on screen coordinate.

So fixes are both required in line 167 and line 180. In the previous one, we do
not need to convert the DesktopRect, which is already screen based, into screen
based coordinate. In the late one, we do not need to convert the DesktopRect at
all. So after these two changes, DxgiOutputDuplicator::TargetRect() function can
be removed.

Flickers of DirectX capturer can happen on any devices, but a virtual machine
can easily reproduce it. While on a regular high performance machine, it's
harder, but not totally impossible, to reproduce the issue.

BUG=314516
==========

[Hzj_jie, 2016-11-14 19:38:34]

On 2016/11/14 18:20:14, Jamie wrote:
> The CL description is not very clear. When you say "before", are you referring
> to another CL (in which case link to it) or to one of the conversations we had
> about VMs vs. physical machines (in which case I don't think it's worth
> mentioning--later readers of this won't have the context to understand it).
> 
> It would also be helpful to explain what caused the bug. Trusting OS-supplied
> dirty rects that are not always correct? A bug in the original clipping logic?
> It's not clear from the context exactly what the problem is, so it's hard to
> assess whether or not the CL fixes it.

Thank you for the comments. I have updated the description to explain the issue
and fix more clearly.

[Jamie, 2016-11-14 21:02:08]

lgtm

[Hzj_jie, 2016-11-15 01:29:48]

Hi, Sergey, I still need your LGTM to submit.

[Sergey Ulanov, 2016-11-15 01:51:36]

lgtm

[Hzj_jie, 2016-11-15 01:51:57]

The CQ bit was checked by zijiehe@chromium.org

[commit-bot: I haz the power, 2016-11-15 01:52:03]

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

[commit-bot: I haz the power, 2016-11-15 02:20:36]

Description was changed from

==========
DirectX capturer flickers on the second monitor

In DxgiOutputDuplicator, we need to convert between a monitor based coordinate
and a entire screen based coordinate. i.e. Copying an updated area from a
monitor (an output in DirectX API) to the entire screen frame (DesktopFrame).
But DxgiOutputDuplicator always assumes the coordinate is based on screen frame.
So we only need to convert a rectange in updated_region to monitor based
coordinate when copying data from texture_. But in last_frame_, the data are
always based on screen coordinate.

So fixes are both required in line 167 and line 180. In the previous one, we do
not need to convert the DesktopRect, which is already screen based, into screen
based coordinate. In the late one, we do not need to convert the DesktopRect at
all. So after these two changes, DxgiOutputDuplicator::TargetRect() function can
be removed.

Flickers of DirectX capturer can happen on any devices, but a virtual machine
can easily reproduce it. While on a regular high performance machine, it's
harder, but not totally impossible, to reproduce the issue.

BUG=314516
==========

to

==========
DirectX capturer flickers on the second monitor

In DxgiOutputDuplicator, we need to convert between a monitor based coordinate
and a entire screen based coordinate. i.e. Copying an updated area from a
monitor (an output in DirectX API) to the entire screen frame (DesktopFrame).
But DxgiOutputDuplicator always assumes the coordinate is based on screen frame.
So we only need to convert a rectange in updated_region to monitor based
coordinate when copying data from texture_. But in last_frame_, the data are
always based on screen coordinate.

So fixes are both required in line 167 and line 180. In the previous one, we do
not need to convert the DesktopRect, which is already screen based, into screen
based coordinate. In the late one, we do not need to convert the DesktopRect at
all. So after these two changes, DxgiOutputDuplicator::TargetRect() function can
be removed.

Flickers of DirectX capturer can happen on any devices, but a virtual machine
can easily reproduce it. While on a regular high performance machine, it's
harder, but not totally impossible, to reproduce the issue.

BUG=314516
==========

[commit-bot: I haz the power, 2016-11-15 02:20:37]

Committed patchset #1 (id:2)

[commit-bot: I haz the power, 2016-11-15 02:20:47]

Description was changed from

==========
DirectX capturer flickers on the second monitor

In DxgiOutputDuplicator, we need to convert between a monitor based coordinate
and a entire screen based coordinate. i.e. Copying an updated area from a
monitor (an output in DirectX API) to the entire screen frame (DesktopFrame).
But DxgiOutputDuplicator always assumes the coordinate is based on screen frame.
So we only need to convert a rectange in updated_region to monitor based
coordinate when copying data from texture_. But in last_frame_, the data are
always based on screen coordinate.

So fixes are both required in line 167 and line 180. In the previous one, we do
not need to convert the DesktopRect, which is already screen based, into screen
based coordinate. In the late one, we do not need to convert the DesktopRect at
all. So after these two changes, DxgiOutputDuplicator::TargetRect() function can
be removed.

Flickers of DirectX capturer can happen on any devices, but a virtual machine
can easily reproduce it. While on a regular high performance machine, it's
harder, but not totally impossible, to reproduce the issue.

BUG=314516
==========

to

==========
DirectX capturer flickers on the second monitor

In DxgiOutputDuplicator, we need to convert between a monitor based coordinate
and a entire screen based coordinate. i.e. Copying an updated area from a
monitor (an output in DirectX API) to the entire screen frame (DesktopFrame).
But DxgiOutputDuplicator always assumes the coordinate is based on screen frame.
So we only need to convert a rectange in updated_region to monitor based
coordinate when copying data from texture_. But in last_frame_, the data are
always based on screen coordinate.

So fixes are both required in line 167 and line 180. In the previous one, we do
not need to convert the DesktopRect, which is already screen based, into screen
based coordinate. In the late one, we do not need to convert the DesktopRect at
all. So after these two changes, DxgiOutputDuplicator::TargetRect() function can
be removed.

Flickers of DirectX capturer can happen on any devices, but a virtual machine
can easily reproduce it. While on a regular high performance machine, it's
harder, but not totally impossible, to reproduce the issue.

BUG=314516

Committed: https://crrev.com/8bc9326a0bc6bb2f07fbb1ef60dae1d8cddd2b4a
Cr-Commit-Position: refs/heads/master@{#15075}
==========

[commit-bot: I haz the power, 2016-11-15 02:20:49]

Patchset 1 (id:??) landed as
https://crrev.com/8bc9326a0bc6bb2f07fbb1ef60dae1d8cddd2b4a
Cr-Commit-Position: refs/heads/master@{#15075}