From 300c6060c855cf607accaed969238b928833623d Mon Sep 17 00:00:00 2001 From: "robertphillips@google.com" Date: Mon, 5 May 2014 19:24:23 +0000 Subject: [PATCH] Revert r14571 (Infrastructure changes to support pull saveLayers forward task - https://codereview.chromium.org/266203003) due to breaking Android unit tests git-svn-id: http://skia.googlecode.com/svn/trunk@14578 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/core/SkPaintPriv.cpp | 21 ------ src/core/SkPaintPriv.h | 7 -- src/core/SkPicture.cpp | 23 +++++- src/gpu/GrPictureUtils.cpp | 43 +---------- src/gpu/GrPictureUtils.h | 23 ------ src/gpu/SkGpuDevice.cpp | 49 +++++++++++- tests/PictureTest.cpp | 150 ------------------------------------- 7 files changed, 72 insertions(+), 244 deletions(-) diff --git a/src/core/SkPaintPriv.cpp b/src/core/SkPaintPriv.cpp index 65fd0e7555..ce05389019 100644 --- a/src/core/SkPaintPriv.cpp +++ b/src/core/SkPaintPriv.cpp @@ -76,24 +76,3 @@ bool isPaintOpaque(const SkPaint* paint, } return false; } - -bool NeedsDeepCopy(const SkPaint& paint) { - /* - * These fields are known to be immutable, and so can be shallow-copied - * - * getTypeface() - * getAnnotation() - * paint.getColorFilter() - * getXfermode() - * getPathEffect() - * getMaskFilter() - */ - - return paint.getShader() || -#ifdef SK_SUPPORT_LEGACY_LAYERRASTERIZER_API - paint.getRasterizer() || -#endif - paint.getLooper() || // needs to hide its addLayer... - paint.getImageFilter(); -} - diff --git a/src/core/SkPaintPriv.h b/src/core/SkPaintPriv.h index 9668fef127..38c9063e56 100644 --- a/src/core/SkPaintPriv.h +++ b/src/core/SkPaintPriv.h @@ -22,11 +22,4 @@ class SkPaint; */ bool isPaintOpaque(const SkPaint* paint, const SkBitmap* bmpReplacesShader = NULL); - -/** Returns true if the provided paint has fields which are not - immutable (and will thus require deep copying). - @param paint the paint to be analyzed - @return true if the paint requires a deep copy -*/ -bool NeedsDeepCopy(const SkPaint& paint); #endif diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp index 68434303fa..3b04906e09 100644 --- a/src/core/SkPicture.cpp +++ b/src/core/SkPicture.cpp @@ -15,7 +15,6 @@ #include "SkBitmapDevice.h" #include "SkCanvas.h" #include "SkChunkAlloc.h" -#include "SkPaintPriv.h" #include "SkPicture.h" #include "SkRegion.h" #include "SkStream.h" @@ -218,6 +217,26 @@ SkPicture* SkPicture::clone() const { return clonedPicture; } +static bool needs_deep_copy(const SkPaint& paint) { + /* + * These fields are known to be immutable, and so can be shallow-copied + * + * getTypeface() + * getAnnotation() + * paint.getColorFilter() + * getXfermode() + * getPathEffect() + * getMaskFilter() + */ + + return paint.getShader() || +#ifdef SK_SUPPORT_LEGACY_LAYERRASTERIZER_API + paint.getRasterizer() || +#endif + paint.getLooper() || // needs to hide its addLayer... + paint.getImageFilter(); +} + void SkPicture::clone(SkPicture* pictures, int count) const { SkPictCopyInfo copyInfo; SkPictInfo info; @@ -263,7 +282,7 @@ void SkPicture::clone(SkPicture* pictures, int count) const { SkDEBUGCODE(int heapSize = SafeCount(fPlayback->fBitmapHeap.get());) for (int i = 0; i < paintCount; i++) { - if (NeedsDeepCopy(fPlayback->fPaints->at(i))) { + if (needs_deep_copy(fPlayback->fPaints->at(i))) { copyInfo.paintData[i] = SkFlatData::Create(©Info.controller, fPlayback->fPaints->at(i), 0); diff --git a/src/gpu/GrPictureUtils.cpp b/src/gpu/GrPictureUtils.cpp index d8e4161c9f..e8c3b504cf 100644 --- a/src/gpu/GrPictureUtils.cpp +++ b/src/gpu/GrPictureUtils.cpp @@ -7,8 +7,6 @@ #include "GrPictureUtils.h" #include "SkDevice.h" -#include "SkDraw.h" -#include "SkPaintPriv.h" // The GrGather device performs GPU-backend-specific preprocessing on // a picture. The results are stored in a GPUAccelData. @@ -22,17 +20,12 @@ class GrGatherDevice : public SkBaseDevice { public: SK_DECLARE_INST_COUNT(GrGatherDevice) - GrGatherDevice(int width, int height, SkPicture* picture, GPUAccelData* accelData, - int saveLayerDepth) { + GrGatherDevice(int width, int height, SkPicture* picture, GPUAccelData* accelData) { fPicture = picture; - fSaveLayerDepth = saveLayerDepth; - fInfo.fValid = true; fInfo.fSize.set(width, height); - fInfo.fPaint = NULL; fInfo.fSaveLayerOpID = fPicture->EXPERIMENTAL_curOpID(); fInfo.fRestoreOpID = 0; fInfo.fHasNestedLayers = false; - fInfo.fIsNested = (2 == fSaveLayerDepth); fEmptyBitmap.setConfig(SkImageInfo::Make(fInfo.fSize.fWidth, fInfo.fSize.fHeight, @@ -117,8 +110,7 @@ protected: const SkPaint& paint) SK_OVERRIDE { } virtual void drawDevice(const SkDraw& draw, SkBaseDevice* deviceIn, int x, int y, - const SkPaint& paint) SK_OVERRIDE { - // deviceIn is the one that is being "restored" back to its parent + const SkPaint&) SK_OVERRIDE { GrGatherDevice* device = static_cast(deviceIn); if (device->fAlreadyDrawn) { @@ -126,29 +118,6 @@ protected: } device->fInfo.fRestoreOpID = fPicture->EXPERIMENTAL_curOpID(); - device->fInfo.fCTM = *draw.fMatrix; - device->fInfo.fCTM.postTranslate(SkIntToScalar(-device->getOrigin().fX), - SkIntToScalar(-device->getOrigin().fY)); - - // We need the x & y values that will yield 'getOrigin' when transformed - // by 'draw.fMatrix'. - device->fInfo.fOffset.iset(device->getOrigin()); - - SkMatrix invMatrix; - if (draw.fMatrix->invert(&invMatrix)) { - invMatrix.mapPoints(&device->fInfo.fOffset, 1); - } else { - device->fInfo.fValid = false; - } - - if (NeedsDeepCopy(paint)) { - // This NULL acts as a signal that the paint was uncopyable (for now) - device->fInfo.fPaint = NULL; - device->fInfo.fValid = false; - } else { - device->fInfo.fPaint = SkNEW_ARGS(SkPaint, (paint)); - } - fAccelData->addSaveLayerInfo(device->fInfo); device->fAlreadyDrawn = true; } @@ -189,9 +158,6 @@ private: // The information regarding the saveLayer call this device represents. GPUAccelData::SaveLayerInfo fInfo; - // The depth of this device in the saveLayer stack - int fSaveLayerDepth; - virtual void replaceBitmapBackendForRasterSurface(const SkBitmap&) SK_OVERRIDE { NotSupported(); } @@ -201,8 +167,7 @@ private: SkASSERT(kSaveLayer_Usage == usage); fInfo.fHasNestedLayers = true; - return SkNEW_ARGS(GrGatherDevice, (info.width(), info.height(), fPicture, - fAccelData, fSaveLayerDepth+1)); + return SkNEW_ARGS(GrGatherDevice, (info.width(), info.height(), fPicture, fAccelData)); } virtual void flush() SK_OVERRIDE {} @@ -274,7 +239,7 @@ void GatherGPUInfo(SkPicture* pict, GPUAccelData* accelData) { return ; } - GrGatherDevice device(pict->width(), pict->height(), pict, accelData, 0); + GrGatherDevice device(pict->width(), pict->height(), pict, accelData); GrGatherCanvas canvas(&device, pict); canvas.gather(); diff --git a/src/gpu/GrPictureUtils.h b/src/gpu/GrPictureUtils.h index 5ca41320e3..6b4d901cc1 100644 --- a/src/gpu/GrPictureUtils.h +++ b/src/gpu/GrPictureUtils.h @@ -17,21 +17,8 @@ class GPUAccelData : public SkPicture::AccelData { public: // Information about a given saveLayer in an SkPicture struct SaveLayerInfo { - // True if the SaveLayerInfo is valid. False if either 'fOffset' is - // invalid (due to a non-invertible CTM) or 'fPaint' is NULL (due - // to a non-copyable paint). - bool fValid; // The size of the saveLayer SkISize fSize; - // The CTM in which this layer's draws must occur. It already incorporates - // the translation needed to map the layer's top-left point to the origin. - SkMatrix fCTM; - // The offset that needs to be passed to drawBitmap to correctly - // position the pre-rendered layer. - SkPoint fOffset; - // The paint to use on restore. NULL if the paint was not copyable (and - // thus that this layer should not be pulled forward). - const SkPaint* fPaint; // The ID of this saveLayer in the picture. 0 is an invalid ID. size_t fSaveLayerOpID; // The ID of the matching restore in the picture. 0 is an invalid ID. @@ -39,8 +26,6 @@ public: // True if this saveLayer has at least one other saveLayer nested within it. // False otherwise. bool fHasNestedLayers; - // True if this saveLayer is nested within another. False otherwise. - bool fIsNested; }; GPUAccelData(Key key) : INHERITED(key) { } @@ -58,14 +43,6 @@ public: return fSaveLayerInfo[index]; } - // We may, in the future, need to pass in the GPUDevice in order to - // incorporate the clip and matrix state into the key - static SkPicture::AccelData::Key ComputeAccelDataKey() { - static const SkPicture::AccelData::Key gGPUID = SkPicture::AccelData::GenerateDomain(); - - return gGPUID; - } - protected: SkTDArray fSaveLayerInfo; diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index 3119a9e7e3..714a6da65d 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -1910,8 +1910,16 @@ SkSurface* SkGpuDevice::newSurface(const SkImageInfo& info) { return SkSurface::NewRenderTarget(fContext, info, fRenderTarget->numSamples()); } +// In the future this may not be a static method if we need to incorporate the +// clip and matrix state into the key +SkPicture::AccelData::Key SkGpuDevice::ComputeAccelDataKey() { + static const SkPicture::AccelData::Key gGPUID = SkPicture::AccelData::GenerateDomain(); + + return gGPUID; +} + void SkGpuDevice::EXPERIMENTAL_optimize(SkPicture* picture) { - SkPicture::AccelData::Key key = GPUAccelData::ComputeAccelDataKey(); + SkPicture::AccelData::Key key = ComputeAccelDataKey(); GPUAccelData* data = SkNEW_ARGS(GPUAccelData, (key)); @@ -1926,7 +1934,7 @@ void SkGpuDevice::EXPERIMENTAL_purge(SkPicture* picture) { bool SkGpuDevice::EXPERIMENTAL_drawPicture(SkCanvas* canvas, SkPicture* picture) { - SkPicture::AccelData::Key key = GPUAccelData::ComputeAccelDataKey(); + SkPicture::AccelData::Key key = ComputeAccelDataKey(); const SkPicture::AccelData* data = picture->EXPERIMENTAL_getAccelData(key); if (NULL == data) { @@ -1935,6 +1943,27 @@ bool SkGpuDevice::EXPERIMENTAL_drawPicture(SkCanvas* canvas, SkPicture* picture) const GPUAccelData *gpuData = static_cast(data); +//#define SK_PRINT_PULL_FORWARD_INFO 1 + +#ifdef SK_PRINT_PULL_FORWARD_INFO + static bool gPrintedAccelData = false; + + if (!gPrintedAccelData) { + for (int i = 0; i < gpuData->numSaveLayers(); ++i) { + const GPUAccelData::SaveLayerInfo& info = gpuData->saveLayerInfo(i); + + SkDebugf("%d: Width: %d Height: %d SL: %d R: %d hasNestedLayers: %s\n", + i, + info.fSize.fWidth, + info.fSize.fHeight, + info.fSaveLayerOpID, + info.fRestoreOpID, + info.fHasNestedLayers ? "T" : "F"); + } + gPrintedAccelData = true; + } +#endif + SkAutoTArray pullForward(gpuData->numSaveLayers()); for (int i = 0; i < gpuData->numSaveLayers(); ++i) { pullForward[i] = false; @@ -1955,6 +1984,10 @@ bool SkGpuDevice::EXPERIMENTAL_drawPicture(SkCanvas* canvas, SkPicture* picture) const SkPicture::OperationList& ops = picture->EXPERIMENTAL_getActiveOps(clip); +#ifdef SK_PRINT_PULL_FORWARD_INFO + SkDebugf("rect: %d %d %d %d\n", clip.fLeft, clip.fTop, clip.fRight, clip.fBottom); +#endif + for (int i = 0; i < ops.numOps(); ++i) { for (int j = 0; j < gpuData->numSaveLayers(); ++j) { const GPUAccelData::SaveLayerInfo& info = gpuData->saveLayerInfo(j); @@ -1965,5 +1998,17 @@ bool SkGpuDevice::EXPERIMENTAL_drawPicture(SkCanvas* canvas, SkPicture* picture) } } +#ifdef SK_PRINT_PULL_FORWARD_INFO + SkDebugf("Need SaveLayers: "); + for (int i = 0; i < gpuData->numSaveLayers(); ++i) { + if (pullForward[i]) { + const GrCachedLayer* layer = fContext->getLayerCache()->findLayerOrCreate(picture, i); + + SkDebugf("%d (%d), ", i, layer->layerID()); + } + } + SkDebugf("\n"); +#endif + return false; } diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp index aa0719175c..8c5ae62857 100644 --- a/tests/PictureTest.cpp +++ b/tests/PictureTest.cpp @@ -6,18 +6,12 @@ */ #include "SkBitmapDevice.h" -#if SK_SUPPORT_GPU -#include "SkBlurImageFilter.h" -#endif #include "SkCanvas.h" #include "SkColorPriv.h" #include "SkDashPathEffect.h" #include "SkData.h" #include "SkDecodingImageGenerator.h" #include "SkError.h" -#if SK_SUPPORT_GPU -#include "SkGpuDevice.h" -#endif #include "SkImageEncoder.h" #include "SkImageGenerator.h" #include "SkPaint.h" @@ -28,12 +22,6 @@ #include "SkRandom.h" #include "SkShader.h" #include "SkStream.h" - -#if SK_SUPPORT_GPU -#include "SkSurface.h" -#include "GrContextFactory.h" -#include "GrPictureUtils.h" -#endif #include "Test.h" static const int gColorScale = 30; @@ -777,138 +765,6 @@ static void test_gpu_veto(skiatest::Reporter* reporter) { // hairline stroked AA concave paths are fine for GPU rendering REPORTER_ASSERT(reporter, picture->suitableForGpuRasterization(NULL)); } - -static void test_gpu_picture_optimization(skiatest::Reporter* reporter, - GrContextFactory* factory) { - - GrContext* context = factory->get(GrContextFactory::kNative_GLContextType); - - static const int kWidth = 100; - static const int kHeight = 100; - - SkAutoTUnref pict; - - // create a picture with the structure: - // 1) - // SaveLayer - // Restore - // 2) - // SaveLayer - // Translate - // SaveLayer w/ bound - // Restore - // Restore - // 3) - // SaveLayer w/ copyable paint - // Restore - // 4) - // SaveLayer w/ non-copyable paint - // Restore - { - SkPictureRecorder recorder; - - SkCanvas* c = recorder.beginRecording(kWidth, kHeight, NULL, 0); - // 1) - c->saveLayer(NULL, NULL); - c->restore(); - - // 2) - c->saveLayer(NULL, NULL); - c->translate(kWidth/2, kHeight/2); - SkRect r = SkRect::MakeXYWH(0, 0, kWidth/2, kHeight/2); - c->saveLayer(&r, NULL); - c->restore(); - c->restore(); - - // 3) - { - SkPaint p; - p.setColor(SK_ColorRED); - c->saveLayer(NULL, &p); - c->restore(); - } - // 4) - // TODO: this case will need to be removed once the paint's are immutable - { - SkPaint p; - SkBitmap bmp; - bmp.allocN32Pixels(10, 10); - bmp.eraseColor(SK_ColorGREEN); - bmp.setAlphaType(kOpaque_SkAlphaType); - SkShader* shader = SkShader::CreateBitmapShader(bmp, - SkShader::kClamp_TileMode, SkShader::kClamp_TileMode); - p.setShader(shader)->unref(); - - c->saveLayer(NULL, &p); - c->restore(); - } - - pict.reset(recorder.endRecording()); - } - - // Now test out the SaveLayer extraction - { - SkImageInfo info = SkImageInfo::MakeN32Premul(kWidth, kHeight); - - SkAutoTUnref surface(SkSurface::NewScratchRenderTarget(context, info)); - - SkCanvas* canvas = surface->getCanvas(); - - canvas->EXPERIMENTAL_optimize(pict); - - SkPicture::AccelData::Key key = GPUAccelData::ComputeAccelDataKey(); - - const SkPicture::AccelData* data = pict->EXPERIMENTAL_getAccelData(key); - REPORTER_ASSERT(reporter, NULL != data); - - const GPUAccelData *gpuData = static_cast(data); - REPORTER_ASSERT(reporter, 5 == gpuData->numSaveLayers()); - - const GPUAccelData::SaveLayerInfo& info0 = gpuData->saveLayerInfo(0); - // The parent/child layer appear in reverse order - const GPUAccelData::SaveLayerInfo& info1 = gpuData->saveLayerInfo(2); - const GPUAccelData::SaveLayerInfo& info2 = gpuData->saveLayerInfo(1); - const GPUAccelData::SaveLayerInfo& info3 = gpuData->saveLayerInfo(3); - const GPUAccelData::SaveLayerInfo& info4 = gpuData->saveLayerInfo(4); - - REPORTER_ASSERT(reporter, info0.fValid); - REPORTER_ASSERT(reporter, kWidth == info0.fSize.fWidth && kHeight == info0.fSize.fHeight); - REPORTER_ASSERT(reporter, info0.fCTM.isIdentity()); - REPORTER_ASSERT(reporter, 0 == info0.fOffset.fX && 0 == info0.fOffset.fY); - REPORTER_ASSERT(reporter, NULL != info0.fPaint); - REPORTER_ASSERT(reporter, !info0.fIsNested && !info0.fHasNestedLayers); - - REPORTER_ASSERT(reporter, info1.fValid); - REPORTER_ASSERT(reporter, kWidth == info1.fSize.fWidth && kHeight == info1.fSize.fHeight); - REPORTER_ASSERT(reporter, info1.fCTM.isIdentity()); - REPORTER_ASSERT(reporter, 0 == info1.fOffset.fX && 0 == info1.fOffset.fY); - REPORTER_ASSERT(reporter, NULL != info1.fPaint); - REPORTER_ASSERT(reporter, !info1.fIsNested && info1.fHasNestedLayers); // has a nested SL - - REPORTER_ASSERT(reporter, info2.fValid); - REPORTER_ASSERT(reporter, kWidth/2 == info2.fSize.fWidth && - kHeight/2 == info2.fSize.fHeight); // bound reduces size - REPORTER_ASSERT(reporter, info2.fCTM.isIdentity()); // translated - REPORTER_ASSERT(reporter, 0 == info2.fOffset.fX && 0 == info2.fOffset.fY); - REPORTER_ASSERT(reporter, NULL != info1.fPaint); - REPORTER_ASSERT(reporter, info2.fIsNested && !info2.fHasNestedLayers); // is nested - - REPORTER_ASSERT(reporter, info3.fValid); - REPORTER_ASSERT(reporter, kWidth == info3.fSize.fWidth && kHeight == info3.fSize.fHeight); - REPORTER_ASSERT(reporter, info3.fCTM.isIdentity()); - REPORTER_ASSERT(reporter, 0 == info3.fOffset.fX && 0 == info3.fOffset.fY); - REPORTER_ASSERT(reporter, NULL != info3.fPaint); - REPORTER_ASSERT(reporter, !info3.fIsNested && !info3.fHasNestedLayers); - - REPORTER_ASSERT(reporter, !info4.fValid); // paint is/was uncopyable - REPORTER_ASSERT(reporter, kWidth == info4.fSize.fWidth && kHeight == info4.fSize.fHeight); - REPORTER_ASSERT(reporter, 0 == info4.fOffset.fX && 0 == info4.fOffset.fY); - REPORTER_ASSERT(reporter, info4.fCTM.isIdentity()); - REPORTER_ASSERT(reporter, NULL == info4.fPaint); // paint is/was uncopyable - REPORTER_ASSERT(reporter, !info4.fIsNested && !info4.fHasNestedLayers); - } -} - #endif static void set_canvas_to_save_count_4(SkCanvas* canvas) { @@ -1428,12 +1284,6 @@ DEF_TEST(Picture, reporter) { test_gen_id(reporter); } -#if SK_SUPPORT_GPU -DEF_GPUTEST(GPUPicture, reporter, factory) { - test_gpu_picture_optimization(reporter, factory); -} -#endif - static void draw_bitmaps(const SkBitmap bitmap, SkCanvas* canvas) { const SkPaint paint; const SkRect rect = { 5.0f, 5.0f, 8.0f, 8.0f };