Revert "Move the code over using the same template type approach previously used for willPlayBackBitmaps in http://skbug.com/2702." This reverts commit 60c2a79cfa8ceebcbafc243407564dc71f5e3b4f. BUG=skia: Review URL: https://codereview.chromium.org/481173003
diff --git a/gyp/core.gypi b/gyp/core.gypi index f0b0869..b6b0ef1 100644 --- a/gyp/core.gypi +++ b/gyp/core.gypi
@@ -156,6 +156,8 @@ '<(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 d6c56ac..004f130 100644 --- a/include/core/SkPicture.h +++ b/include/core/SkPicture.h
@@ -305,16 +305,7 @@ SkAutoTDelete<SkRecord> fRecord; SkAutoTUnref<SkBBoxHierarchy> fBBH; - - struct Analysis { - Analysis() - : fWillPlaybackBitmaps(false) - , fSuitableForGpuRasterization(false) { } - Analysis(const SkRecord&); - - bool fWillPlaybackBitmaps; - bool fSuitableForGpuRasterization; - } const fAnalysis; + bool fRecordWillPlayBackBitmaps; // TODO: const }; #endif
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp index 1fc2bb3..8825123 100644 --- a/src/core/SkPicture.cpp +++ b/src/core/SkPicture.cpp
@@ -19,13 +19,11 @@ #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" @@ -48,180 +46,12 @@ /////////////////////////////////////////////////////////////////////////////// -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) { + , fHeight(0) + , fRecordWillPlayBackBitmaps(false) { this->needsNewGenID(); } #endif @@ -231,7 +61,8 @@ const SkPictureRecord& record, bool deepCopyOps) : fWidth(width) - , fHeight(height) { + , fHeight(height) + , fRecordWillPlayBackBitmaps(false) { this->needsNewGenID(); SkPictInfo info; @@ -306,6 +137,7 @@ } 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; @@ -438,7 +270,8 @@ SkPicture::SkPicture(SkPictureData* data, int width, int height) : fData(data) , fWidth(width) - , fHeight(height) { + , fHeight(height) + , fRecordWillPlayBackBitmaps(false) { this->needsNewGenID(); } @@ -553,11 +386,8 @@ } #if SK_SUPPORT_GPU -// fRecord OK +// fRecord TODO 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."; @@ -577,7 +407,7 @@ // fRecord OK bool SkPicture::willPlayBackBitmaps() const { if (fRecord.get()) { - return fAnalysis.fWillPlaybackBitmaps; + return fRecordWillPlayBackBitmaps; } if (!fData.get()) { return false; @@ -611,7 +441,7 @@ , fHeight(height) , fRecord(record) , fBBH(SkSafeRef(bbh)) - , fAnalysis(*record) { + , fRecordWillPlayBackBitmaps(SkRecordWillPlaybackBitmaps(*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 new file mode 100644 index 0000000..0bfbaef --- /dev/null +++ b/src/core/SkRecordAnalysis.cpp
@@ -0,0 +1,66 @@ +#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 new file mode 100644 index 0000000..6bdd5bc --- /dev/null +++ b/src/core/SkRecordAnalysis.h
@@ -0,0 +1,8 @@ +#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 6f62d18..61cc502 100644 --- a/tests/PictureTest.cpp +++ b/tests/PictureTest.cpp
@@ -578,44 +578,6 @@ } } -#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; @@ -744,16 +706,11 @@ } #if SK_SUPPORT_GPU -#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) { +static void test_gpu_veto(skiatest::Reporter* reporter) { SkPictureRecorder recorder; - SkCanvas* canvas = GENERATE_CANVAS(recorder, useNewPath); + SkCanvas* canvas = recorder.beginRecording(100, 100); { SkPath path; path.moveTo(0, 0); @@ -775,7 +732,7 @@ REPORTER_ASSERT(reporter, !picture->suitableForGpuRasterization(NULL, &reason)); REPORTER_ASSERT(reporter, NULL != reason); - canvas = GENERATE_CANVAS(recorder, useNewPath); + canvas = recorder.beginRecording(100, 100); { SkPath path; @@ -797,7 +754,7 @@ // A lot of AA concave paths currently render an SkPicture undesireable for GPU rendering REPORTER_ASSERT(reporter, !picture->suitableForGpuRasterization(NULL)); - canvas = GENERATE_CANVAS(recorder, useNewPath); + canvas = recorder.beginRecording(100, 100); { SkPath path; @@ -820,37 +777,8 @@ 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) { @@ -1689,12 +1617,9 @@ test_unbalanced_save_restores(reporter); test_peephole(); #if SK_SUPPORT_GPU - test_gpu_veto(reporter, false); - test_gpu_veto(reporter, true); + test_gpu_veto(reporter); #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 2240ae9..2fcc1e9 100644 --- a/tests/RecordTest.cpp +++ b/tests/RecordTest.cpp
@@ -11,6 +11,7 @@ #include "SkImageInfo.h" #include "SkShader.h" #include "SkRecord.h" +#include "SkRecordAnalysis.h" #include "SkRecords.h" // Sums the area of any DrawRect command it sees. @@ -76,5 +77,37 @@ 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