diff options
| author |  mtklein <mtklein@chromium.org> | 2014-09-10 16:08:27 -0700 |
|---|---|---|
| committer |  Commit bot <commit-bot@chromium.org> | 2014-09-10 16:08:28 -0700 |
| commit | 99d6a9ee8b3516de892d118c71aa5e6e5c865efd (patch) | |
| tree | db1f93c0fb435d1eac274d302d65eac45a9abf88 | |
| parent | f91c47d91d72a1d85e2d6701864b8d7accc81647 (diff) | |
Fix a bug in Save-Restore no-op optimization.
We optimize
Save
SaveLayer
Restore
Restore
into
NoOp
NoOp
NoOp
Restore
I'm considering skipping the call to SkRecordOptimize again just to eliminate
this extra variable from landing SkRecord. Thoughts?
BUG=skia:
R=robertphillips@google.com, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/560163002
| -rw-r--r-- | src/core/SkRecordOpts.cpp | 4 | ||||
| -rw-r--r-- | src/core/SkRecordPattern.h | 4 | ||||
| -rw-r--r-- | tests/RecordOptsTest.cpp | 17 |
3 files changed, 24 insertions, 1 deletions
diff --git a/src/core/SkRecordOpts.cpp b/src/core/SkRecordOpts.cpp index 3302d0cb4d..47718e102c 100644 --- a/src/core/SkRecordOpts.cpp +++ b/src/core/SkRecordOpts.cpp @@ -55,8 +55,10 @@ struct SaveOnlyDrawsRestoreNooper { // Turns logical no-op Save-[non-drawing command]*-Restore patterns into actual no-ops. struct SaveNoDrawsRestoreNooper { // Star matches greedily, so we also have to exclude Save and Restore. + // Nested SaveLayers need to be excluded, or we'll match their Restore! typedef Pattern3<Is<Save>, - Star<Not<Or3<Is<Save>, + Star<Not<Or4<Is<Save>, + Is<SaveLayer>, Is<Restore>, IsDraw> > >, Is<Restore> > diff --git a/src/core/SkRecordPattern.h b/src/core/SkRecordPattern.h index 57779ffd76..68a3aa315e 100644 --- a/src/core/SkRecordPattern.h +++ b/src/core/SkRecordPattern.h @@ -85,6 +85,10 @@ struct Or { template <typename A, typename B, typename C> struct Or3 : Or<A, Or<B, C> > {}; +// Matches if any of A, B, C or D does. Stores nothing. +template <typename A, typename B, typename C, typename D> +struct Or4 : Or<A, Or<B, Or<C, D> > > {}; + // Star is a special matcher that greedily matches Matcher 0 or more times. Stores nothing. template <typename Matcher> struct Star { diff --git a/tests/RecordOptsTest.cpp b/tests/RecordOptsTest.cpp index dca9482197..e17b5e6bfa 100644 --- a/tests/RecordOptsTest.cpp +++ b/tests/RecordOptsTest.cpp @@ -99,6 +99,23 @@ DEF_TEST(RecordOpts_NoopSaveRestores, r) { } } +DEF_TEST(RecordOpts_SaveSaveLayerRestoreRestore, r) { + SkRecord record; + SkRecorder recorder(&record, W, H); + + // A previous bug NoOp'd away the first 3 commands. + recorder.save(); + recorder.saveLayer(NULL, NULL); + recorder.restore(); + recorder.restore(); + + SkRecordNoopSaveRestores(&record); + assert_type<SkRecords::Save> (r, record, 0); + assert_type<SkRecords::SaveLayer>(r, record, 1); + assert_type<SkRecords::Restore> (r, record, 2); + assert_type<SkRecords::Restore> (r, record, 3); +} + static void assert_savelayer_restore(skiatest::Reporter* r, SkRecord* record, unsigned i, |