diff options
author | 2014-08-18 15:07:13 -0700 | |
---|---|---|
committer | 2014-08-18 15:07:13 -0700 | |
commit | 60c2a79cfa8ceebcbafc243407564dc71f5e3b4f (patch) | |
tree | 6ad04385d0ec4aaf3d6ecc89eeec10eead08118e | |
parent | f128f53a5bc36e2a991302cb40848df580ad515f (diff) |
Move the code over using the same template type approach previously used for willPlayBackBitmaps in http://skbug.com/2702.
Also unifies that flag and this one into a struct so they and others can be computed together. The struct is stored const to enforce lifetime expectations. Adds a few new cases to the unit test.
BUG=skia:2700
R=mtklein@google.com, reed@google.com, robertphillips@google.com, tomhudson@google.com
Author: tomhudson@chromium.org
Review URL: https://codereview.chromium.org/364823009
-rw-r--r-- | gyp/core.gypi | 2 | ||||
-rw-r--r-- | include/core/SkPicture.h | 11 | ||||
-rw-r--r-- | src/core/SkPicture.cpp | 192 | ||||
-rw-r--r-- | src/core/SkRecordAnalysis.cpp | 66 | ||||
-rw-r--r-- | src/core/SkRecordAnalysis.h | 8 | ||||
-rw-r--r-- | tests/PictureTest.cpp | 85 | ||||
-rw-r--r-- | tests/RecordTest.cpp | 33 |
7 files changed, 271 insertions, 126 deletions
diff --git a/gyp/core.gypi b/gyp/core.gypi index b6b0ef1ee0..f0b08693cd 100644 --- a/gyp/core.gypi +++ b/gyp/core.gypi @@ -156,8 +156,6 @@ '<(skia_src_path)/core/SkRasterClip.cpp', '<(skia_src_path)/core/SkRasterizer.cpp', '<(skia_src_path)/core/SkReadBuffer.cpp', - '<(skia_src_path)/core/SkRecordAnalysis.cpp', - '<(skia_src_path)/core/SkRecordAnalysis.h', '<(skia_src_path)/core/SkRecordDraw.cpp', '<(skia_src_path)/core/SkRecordOpts.cpp', '<(skia_src_path)/core/SkRecorder.cpp', diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h index 004f130621..d6c56ac883 100644 --- a/include/core/SkPicture.h +++ b/include/core/SkPicture.h @@ -305,7 +305,16 @@ private: SkAutoTDelete<SkRecord> fRecord; SkAutoTUnref<SkBBoxHierarchy> fBBH; - bool fRecordWillPlayBackBitmaps; // TODO: const + + struct Analysis { + Analysis() + : fWillPlaybackBitmaps(false) + , fSuitableForGpuRasterization(false) { } + Analysis(const SkRecord&); + + bool fWillPlaybackBitmaps; + bool fSuitableForGpuRasterization; + } const fAnalysis; }; #endif diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp index 8825123336..1fc2bb32c8 100644 --- a/src/core/SkPicture.cpp +++ b/src/core/SkPicture.cpp @@ -19,11 +19,13 @@ #include "SkChunkAlloc.h" #include "SkDrawPictureCallback.h" #include "SkPaintPriv.h" +#include "SkPathEffect.h" #include "SkPicture.h" -#include "SkRecordAnalysis.h" #include "SkRegion.h" +#include "SkShader.h" #include "SkStream.h" #include "SkTDArray.h" +#include "SkTLogic.h" #include "SkTSearch.h" #include "SkTime.h" @@ -46,12 +48,180 @@ template <typename T> int SafeCount(const T* obj) { /////////////////////////////////////////////////////////////////////////////// +namespace { + +// Some commands have a paint, some have an optional paint. Either way, get back a pointer. +static const SkPaint* AsPtr(const SkPaint& p) { return &p; } +static const SkPaint* AsPtr(const SkRecords::Optional<SkPaint>& p) { return p; } + +/** SkRecords visitor to determine whether an instance may require an + "external" bitmap to rasterize. May return false positives. + Does not return true for bitmap text. + + Expected use is to determine whether images need to be decoded before + rasterizing a particular SkRecord. + */ +struct BitmapTester { + // Helpers. These create HasMember_bitmap and HasMember_paint. + SK_CREATE_MEMBER_DETECTOR(bitmap); + SK_CREATE_MEMBER_DETECTOR(paint); + + + // Main entry for visitor: + // If the command has a bitmap directly, return true. + // If the command has a paint and the paint has a bitmap, return true. + // Otherwise, return false. + template <typename T> + bool operator()(const T& r) { return CheckBitmap(r); } + + + // If the command has a bitmap, of course we're going to play back bitmaps. + template <typename T> + static SK_WHEN(HasMember_bitmap<T>, bool) CheckBitmap(const T&) { return true; } + + // If not, look for one in its paint (if it has a paint). + template <typename T> + static SK_WHEN(!HasMember_bitmap<T>, bool) CheckBitmap(const T& r) { return CheckPaint(r); } + + // If we have a paint, dig down into the effects looking for a bitmap. + template <typename T> + static SK_WHEN(HasMember_paint<T>, bool) CheckPaint(const T& r) { + const SkPaint* paint = AsPtr(r.paint); + if (paint) { + const SkShader* shader = paint->getShader(); + if (shader && + shader->asABitmap(NULL, NULL, NULL) == SkShader::kDefault_BitmapType) { + return true; + } + } + return false; + } + + // If we don't have a paint, that non-paint has no bitmap. + template <typename T> + static SK_WHEN(!HasMember_paint<T>, bool) CheckPaint(const T&) { return false; } +}; + +bool WillPlaybackBitmaps(const SkRecord& record) { + BitmapTester tester; + for (unsigned i = 0; i < record.count(); i++) { + if (record.visit<bool>(i, tester)) { + return true; + } + } + return false; +} + +/** SkRecords visitor to determine heuristically whether or not a SkPicture + will be performant when rasterized on the GPU. + */ +struct PathCounter { + SK_CREATE_MEMBER_DETECTOR(paint); + + PathCounter() + : numPaintWithPathEffectUses (0) + , numFastPathDashEffects (0) + , numAAConcavePaths (0) + , numAAHairlineConcavePaths (0) { + } + + void checkPaint(const SkPaint* paint) { + if (paint && paint->getPathEffect()) { + numPaintWithPathEffectUses++; + } + } + + void operator()(const SkRecords::DrawPoints& op) { + this->checkPaint(&op.paint); + const SkPathEffect* effect = op.paint.getPathEffect(); + if (effect) { + SkPathEffect::DashInfo info; + SkPathEffect::DashType dashType = effect->asADash(&info); + if (2 == op.count && SkPaint::kRound_Cap != op.paint.getStrokeCap() && + SkPathEffect::kDash_DashType == dashType && 2 == info.fCount) { + numFastPathDashEffects++; + } + } + } + + void operator()(const SkRecords::DrawPath& op) { + this->checkPaint(&op.paint); + if (op.paint.isAntiAlias() && !op.path.isConvex()) { + numAAConcavePaths++; + + if (SkPaint::kStroke_Style == op.paint.getStyle() && + 0 == op.paint.getStrokeWidth()) { + numAAHairlineConcavePaths++; + } + } + } + + template <typename T> + SK_WHEN(HasMember_paint<T>, void) operator()(const T& op) { + this->checkPaint(AsPtr(op.paint)); + } + + template <typename T> + SK_WHEN(!HasMember_paint<T>, void) operator()(const T& op) { /* do nothing */ } + + + int numPaintWithPathEffectUses; + int numFastPathDashEffects; + int numAAConcavePaths; + int numAAHairlineConcavePaths; +}; + +bool SuitableForGpuRasterization(const SkRecord& record, + const char** reason = NULL, + int sampleCount = 0) { + PathCounter counter; + for (unsigned i = 0; i < record.count(); i++) { + record.visit<void>(i, counter); + } + + // TODO: the heuristic used here needs to be refined + static const int kNumPaintWithPathEffectsUsesTol = 1; + static const int kNumAAConcavePathsTol = 5; + + int numNonDashedPathEffects = counter.numPaintWithPathEffectUses - + counter.numFastPathDashEffects; + bool suitableForDash = (0 == counter.numPaintWithPathEffectUses) || + (numNonDashedPathEffects < kNumPaintWithPathEffectsUsesTol + && 0 == sampleCount); + + bool ret = suitableForDash && + (counter.numAAConcavePaths - counter.numAAHairlineConcavePaths) + < kNumAAConcavePathsTol; + + if (!ret && NULL != reason) { + if (!suitableForDash) { + if (0 != sampleCount) { + *reason = "Can't use multisample on dash effect."; + } else { + *reason = "Too many non dashed path effects."; + } + } else if ((counter.numAAConcavePaths - counter.numAAHairlineConcavePaths) + >= kNumAAConcavePathsTol) + *reason = "Too many anti-aliased concave paths."; + else + *reason = "Unknown reason for GPU unsuitability."; + } + return ret; +} + +} // namespace + +SkPicture::Analysis::Analysis(const SkRecord& record) + : fWillPlaybackBitmaps (WillPlaybackBitmaps(record)) + , fSuitableForGpuRasterization (SuitableForGpuRasterization(record)) { } + +/////////////////////////////////////////////////////////////////////////////// + #ifdef SK_SUPPORT_LEGACY_DEFAULT_PICTURE_CTOR // fRecord OK SkPicture::SkPicture() : fWidth(0) - , fHeight(0) - , fRecordWillPlayBackBitmaps(false) { + , fHeight(0) { this->needsNewGenID(); } #endif @@ -61,8 +231,7 @@ SkPicture::SkPicture(int width, int height, const SkPictureRecord& record, bool deepCopyOps) : fWidth(width) - , fHeight(height) - , fRecordWillPlayBackBitmaps(false) { + , fHeight(height) { this->needsNewGenID(); SkPictInfo info; @@ -137,7 +306,6 @@ SkPicture* SkPicture::clone() const { } SkPicture* clone = SkNEW_ARGS(SkPicture, (newData.detach(), fWidth, fHeight)); - clone->fRecordWillPlayBackBitmaps = fRecordWillPlayBackBitmaps; clone->fUniqueID = this->uniqueID(); // need to call method to ensure != 0 return clone; @@ -270,8 +438,7 @@ bool SkPicture::InternalOnly_BufferIsSKP(SkReadBuffer& buffer, SkPictInfo* pInfo SkPicture::SkPicture(SkPictureData* data, int width, int height) : fData(data) , fWidth(width) - , fHeight(height) - , fRecordWillPlayBackBitmaps(false) { + , fHeight(height) { this->needsNewGenID(); } @@ -386,8 +553,11 @@ void SkPicture::flatten(SkWriteBuffer& buffer) const { } #if SK_SUPPORT_GPU -// fRecord TODO +// fRecord OK bool SkPicture::suitableForGpuRasterization(GrContext* context, const char **reason) const { + if (fRecord.get()) { + return fAnalysis.fSuitableForGpuRasterization; + } if (NULL == fData.get()) { if (NULL != reason) { *reason = "Missing internal data."; @@ -407,7 +577,7 @@ bool SkPicture::hasText() const { // fRecord OK bool SkPicture::willPlayBackBitmaps() const { if (fRecord.get()) { - return fRecordWillPlayBackBitmaps; + return fAnalysis.fWillPlaybackBitmaps; } if (!fData.get()) { return false; @@ -441,7 +611,7 @@ SkPicture::SkPicture(int width, int height, SkRecord* record, SkBBoxHierarchy* b , fHeight(height) , fRecord(record) , fBBH(SkSafeRef(bbh)) - , fRecordWillPlayBackBitmaps(SkRecordWillPlaybackBitmaps(*record)) { + , fAnalysis(*record) { // TODO: delay as much of this work until just before first playback? if (fBBH.get()) { SkRecordFillBounds(*record, fBBH.get()); diff --git a/src/core/SkRecordAnalysis.cpp b/src/core/SkRecordAnalysis.cpp deleted file mode 100644 index 0bfbaef9a6..0000000000 --- a/src/core/SkRecordAnalysis.cpp +++ /dev/null @@ -1,66 +0,0 @@ -#include "SkRecordAnalysis.h" - -#include "SkShader.h" -#include "SkTLogic.h" - -/** SkRecords visitor to determine whether an instance may require an - "external" bitmap to rasterize. May return false positives. - Does not return true for bitmap text. - - Expected use is to determine whether images need to be decoded before - rasterizing a particular SkRecord. - */ -struct BitmapTester { - // Helpers. These create HasMember_bitmap and HasMember_paint. - SK_CREATE_MEMBER_DETECTOR(bitmap); - SK_CREATE_MEMBER_DETECTOR(paint); - - // Some commands have a paint, some have an optional paint. Either way, get back a pointer. - static const SkPaint* AsPtr(const SkPaint& p) { return &p; } - static const SkPaint* AsPtr(const SkRecords::Optional<SkPaint>& p) { return p; } - - - // Main entry for visitor: - // If the command has a bitmap directly, return true. - // If the command has a paint and the paint has a bitmap, return true. - // Otherwise, return false. - template <typename T> - bool operator()(const T& r) { return CheckBitmap(r); } - - - // If the command has a bitmap, of course we're going to play back bitmaps. - template <typename T> - static SK_WHEN(HasMember_bitmap<T>, bool) CheckBitmap(const T&) { return true; } - - // If not, look for one in its paint (if it has a paint). - template <typename T> - static SK_WHEN(!HasMember_bitmap<T>, bool) CheckBitmap(const T& r) { return CheckPaint(r); } - - // If we have a paint, dig down into the effects looking for a bitmap. - template <typename T> - static SK_WHEN(HasMember_paint<T>, bool) CheckPaint(const T& r) { - const SkPaint* paint = AsPtr(r.paint); - if (paint) { - const SkShader* shader = paint->getShader(); - if (shader && - shader->asABitmap(NULL, NULL, NULL) == SkShader::kDefault_BitmapType) { - return true; - } - } - return false; - } - - // If we don't have a paint, that non-paint has no bitmap. - template <typename T> - static SK_WHEN(!HasMember_paint<T>, bool) CheckPaint(const T&) { return false; } -}; - -bool SkRecordWillPlaybackBitmaps(const SkRecord& record) { - BitmapTester tester; - for (unsigned i = 0; i < record.count(); i++) { - if (record.visit<bool>(i, tester)) { - return true; - } - } - return false; -} diff --git a/src/core/SkRecordAnalysis.h b/src/core/SkRecordAnalysis.h deleted file mode 100644 index 6bdd5bc311..0000000000 --- a/src/core/SkRecordAnalysis.h +++ /dev/null @@ -1,8 +0,0 @@ -#ifndef SkRecordAnalysis_DEFINED -#define SkRecordAnalysis_DEFINED - -#include "SkRecord.h" - -bool SkRecordWillPlaybackBitmaps(const SkRecord& record); - -#endif // SkRecordAnalysis_DEFINED diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp index 61cc5021fc..6f62d18538 100644 --- a/tests/PictureTest.cpp +++ b/tests/PictureTest.cpp @@ -578,6 +578,44 @@ static void test_gatherpixelrefs(skiatest::Reporter* reporter) { } } +#define GENERATE_CANVAS(recorder, x) \ + (x) ? recorder.EXPERIMENTAL_beginRecording(100, 100) \ + : recorder.beginRecording(100,100); + +/* Hit a few SkPicture::Analysis cases not handled elsewhere. */ +static void test_analysis(skiatest::Reporter* reporter, bool useNewPath) { + SkPictureRecorder recorder; + + SkCanvas* canvas = GENERATE_CANVAS(recorder, useNewPath); + { + canvas->drawRect(SkRect::MakeWH(10, 10), SkPaint ()); + } + SkAutoTUnref<SkPicture> picture(recorder.endRecording()); + REPORTER_ASSERT(reporter, !picture->willPlayBackBitmaps()); + + canvas = GENERATE_CANVAS(recorder, useNewPath); + { + SkPaint paint; + // CreateBitmapShader is too smart for us; an empty (or 1x1) bitmap shader + // gets optimized into a non-bitmap form, so we create a 2x2 bitmap here. + SkBitmap bitmap; + bitmap.allocPixels(SkImageInfo::MakeN32Premul(2, 2)); + bitmap.eraseColor(SK_ColorBLUE); + *(bitmap.getAddr32(0, 0)) = SK_ColorGREEN; + SkShader* shader = SkShader::CreateBitmapShader(bitmap, SkShader::kClamp_TileMode, + SkShader::kClamp_TileMode); + paint.setShader(shader)->unref(); + REPORTER_ASSERT(reporter, + shader->asABitmap(NULL, NULL, NULL) == SkShader::kDefault_BitmapType); + + canvas->drawRect(SkRect::MakeWH(10, 10), paint); + } + picture.reset(recorder.endRecording()); + REPORTER_ASSERT(reporter, picture->willPlayBackBitmaps()); +} + +#undef GENERATE_CANVAS + static void test_gatherpixelrefsandrects(skiatest::Reporter* reporter) { const int IW = 32; const int IH = IW; @@ -706,11 +744,16 @@ static void rand_op(SkCanvas* canvas, SkRandom& rand) { } #if SK_SUPPORT_GPU -static void test_gpu_veto(skiatest::Reporter* reporter) { +#define GENERATE_CANVAS(recorder, x) \ + (x) ? recorder.EXPERIMENTAL_beginRecording(100, 100) \ + : recorder.beginRecording(100,100); + +static void test_gpu_veto(skiatest::Reporter* reporter, + bool useNewPath) { SkPictureRecorder recorder; - SkCanvas* canvas = recorder.beginRecording(100, 100); + SkCanvas* canvas = GENERATE_CANVAS(recorder, useNewPath); { SkPath path; path.moveTo(0, 0); @@ -732,7 +775,7 @@ static void test_gpu_veto(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, !picture->suitableForGpuRasterization(NULL, &reason)); REPORTER_ASSERT(reporter, NULL != reason); - canvas = recorder.beginRecording(100, 100); + canvas = GENERATE_CANVAS(recorder, useNewPath); { SkPath path; @@ -754,7 +797,7 @@ static void test_gpu_veto(skiatest::Reporter* reporter) { // A lot of AA concave paths currently render an SkPicture undesireable for GPU rendering REPORTER_ASSERT(reporter, !picture->suitableForGpuRasterization(NULL)); - canvas = recorder.beginRecording(100, 100); + canvas = GENERATE_CANVAS(recorder, useNewPath); { SkPath path; @@ -777,8 +820,37 @@ static void test_gpu_veto(skiatest::Reporter* reporter) { picture.reset(recorder.endRecording()); // hairline stroked AA concave paths are fine for GPU rendering REPORTER_ASSERT(reporter, picture->suitableForGpuRasterization(NULL)); + + canvas = GENERATE_CANVAS(recorder, useNewPath); + { + SkPaint paint; + SkScalar intervals [] = { 10, 20 }; + SkPathEffect* pe = SkDashPathEffect::Create(intervals, 2, 25); + paint.setPathEffect(pe)->unref(); + + SkPoint points [2] = { { 0, 0 }, { 100, 0 } }; + canvas->drawPoints(SkCanvas::kLines_PointMode, 2, points, paint); + } + picture.reset(recorder.endRecording()); + // fast-path dashed effects are fine for GPU rendering ... + REPORTER_ASSERT(reporter, picture->suitableForGpuRasterization(NULL)); + + canvas = GENERATE_CANVAS(recorder, useNewPath); + { + SkPaint paint; + SkScalar intervals [] = { 10, 20 }; + SkPathEffect* pe = SkDashPathEffect::Create(intervals, 2, 25); + paint.setPathEffect(pe)->unref(); + + canvas->drawRect(SkRect::MakeWH(10, 10), paint); + } + picture.reset(recorder.endRecording()); + // ... but only when applied to drawPoint() calls + REPORTER_ASSERT(reporter, !picture->suitableForGpuRasterization(NULL)); } +#undef GENERATE_CANVAS + static void test_gpu_picture_optimization(skiatest::Reporter* reporter, GrContextFactory* factory) { for (int i= 0; i < GrContextFactory::kGLContextTypeCnt; ++i) { @@ -1617,9 +1689,12 @@ DEF_TEST(Picture, reporter) { test_unbalanced_save_restores(reporter); test_peephole(); #if SK_SUPPORT_GPU - test_gpu_veto(reporter); + test_gpu_veto(reporter, false); + test_gpu_veto(reporter, true); #endif test_has_text(reporter); + test_analysis(reporter, false); + test_analysis(reporter, true); test_gatherpixelrefs(reporter); test_gatherpixelrefsandrects(reporter); test_bitmap_with_encoded_data(reporter); diff --git a/tests/RecordTest.cpp b/tests/RecordTest.cpp index 2fcc1e9c53..2240ae9858 100644 --- a/tests/RecordTest.cpp +++ b/tests/RecordTest.cpp @@ -11,7 +11,6 @@ #include "SkImageInfo.h" #include "SkShader.h" #include "SkRecord.h" -#include "SkRecordAnalysis.h" #include "SkRecords.h" // Sums the area of any DrawRect command it sees. @@ -77,37 +76,5 @@ DEF_TEST(Record, r) { REPORTER_ASSERT(r, summer.area() == 500); } -DEF_TEST(RecordAnalysis, r) { - SkRecord record; - - SkRect rect = SkRect::MakeWH(10, 10); - SkPaint paint; - APPEND(record, SkRecords::DrawRect, paint, rect); - REPORTER_ASSERT(r, !SkRecordWillPlaybackBitmaps(record)); - - SkBitmap bitmap; - APPEND(record, SkRecords::DrawBitmap, &paint, bitmap, 0.0f, 0.0f); - REPORTER_ASSERT(r, SkRecordWillPlaybackBitmaps(record)); - - SkNEW_PLACEMENT_ARGS(record.replace<SkRecords::DrawRect>(1), - SkRecords::DrawRect, (paint, rect)); - REPORTER_ASSERT(r, !SkRecordWillPlaybackBitmaps(record)); - - SkPaint paint2; - // CreateBitmapShader is too smart for us; an empty (or 1x1) bitmap shader - // gets optimized into a non-bitmap form, so we create a 2x2 bitmap here. - SkBitmap bitmap2; - bitmap2.allocPixels(SkImageInfo::MakeN32Premul(2, 2)); - bitmap2.eraseColor(SK_ColorBLUE); - *(bitmap2.getAddr32(0, 0)) = SK_ColorGREEN; - SkShader* shader = SkShader::CreateBitmapShader(bitmap2, SkShader::kClamp_TileMode, - SkShader::kClamp_TileMode); - paint2.setShader(shader)->unref(); - REPORTER_ASSERT(r, shader->asABitmap(NULL, NULL, NULL) == SkShader::kDefault_BitmapType); - - APPEND(record, SkRecords::DrawRect, paint2, rect); - REPORTER_ASSERT(r, SkRecordWillPlaybackBitmaps(record)); -} - #undef APPEND |