Correctly handle damage involving filters in SurfaceAggregator

Correctly handle damage involving filters in SurfaceAggregator

There are two main issues with the handling of damage of contents
affected by RenderPassDrawQuad filters:
1) Damage of a small part of a source Surface can affect a large part
of the destination and
2) To draw damage on the destination, an unexpectedly large portion
of the source must be used.

To deal with these, the SurfaceAggregator must keep track of all output
render passes affected by filters. Any Surface in an affected RenderPass
will cause its parent Surface to be completely damaged (handling #1
above).

To handle #2, affected RenderPasses will have their damage rects set to
contain the entire output_rect. Then DirectRenderer can use that damage
rect to determine what to draw.

CopyRequests can also cause areas outside the damage rect to be read, so
any RenderPass inside a CopyRequest (directly or indirectly) will have
its damage rect set to the output rect.

This system is very pessimistic about what should be drawn, but in
general it shouldn't affect that many cases and should always be
correct.

BUG=602941
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/575a9b96a2b2776a6a4ae81f2e20c2bf3e975250
Cr-Commit-Position: refs/heads/master@{#391733}

[jbauman, 2016-04-29 02:03:03]

Description was changed from

==========
Try fixing filter logic 2
==========

to

==========
Try fixing filter logic 2
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jbauman, 2016-05-03 02:02:50]

Description was changed from

==========
Try fixing filter logic 2
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Correctly handle damage involving filters in SurfaceAggregator

There are two main issues with the handling of damage of contents
affected by RenderPassDrawQuad filters:
1) Damage of a small part of a source Surface can affect a large part
of the destination and
2) To draw damage on the destination, an unexpectedly large portion
of the source must be used.

To deal with these, the SurfaceAggregator must keep track of all output
render passes affected by filters. Any Surface in an affected RenderPass
will cause its parent Surface to be completely damaged (handling #1
above).

To handle #2, affected RenderPasses will have their damage rects set to
contain the entire output_rect. Then DirectRenderer can use that damage
rect to determine what to draw.

CopyRequests can also cause areas outside the damage rect to be read, so
any RenderPass inside a CopyRequest (directly or indirectly) will have
its damage rect set to the output rect.

This system is very pessimistic about what should be drawn, but in
general it shouldn't affect that many cases and should always be
correct.

BUG=602941
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jbauman, 2016-05-03 21:54:44]

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

[piman, 2016-05-03 23:01:12]

lgtm

https://codereview.chromium.org/1927413002/diff/60001/cc/surfaces/surface_agg...
File cc/surfaces/surface_aggregator.cc (right):

https://codereview.chromium.org/1927413002/diff/60001/cc/surfaces/surface_agg...
cc/surfaces/surface_aggregator.cc:567: if (frame_data->render_pass_list.size())
{
nit: !frame_data->render_pass_list.empty()

https://codereview.chromium.org/1927413002/diff/60001/cc/surfaces/surface_agg...
cc/surfaces/surface_aggregator.cc:600: if (quad->material ==
DrawQuad::RENDER_PASS) {
nit: else if

[jbauman, 2016-05-05 01:46:14]

The CQ bit was checked by jbauman@chromium.org

[jbauman, 2016-05-05 01:46:14]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1927413002/#ps100001
(title: " ")

[commit-bot: I haz the power, 2016-05-05 01:46:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1927413002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1927413002/100001

[commit-bot: I haz the power, 2016-05-05 02:19:21]

Description was changed from

==========
Correctly handle damage involving filters in SurfaceAggregator

There are two main issues with the handling of damage of contents
affected by RenderPassDrawQuad filters:
1) Damage of a small part of a source Surface can affect a large part
of the destination and
2) To draw damage on the destination, an unexpectedly large portion
of the source must be used.

To deal with these, the SurfaceAggregator must keep track of all output
render passes affected by filters. Any Surface in an affected RenderPass
will cause its parent Surface to be completely damaged (handling #1
above).

To handle #2, affected RenderPasses will have their damage rects set to
contain the entire output_rect. Then DirectRenderer can use that damage
rect to determine what to draw.

CopyRequests can also cause areas outside the damage rect to be read, so
any RenderPass inside a CopyRequest (directly or indirectly) will have
its damage rect set to the output rect.

This system is very pessimistic about what should be drawn, but in
general it shouldn't affect that many cases and should always be
correct.

BUG=602941
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Correctly handle damage involving filters in SurfaceAggregator

There are two main issues with the handling of damage of contents
affected by RenderPassDrawQuad filters:
1) Damage of a small part of a source Surface can affect a large part
of the destination and
2) To draw damage on the destination, an unexpectedly large portion
of the source must be used.

