Track slow paths in DisplayItemList

Track slow paths in DisplayItemList

Previously, DisplayItemList only stored a boolean for whether or not
the entire DisplayItemList was suitable for gpu raster.  (This is
used for turning on msaa or not, but it hasn't been renamed).

PaintOpBuffer stores indirections to DisplayItemList and so needs to
properly track the number of slow paths stored inside of it.  The
solution is to push down the msaa veto logic from PaintController
down into PictureLayer and store slow path count the whole way.

I think this is more compatible with spv2, which will want to
combine paint artifacts and then decide if the result needs
msaa or not.

Also, issue 513016 is quite solved, so comments related to it can be
removed.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2876033005
Cr-Commit-Position: refs/heads/master@{#476040}
Committed: https://chromium.googlesource.com/chromium/src/+/36ee28f4f311d6f307de81dc4fa42fed14c4bac7

[enne (OOO), 2017-05-12 22:58:39]

Description was changed from

==========
Track slow paths in DisplayItemList

Previously, DisplayItemList only stored a boolean for whether or not
the entire DisplayItemList was suitable for gpu raster.  (This is
used for turning on msaa or not, but it hasn't been renamed).

PaintOpBuffer stores indirections to DisplayItemList and so needs to
properly track the number of slow paths stored inside of it.  The
solution is to push down the msaa veto logic from PaintController
down into PictureLayer and store slow path count the whole way.

I think this is more compatible with spv2, which will want to
combine paint artifacts and then decide if the result needs
msaa or not.

Also, issue 513016 is quite solved, so comments related to it can be
removed.
==========

to

==========
Track slow paths in DisplayItemList

Previously, DisplayItemList only stored a boolean for whether or not
the entire DisplayItemList was suitable for gpu raster.  (This is
used for turning on msaa or not, but it hasn't been renamed).

PaintOpBuffer stores indirections to DisplayItemList and so needs to
properly track the number of slow paths stored inside of it.  The
solution is to push down the msaa veto logic from PaintController
down into PictureLayer and store slow path count the whole way.

I think this is more compatible with spv2, which will want to
combine paint artifacts and then decide if the result needs
msaa or not.

Also, issue 513016 is quite solved, so comments related to it can be
removed.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[enne (OOO), 2017-05-12 22:58:40]

Description was changed from

==========
Track slow paths in DisplayItemList

Previously, DisplayItemList only stored a boolean for whether or not
the entire DisplayItemList was suitable for gpu raster.  (This is
used for turning on msaa or not, but it hasn't been renamed).

PaintOpBuffer stores indirections to DisplayItemList and so needs to
properly track the number of slow paths stored inside of it.  The
solution is to push down the msaa veto logic from PaintController
down into PictureLayer and store slow path count the whole way.

I think this is more compatible with spv2, which will want to
combine paint artifacts and then decide if the result needs
msaa or not.

Also, issue 513016 is quite solved, so comments related to it can be
removed.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Track slow paths in DisplayItemList

Previously, DisplayItemList only stored a boolean for whether or not
the entire DisplayItemList was suitable for gpu raster.  (This is
used for turning on msaa or not, but it hasn't been renamed).

PaintOpBuffer stores indirections to DisplayItemList and so needs to
properly track the number of slow paths stored inside of it.  The
solution is to push down the msaa veto logic from PaintController
down into PictureLayer and store slow path count the whole way.

I think this is more compatible with spv2, which will want to
combine paint artifacts and then decide if the result needs
msaa or not.

Also, issue 513016 is quite solved, so comments related to it can be
removed.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[enne (OOO), 2017-05-12 22:59:56]

enne@chromium.org changed reviewers:
+ chrishtr@chromium.org

[enne (OOO), 2017-05-12 22:59:56]

This depends on https://chromium-review.googlesource.com/c/503472/ and
https://codereview.chromium.org/2875343002/ (and the tests in the comment
there).

Couldn't figure out how to make gerrit upload a patch that depended on danakj's
gerrit patch, so here's some rietveld.

[chrishtr, 2017-05-13 01:12:04]

https://codereview.chromium.org/2876033005/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
(right):

https://codereview.chromium.org/2876033005/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp:575:
num_slow_paths += item.NumberOfSlowPaths();
Why remove the early-out?

[enne (OOO), 2017-05-13 18:27:41]

https://codereview.chromium.org/2876033005/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
(right):

https://codereview.chromium.org/2876033005/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp:575:
num_slow_paths += item.NumberOfSlowPaths();
On 2017/05/13 at 01:12:04, chrishtr wrote:
> Why remove the early-out?

Because number of slow paths is a member accessor now and the comment is out of
date.  I didn't want to duplicate kMaxNumberOfSlowPathsBeforeVeto here, and a
conditional to avoid a member access (on an object that is already being used
below) does not seem like it's worth doing even without the code duplication
issue.

[chrishtr, 2017-05-15 18:08:41]

lgtm

[enne (OOO), 2017-05-23 20:34:44]

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

[enne (OOO), 2017-05-23 20:34:54]

enne@chromium.org changed reviewers:
+ danakj@chromium.org

[enne (OOO), 2017-05-23 20:35:59]

danakj: cq+2 if'n you like

[commit-bot: I haz the power, 2017-05-23 20:36:33]

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-05-23 22:46:33]

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

[commit-bot: I haz the power, 2017-05-23 22:46:34]

Dry run: This issue passed the CQ dry run.

[danakj, 2017-05-31 18:03:38]

https://codereview.chromium.org/2876033005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/paint/PaintArtifact.cpp
(right):

https://codereview.chromium.org/2876033005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintArtifact.cpp:76:
paint_chunks_.clear();
reset it here?

https://codereview.chromium.org/2876033005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp
(right):

https://codereview.chromium.org/2876033005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp:1927:
EXPECT_EQ(GetPaintController().GetPaintArtifact().NumSlowPaths(), 1);
These EXPECT_EQ's are backwards, expected first

[danakj, 2017-05-31 18:03:46]

LGTM % that stuff

[enne (OOO), 2017-05-31 18:18:44]

The CQ bit was checked by enne@chromium.org

[enne (OOO), 2017-05-31 18:18:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org, danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/2876033005/#ps40001
(title: "danakj review")

[commit-bot: I haz the power, 2017-05-31 18:19: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-05-31 21:42:12]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496254724575150,
"parent_rev": "4323686ff309aa553948ed0f5a9647b1172c64e2", "commit_rev":
"36ee28f4f311d6f307de81dc4fa42fed14c4bac7"}

[commit-bot: I haz the power, 2017-05-31 21:42:25]

Description was changed from

==========
Track slow paths in DisplayItemList

Previously, DisplayItemList only stored a boolean for whether or not
the entire DisplayItemList was suitable for gpu raster.  (This is
used for turning on msaa or not, but it hasn't been renamed).

PaintOpBuffer stores indirections to DisplayItemList and so needs to
properly track the number of slow paths stored inside of it.  The
solution is to push down the msaa veto logic from PaintController
down into PictureLayer and store slow path count the whole way.

I think this is more compatible with spv2, which will want to
combine paint artifacts and then decide if the result needs
msaa or not.

Also, issue 513016 is quite solved, so comments related to it can be
removed.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Track slow paths in DisplayItemList

Previously, DisplayItemList only stored a boolean for whether or not
the entire DisplayItemList was suitable for gpu raster.  (This is
used for turning on msaa or not, but it hasn't been renamed).

PaintOpBuffer stores indirections to DisplayItemList and so needs to
properly track the number of slow paths stored inside of it.  The
solution is to push down the msaa veto logic from PaintController
down into PictureLayer and store slow path count the whole way.

I think this is more compatible with spv2, which will want to
combine paint artifacts and then decide if the result needs
msaa or not.

Also, issue 513016 is quite solved, so comments related to it can be
removed.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2876033005
Cr-Commit-Position: refs/heads/master@{#476040}
Committed:
https://chromium.googlesource.com/chromium/src/+/36ee28f4f311d6f307de81dc4fa4...
==========

[commit-bot: I haz the power, 2017-05-31 21:42:27]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/36ee28f4f311d6f307de81dc4fa4...