Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2875343002:
cc: fix for MSAA veto. (Closed)
|
Created:
3 years, 7 months ago by Stephen White Modified:
3 years, 7 months ago Reviewers:
enne (OOO) CC:
cc-bugs_chromium.org, chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Descriptioncc: fix for MSAA veto.
(Tests by enne@.)
Recurse into RecordDrawOp when counting slow paths.
BUG=718057
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2875343002
Cr-Commit-Position: refs/heads/master@{#471929}
Committed: https://chromium.googlesource.com/chromium/src/+/4b04a623b3af961b6284755050add2395e1d46f6
Patch Set 1 #Patch Set 2 : Revert whitespace change. #Patch Set 3 : Add tests from enne@. #Patch Set 4 : Windows fix. #
Messages
Total messages: 24 (15 generated)
Description was changed from ========== cc: fix for MSAA veto. Recurse into RecordDrawOp when counting slow paths. BUG=718057 ========== to ========== cc: fix for MSAA veto. Recurse into RecordDrawOp when counting slow paths. BUG=718057 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run
Description was changed from ========== cc: fix for MSAA veto. Recurse into RecordDrawOp when counting slow paths. BUG=718057 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== cc: fix for MSAA veto. Recurse into DrawRecordOp when counting slow paths. BUG=718057 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
enne@chromium.org changed reviewers: + enne@chromium.org
diff --git a/cc/paint/paint_op_buffer_unittest.cc
b/cc/paint/paint_op_buffer_unittest.cc
index 6c0de66a73c3..bd56dcab7d2f 100644
--- a/cc/paint/paint_op_buffer_unittest.cc
+++ b/cc/paint/paint_op_buffer_unittest.cc
@@ -7,6 +7,7 @@
#include "cc/test/skia_common.h"
#include "cc/test/test_skcanvas.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/effects/SkDashPathEffect.h"
namespace {
@@ -475,4 +476,54 @@ TEST(PaintOpBufferTest,
DiscardableImagesTracking_OpWithFlags) {
EXPECT_TRUE(buffer.HasDiscardableImages());
}
+TEST(PaintOpBufferTest, SlowPaths) {
+ auto buffer = sk_make_sp<PaintOpBuffer>();
+ EXPECT_EQ(buffer->numSlowPaths(), 0);
+
+ // Op without slow paths
+ PaintFlags noop_flags;
+ SkRect rect = SkRect::MakeXYWH(2, 3, 4, 5);
+ buffer->push<SaveLayerOp>(&rect, &noop_flags);
+
+ // Line op with a slow path
+ PaintFlags line_effect_slow;
+ line_effect_slow.setStrokeWidth(1.f);
+ line_effect_slow.setStyle(PaintFlags::kStroke_Style);
+ line_effect_slow.setStrokeCap(PaintFlags::kRound_Cap);
+ SkScalar intervals[] = {1.f, 1.f};
+ line_effect_slow.setPathEffect(SkDashPathEffect::Make(intervals, 2, 0));
+
+ buffer->push<DrawLineOp>(1, 2, 3, 4, line_effect_slow);
+ EXPECT_EQ(buffer->numSlowPaths(), 1);
+
+ // Line effect special case that Skia handles specially.
+ PaintFlags line_effect = line_effect_slow;
+ line_effect.setStrokeCap(PaintFlags::kButt_Cap);
+ buffer->push<DrawLineOp>(1, 2, 3, 4, line_effect);
+ EXPECT_EQ(buffer->numSlowPaths(), 1);
+
+ // Antialiased convex path is not slow.
+ SkPath path;
+ path.addCircle(2, 2, 5);
+ EXPECT_TRUE(path.isConvex());
+ buffer->push<ClipPathOp>(path, SkClipOp::kIntersect, true);
+ EXPECT_EQ(buffer->numSlowPaths(), 1);
+
+ // Concave paths are slow only when antialiased.
+ SkPath concave = path;
+ concave.addCircle(3, 4, 2);
+ EXPECT_FALSE(concave.isConvex());
+ buffer->push<ClipPathOp>(concave, SkClipOp::kIntersect, true);
+ EXPECT_EQ(buffer->numSlowPaths(), 2);
+ buffer->push<ClipPathOp>(concave, SkClipOp::kIntersect, false);
+ EXPECT_EQ(buffer->numSlowPaths(), 2);
+
+ // Drawing a record with slow paths into another adds the same
+ // number of slow paths as the record.
+ auto buffer2 = sk_make_sp<PaintOpBuffer>();
+ EXPECT_EQ(buffer2->numSlowPaths(), 0);
+ buffer2->push<DrawRecordOp>(buffer);
+ EXPECT_EQ(buffer->numSlowPaths(), buffer2->numSlowPaths());
+}
+
} // namespace cc
lgtm. Here's some tests you wanted. I can land them separately if you just want to land this and fix the bug. I'll follow up on the display item list counting too, which is also wrong, but is a bit more invasive to change.
On 2017/05/12 18:51:02, enne wrote: > lgtm. Here's some tests you wanted. I can land them separately if you just > want to land this and fix the bug. Great, thanks! I'll attach them to this bug. > I'll follow up on the display item list counting too, which is also wrong, but > is a bit more invasive to change. SGTM
Description was changed from ========== cc: fix for MSAA veto. Recurse into RecordDrawOp when counting slow paths. BUG=718057 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== cc: fix for MSAA veto. (Tests by enne@.) Recurse into RecordDrawOp when counting slow paths. BUG=718057 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
The CQ bit was checked by senorblanco@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org Link to the patchset: https://codereview.chromium.org/2875343002/#ps40001 (title: "Add tests from enne@.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by senorblanco@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org Link to the patchset: https://codereview.chromium.org/2875343002/#ps60001 (title: "Windows fix.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by senorblanco@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494875648533800,
"parent_rev": "ecfbc2ccd1072ed4f01a4a9c902fc5be9ed458a4", "commit_rev":
"4b04a623b3af961b6284755050add2395e1d46f6"}
Message was sent while issue was closed.
Description was changed from ========== cc: fix for MSAA veto. (Tests by enne@.) Recurse into RecordDrawOp when counting slow paths. BUG=718057 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== cc: fix for MSAA veto. (Tests by enne@.) Recurse into RecordDrawOp when counting slow paths. BUG=718057 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2875343002 Cr-Commit-Position: refs/heads/master@{#471929} Committed: https://chromium.googlesource.com/chromium/src/+/4b04a623b3af961b6284755050ad... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/4b04a623b3af961b6284755050ad... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
