Revert "hide picture virtuals (no public callers)"
This reverts commit 8005bff7e631a269f0dfaae93ff9963dc0e5ff39.
Reason for revert: hwui, flutter, and headless blink in G3 all still using these.
Original change's description:
> hide picture virtuals (no public callers)
>
> This prepares the way for a clean impl of a "placeholder" picture that never unrolls
>
> Bug: skia:
> Change-Id: I3b5785c5c94432b54e9a7dc280b2a6e716592473
> Reviewed-on: https://skia-review.googlesource.com/100260
> Commit-Queue: Mike Reed <reed@google.com>
> Reviewed-by: Mike Klein <mtklein@chromium.org>
TBR=mtklein@chromium.org,mtklein@google.com,reed@google.com
Change-Id: I385789dd420588ea9a9390c8a44c6ecb96c7f358
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/100880
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index 1f275d8..b9ecfa7 100644
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -38,7 +38,6 @@
#include "SkLeanWindows.h"
#include "SkOSFile.h"
#include "SkOSPath.h"
-#include "SkPicturePriv.h"
#include "SkPictureRecorder.h"
#include "SkSVGDOM.h"
#include "SkScan.h"
@@ -739,7 +738,8 @@
SkString name = SkOSPath::Basename(path.c_str());
fSourceType = "skp";
fBenchType = "recording";
- fSKPBytes = static_cast<double>(SkPicturePriv::ApproxBytesUsed(pic.get()));
+ fSKPBytes = static_cast<double>(pic->approximateBytesUsed());
+ fSKPOps = pic->approximateOpCount();
return new RecordingBench(name.c_str(), pic.get(), FLAGS_bbh, FLAGS_lite);
}
@@ -753,7 +753,8 @@
SkString name = SkOSPath::Basename(path.c_str());
fSourceType = "skp";
fBenchType = "piping";
- fSKPBytes = static_cast<double>(SkPicturePriv::ApproxBytesUsed(pic.get()));
+ fSKPBytes = static_cast<double>(pic->approximateBytesUsed());
+ fSKPOps = pic->approximateOpCount();
return new PipingBench(name.c_str(), pic.get());
}
@@ -768,6 +769,7 @@
fSourceType = "skp";
fBenchType = "deserial";
fSKPBytes = static_cast<double>(data->size());
+ fSKPOps = 0;
return new DeserializePictureBench(name.c_str(), std::move(data));
}
@@ -1061,6 +1063,7 @@
}
if (0 == strcmp(fBenchType, "recording")) {
log->metric("bytes", fSKPBytes);
+ log->metric("ops", fSKPOps);
}
}
@@ -1090,7 +1093,7 @@
SkScalar fZoomMax;
double fZoomPeriodMs;
- double fSKPBytes;
+ double fSKPBytes, fSKPOps;
const char* fSourceType; // What we're benching: bench, GM, SKP, ...
const char* fBenchType; // How we bench it: micro, recording, playback, ...
diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index d505a0a..4887db9 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -90,7 +90,18 @@
sk_sp<SkData> serialize(const SkSerialProcs* = nullptr) const;
void serialize(SkWStream*, const SkSerialProcs* = nullptr) const;
-protected:
+ /**
+ * Serialize to a buffer.
+ */
+ void flatten(SkWriteBuffer&) const;
+
+ /** Return the approximate number of operations in this picture. This
+ * number may be greater or less than the number of SkCanvas calls
+ * recorded: some calls may be recorded as more than one operation, or some
+ * calls may be optimized away.
+ */
+ virtual int approximateOpCount() const = 0;
+
/** Returns the approximate byte size of this picture, not including large ref'd objects. */
virtual size_t approximateBytesUsed() const = 0;
@@ -102,8 +113,6 @@
SkPicture();
friend class SkBigPicture;
friend class SkEmptyPicture;
- friend class SkPicturePriv;
- friend class SkPictureRecorder;
template <typename> friend class SkMiniPicture;
void serialize(SkWStream*, const SkSerialProcs*, SkRefCntSet* typefaces) const;
@@ -163,7 +172,6 @@
SkPictInfo createHeader() const;
SkPictureData* backport() const;
- void flatten(SkWriteBuffer&) const;
mutable uint32_t fUniqueID;
};
diff --git a/src/core/SkBigPicture.cpp b/src/core/SkBigPicture.cpp
index e91f897..2ae9248 100644
--- a/src/core/SkBigPicture.cpp
+++ b/src/core/SkBigPicture.cpp
@@ -54,6 +54,7 @@
}
SkRect SkBigPicture::cullRect() const { return fCullRect; }
+int SkBigPicture::approximateOpCount() const { return fRecord->count(); }
size_t SkBigPicture::approximateBytesUsed() const {
size_t bytes = sizeof(*this) + fRecord->bytesUsed() + fApproxBytesUsedBySubPictures;
if (fBBH) { bytes += fBBH->bytesUsed(); }
diff --git a/src/core/SkBigPicture.h b/src/core/SkBigPicture.h
index 3fc7071..cbe492c 100644
--- a/src/core/SkBigPicture.h
+++ b/src/core/SkBigPicture.h
@@ -43,6 +43,7 @@
// SkPicture overrides
void playback(SkCanvas*, AbortCallback*) const override;
SkRect cullRect() const override;
+ int approximateOpCount() const override;
size_t approximateBytesUsed() const override;
const SkBigPicture* asSkBigPicture() const override { return this; }
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 4f43a93..49da8eb 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -28,7 +28,7 @@
#include "SkNx.h"
#include "SkPaintPriv.h"
#include "SkPatchUtils.h"
-#include "SkPicturePriv.h"
+#include "SkPicture.h"
#include "SkRasterClip.h"
#include "SkRasterHandleAllocator.h"
#include "SkRRect.h"
@@ -2786,6 +2786,15 @@
///////////////////////////////////////////////////////////////////////////////
+/**
+ * This constant is trying to balance the speed of ref'ing a subpicture into a parent picture,
+ * against the playback cost of recursing into the subpicture to get at its actual ops.
+ *
+ * For now we pick a conservatively small value, though measurement (and other heuristics like
+ * the type of ops contained) may justify changing this value.
+ */
+#define kMaxPictureOpsToUnrollInsteadOfRef 1
+
void SkCanvas::drawPicture(const SkPicture* picture, const SkMatrix* matrix, const SkPaint* paint) {
TRACE_EVENT0("skia", TRACE_FUNC);
RETURN_ON_NULL(picture);
@@ -2793,7 +2802,12 @@
if (matrix && matrix->isIdentity()) {
matrix = nullptr;
}
- this->onDrawPicture(picture, matrix, paint);
+ if (picture->approximateOpCount() <= kMaxPictureOpsToUnrollInsteadOfRef) {
+ SkAutoCanvasMatrixPaint acmp(this, matrix, paint, picture->cullRect());
+ picture->playback(this);
+ } else {
+ this->onDrawPicture(picture, matrix, paint);
+ }
}
void SkCanvas::onDrawPicture(const SkPicture* picture, const SkMatrix* matrix,
diff --git a/src/core/SkMiniRecorder.cpp b/src/core/SkMiniRecorder.cpp
index 22b45fd..6a2edf1 100644
--- a/src/core/SkMiniRecorder.cpp
+++ b/src/core/SkMiniRecorder.cpp
@@ -22,6 +22,7 @@
void playback(SkCanvas*, AbortCallback*) const override { }
size_t approximateBytesUsed() const override { return sizeof(*this); }
+ int approximateOpCount() const override { return 0; }
SkRect cullRect() const override { return SkRect::MakeEmpty(); }
};
@@ -54,6 +55,7 @@
}
size_t approximateBytesUsed() const override { return sizeof(*this); }
+ int approximateOpCount() const override { return 1; }
SkRect cullRect() const override { return fCull; }
private:
diff --git a/src/core/SkPicturePriv.h b/src/core/SkPicturePriv.h
deleted file mode 100644
index 5da3fea..0000000
--- a/src/core/SkPicturePriv.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * Copyright 2018 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#ifndef SkPicturePriv_DEFINED
-#define SkPicturePriv_DEFINED
-
-#include "SkPicture.h"
-
-class SkWriteBuffer;
-
-class SkPicturePriv {
-public:
- static size_t ApproxBytesUsed(const SkPicture* pic) {
- return pic->approximateBytesUsed();
- }
- static const SkBigPicture* AsBigPicture(const SkPicture* pic) {
- return pic->asSkBigPicture();
- }
- static void Flatten(SkWriteBuffer& buffer, const SkPicture* pic) {
- pic->flatten(buffer);
- }
-};
-
-#endif
diff --git a/src/core/SkRecordedDrawable.cpp b/src/core/SkRecordedDrawable.cpp
index bead309..be8a2cb 100644
--- a/src/core/SkRecordedDrawable.cpp
+++ b/src/core/SkRecordedDrawable.cpp
@@ -8,7 +8,6 @@
#include "SkMatrix.h"
#include "SkPictureData.h"
#include "SkPicturePlayback.h"
-#include "SkPicturePriv.h"
#include "SkPictureRecord.h"
#include "SkPictureRecorder.h"
#include "SkRecordedDrawable.h"
@@ -34,7 +33,7 @@
size_t subPictureBytes = 0;
for (int i = 0; pictList && i < pictList->count(); i++) {
- subPictureBytes += SkPicturePriv::ApproxBytesUsed(pictList->begin()[i]);
+ subPictureBytes += pictList->begin()[i]->approximateBytesUsed();
}
// SkBigPicture will take ownership of a ref on both fRecord and fBBH.
// We're not willing to give up our ownership, so we must ref them for SkPicture.
diff --git a/src/core/SkRecorder.cpp b/src/core/SkRecorder.cpp
index 302b938..9374b60 100644
--- a/src/core/SkRecorder.cpp
+++ b/src/core/SkRecorder.cpp
@@ -9,7 +9,7 @@
#include "SkCanvasPriv.h"
#include "SkImage.h"
#include "SkPatchUtils.h"
-#include "SkPicturePriv.h"
+#include "SkPicture.h"
#include "SkRecorder.h"
#include "SkSurface.h"
@@ -302,7 +302,7 @@
void SkRecorder::onDrawPicture(const SkPicture* pic, const SkMatrix* matrix, const SkPaint* paint) {
if (fDrawPictureMode == Record_DrawPictureMode) {
- fApproxBytesUsedBySubPictures += SkPicturePriv::ApproxBytesUsed(pic);
+ fApproxBytesUsedBySubPictures += pic->approximateBytesUsed();
APPEND(DrawPicture, this->copy(paint), sk_ref_sp(pic), matrix ? *matrix : SkMatrix::I());
} else {
SkASSERT(fDrawPictureMode == Playback_DrawPictureMode);
diff --git a/src/effects/SkPictureImageFilter.cpp b/src/effects/SkPictureImageFilter.cpp
index d0ea6f3..304b91c 100644
--- a/src/effects/SkPictureImageFilter.cpp
+++ b/src/effects/SkPictureImageFilter.cpp
@@ -11,7 +11,6 @@
#include "SkColorSpaceXformCanvas.h"
#include "SkColorSpaceXformer.h"
#include "SkImageSource.h"
-#include "SkPicturePriv.h"
#include "SkReadBuffer.h"
#include "SkSpecialImage.h"
#include "SkSpecialSurface.h"
@@ -77,7 +76,7 @@
bool hasPicture = (fPicture != nullptr);
buffer.writeBool(hasPicture);
if (hasPicture) {
- SkPicturePriv::Flatten(buffer, fPicture.get());
+ fPicture->flatten(buffer);
}
buffer.writeRect(fCropRect);
}
diff --git a/src/shaders/SkPictureShader.cpp b/src/shaders/SkPictureShader.cpp
index fb15f66..21bd5a5 100644
--- a/src/shaders/SkPictureShader.cpp
+++ b/src/shaders/SkPictureShader.cpp
@@ -15,7 +15,7 @@
#include "SkImage.h"
#include "SkImageShader.h"
#include "SkMatrixUtils.h"
-#include "SkPicturePriv.h"
+#include "SkPicture.h"
#include "SkPictureImageGenerator.h"
#include "SkReadBuffer.h"
#include "SkResourceCache.h"
@@ -176,7 +176,7 @@
buffer.writeRect(fTile);
buffer.writeBool(true);
- SkPicturePriv::Flatten(buffer, fPicture.get());
+ fPicture->flatten(buffer);
}
sk_sp<SkShader> SkPictureShader::refBitmapShader(const SkMatrix& viewMatrix, const SkMatrix* localM,
diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp
index 3efb72e..6cc0fee 100644
--- a/tests/ImageTest.cpp
+++ b/tests/ImageTest.cpp
@@ -233,6 +233,7 @@
canvas->drawImage(image, 0, 0);
sk_sp<SkPicture> picture(recorder.finishRecordingAsPicture());
REPORTER_ASSERT(reporter, picture);
+ REPORTER_ASSERT(reporter, picture->approximateOpCount() > 0);
bool was_called = false;
SkSerialProcs procs;
@@ -249,6 +250,7 @@
auto deserialized = SkPicture::MakeFromData(data->data(), data->size());
REPORTER_ASSERT(reporter, deserialized);
+ REPORTER_ASSERT(reporter, deserialized->approximateOpCount() > 0);
}
// Test that a draw that only partially covers the drawing surface isn't
diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp
index 4eeec35..714338f 100644
--- a/tests/PictureTest.cpp
+++ b/tests/PictureTest.cpp
@@ -19,7 +19,7 @@
#include "SkMD5.h"
#include "SkMiniRecorder.h"
#include "SkPaint.h"
-#include "SkPicturePriv.h"
+#include "SkPicture.h"
#include "SkPictureRecorder.h"
#include "SkPixelRef.h"
#include "SkRectPriv.h"
@@ -446,7 +446,7 @@
canvas->drawRect(bounds, paint);
canvas->drawRect(bounds, paint);
sk_sp<SkPicture> p(recorder.finishRecordingAsPictureWithCull(bounds));
- const SkBigPicture* picture = SkPicturePriv::AsBigPicture(p.get());
+ const SkBigPicture* picture = p->asSkBigPicture();
REPORTER_ASSERT(reporter, picture);
SkRect finalCullRect = picture->cullRect();
@@ -813,3 +813,24 @@
REPORTER_ASSERT(r, pic->cullRect() == SkRectPriv::MakeLargest());
}
+DEF_TEST(Picture_RecordsFlush, r) {
+ SkPictureRecorder recorder;
+
+ auto canvas = recorder.beginRecording(SkRect::MakeWH(100,100));
+ for (int i = 0; i < 10; i++) {
+ canvas->clear(0);
+ for (int j = 0; j < 10; j++) {
+ canvas->drawRect(SkRect::MakeXYWH(i*10,j*10,10,10), SkPaint());
+ }
+ canvas->flush();
+ }
+
+ // Did we record the flushes?
+ auto pic = recorder.finishRecordingAsPicture();
+ REPORTER_ASSERT(r, pic->approximateOpCount() == 120); // 10 clears, 100 draws, 10 flushes
+
+ // Do we serialize and deserialize flushes?
+ auto skp = pic->serialize();
+ auto back = SkPicture::MakeFromData(skp->data(), skp->size());
+ REPORTER_ASSERT(r, back->approximateOpCount() == pic->approximateOpCount());
+}
diff --git a/tests/SerializationTest.cpp b/tests/SerializationTest.cpp
index b9c8a37..2543870 100644
--- a/tests/SerializationTest.cpp
+++ b/tests/SerializationTest.cpp
@@ -17,7 +17,6 @@
#include "SkMatrixPriv.h"
#include "SkOSFile.h"
#include "SkReadBuffer.h"
-#include "SkPicturePriv.h"
#include "SkPictureRecorder.h"
#include "SkShaderBase.h"
#include "SkTableColorFilter.h"
@@ -543,7 +542,7 @@
// Serialize picture
SkBinaryWriteBuffer writer;
- SkPicturePriv::Flatten(writer, pict.get());
+ pict->flatten(writer);
size_t size = writer.bytesWritten();
SkAutoTMalloc<unsigned char> data(size);
writer.writeToMemory(static_cast<void*>(data.get()));
diff --git a/tools/DumpRecord.cpp b/tools/DumpRecord.cpp
index dfe1968..e9374fd 100644
--- a/tools/DumpRecord.cpp
+++ b/tools/DumpRecord.cpp
@@ -7,7 +7,6 @@
#include <stdio.h>
-#include "SkPicturePriv.h"
#include "SkRecord.h"
#include "SkRecordDraw.h"
@@ -64,7 +63,7 @@
void print(const SkRecords::DrawPicture& command, double ns) {
this->printNameAndTime(command, ns);
- if (auto bp = SkPicturePriv::AsBigPicture(command.picture.get())) {
+ if (auto bp = command.picture->asSkBigPicture()) {
++fIndent;
const SkRecord& record = *bp->record();