To deal with these, the SurfaceAggregator must keep track of all output
render passes affected by filters. Any Surface in an affected RenderPass
will cause its parent Surface to be completely damaged (handling #1
above).

To handle #2, affected RenderPasses will have their damage rects set to
contain the entire output_rect. Then DirectRenderer can use that damage
rect to determine what to draw.

CopyRequests can also cause areas outside the damage rect to be read, so
any RenderPass inside a CopyRequest (directly or indirectly) will have
its damage rect set to the output rect.

This system is very pessimistic about what should be drawn, but in
general it shouldn't affect that many cases and should always be
correct.

BUG=602941
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-05-05 02:19:22]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-05-05 02:20:20]

Description was changed from

==========
Correctly handle damage involving filters in SurfaceAggregator

There are two main issues with the handling of damage of contents
affected by RenderPassDrawQuad filters:
1) Damage of a small part of a source Surface can affect a large part
of the destination and
2) To draw damage on the destination, an unexpectedly large portion
of the source must be used.

To deal with these, the SurfaceAggregator must keep track of all output
render passes affected by filters. Any Surface in an affected RenderPass
will cause its parent Surface to be completely damaged (handling #1
above).

To handle #2, affected RenderPasses will have their damage rects set to
contain the entire output_rect. Then DirectRenderer can use that damage
rect to determine what to draw.

CopyRequests can also cause areas outside the damage rect to be read, so
any RenderPass inside a CopyRequest (directly or indirectly) will have
its damage rect set to the output rect.

This system is very pessimistic about what should be drawn, but in
general it shouldn't affect that many cases and should always be
correct.

BUG=602941
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Correctly handle damage involving filters in SurfaceAggregator

There are two main issues with the handling of damage of contents
affected by RenderPassDrawQuad filters:
1) Damage of a small part of a source Surface can affect a large part
of the destination and
2) To draw damage on the destination, an unexpectedly large portion
of the source must be used.

To deal with these, the SurfaceAggregator must keep track of all output
render passes affected by filters. Any Surface in an affected RenderPass
will cause its parent Surface to be completely damaged (handling #1
above).

To handle #2, affected RenderPasses will have their damage rects set to
contain the entire output_rect. Then DirectRenderer can use that damage
rect to determine what to draw.

CopyRequests can also cause areas outside the damage rect to be read, so
any RenderPass inside a CopyRequest (directly or indirectly) will have
its damage rect set to the output rect.

This system is very pessimistic about what should be drawn, but in
general it shouldn't affect that many cases and should always be
correct.

BUG=602941
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/575a9b96a2b2776a6a4ae81f2e20c2bf3e975250
Cr-Commit-Position: refs/heads/master@{#391733}
==========

[commit-bot: I haz the power, 2016-05-05 02:20:21]

Patchset 6 (id:??) landed as
https://crrev.com/575a9b96a2b2776a6a4ae81f2e20c2bf3e975250
Cr-Commit-Position: refs/heads/master@{#391733}

[Stephen White, 2016-05-05 03:03:15]

FYI: filters do have a method, SkImageFilter::filterBounds(), for mapping a rect
in both directions. If I understand your use case correctly, source rect to
damage rect would be kForward_MapDirection, and damage rect back to source rect
would be kReverse_MapDirection. It's exposed via FilterOperation::MapRect(),
although currently it only does the forward mapping, but could easily be
extended to do both.

[jbauman, 2016-05-09 21:20:43]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1963693003/ by jbauman@chromium.org.

The reason for reverting is: Seems to cause rendering issues on mac.

BUG=610181.