AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Remove SkPicture::kUsePathBoundsForClip_RecordingFlag

Browse Source 
The real question is whether we ever want to record a picture without using the path bounds for a conservative (but faster) clip answer?

R=reed@google.com, mtklein@google.com, djsollen@google.com, scroggo@google.com

Author: robertphillips@google.com

Review URL: https://codereview.chromium.org/316143003
This commit is contained in:
robertphillips 2014-06-09 06:25:34 -07:00 committed by Commit bot
parent 5713352a64
commit 9f1c241e0d
11 changed files with 57 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
bench/benchmain.cpp
View File

@ -490,9 +490,6 @@ int tool_main(int argc, char** argv) {
SkPictureRecorder recorderTo;
const SkIPoint dim = bench->getSize();
const SkPicture::RecordingFlags kRecordFlags =
SkPicture::kUsePathBoundsForClip_RecordingFlag;
SkAutoTUnref<SkSurface> surface;
if (SkBenchmark::kNonRendering_Backend != config.backend) {
surface.reset(make_surface(config.fColorType,
@ -512,16 +509,13 @@ int tool_main(int argc, char** argv) {
canvas.reset(SkDeferredCanvas::Create(surface.get()));
break;
case kRecord_BenchMode:
canvas.reset(SkRef(recorderTo.beginRecording(dim.fX, dim.fY,
NULL, kRecordFlags)));
canvas.reset(SkRef(recorderTo.beginRecording(dim.fX, dim.fY)));
break;
case kPictureRecord_BenchMode: {
SkPictureRecorder recorderFrom;
bench->draw(1, recorderFrom.beginRecording(dim.fX, dim.fY,
NULL, kRecordFlags));
bench->draw(1, recorderFrom.beginRecording(dim.fX, dim.fY));
recordFrom.reset(recorderFrom.endRecording());
canvas.reset(SkRef(recorderTo.beginRecording(dim.fX, dim.fY,
NULL, kRecordFlags)));
canvas.reset(SkRef(recorderTo.beginRecording(dim.fX, dim.fY)));
break;
}
case kNormal_BenchMode:
@ -587,8 +581,7 @@ int tool_main(int argc, char** argv) {
if ((benchMode == kRecord_BenchMode || benchMode == kPictureRecord_BenchMode)) {
// Clear the recorded commands so that they do not accumulate.
canvas.reset(SkRef(recorderTo.beginRecording(dim.fX, dim.fY,
NULL, kRecordFlags)));
canvas.reset(SkRef(recorderTo.beginRecording(dim.fX, dim.fY)));
}
timer.start();

4
expectations/gm/ignored-tests.txt
View File

@ -52,3 +52,7 @@ tilemodes
shadertext3
drawbitmaprect
coloremoji
# Added by robertphillips for https://codereview.chromium.org/316143003/
# This CL actually fixes this GM's image
distantclip

3
gm/gmmain.cpp
View File

@ -1669,8 +1669,7 @@ ErrorCombination run_multiple_modes(GMMain &gmmain, GM *gm, const ConfigData &co
// result that can be validated against comparisonBitmap.
SkScalar recordScale = SkScalarInvert(replayScale);
SkPicture* pict = gmmain.generate_new_picture(
gm, kTileGrid_BbhType, SkPicture::kUsePathBoundsForClip_RecordingFlag,
recordScale);
gm, kTileGrid_BbhType, 0, recordScale);
SkAutoUnref aur(pict);
SkBitmap bitmap;
// We cannot yet pass 'true' to generate_image_from_picture to

2
include/core/SkPicture.h
View File

@ -132,6 +132,7 @@ public:
*/
void clone(SkPicture* pictures, int count) const;
#ifdef SK_SUPPORT_LEGACY_RECORDING_FLAG
// TODO: kUsePathBoundsForClip_RecordingFlag no longer belongs in
// SkPicture. It should be moved to SkPictureRecorder (or just made
// the default behavior).
@ -146,6 +147,7 @@ public:
*/
kUsePathBoundsForClip_RecordingFlag = 0x01
};
#endif
/** Replays the drawing commands on the specified canvas.
@param canvas the canvas receiving the drawing commands.

5
include/core/SkPictureRecorder.h
View File

@ -29,10 +29,9 @@ public:
@param recordFlags optional flags that control recording.
@return the canvas.
*/
// TODO: allow default parameters once the other beginRecoding entry point is gone
SkCanvas* beginRecording(int width, int height,
SkBBHFactory* bbhFactory /* = NULL */,
uint32_t recordFlags /* = 0 */);
SkBBHFactory* bbhFactory = NULL,
uint32_t recordFlags = 0);
/** Returns the recording canvas if one is active, or NULL if recording is
not active. This does not alter the refcnt on the canvas (if present).

14
src/core/SkPictureRecord.cpp
View File

@ -863,11 +863,7 @@ void SkPictureRecord::onClipRRect(const SkRRect& rrect, SkRegion::Op op, ClipEdg
#else
this->recordClipRRect(rrect, op, kSoft_ClipEdgeStyle == edgeStyle);
#endif
if (fRecordFlags & SkPicture::kUsePathBoundsForClip_RecordingFlag) {
this->updateClipConservativelyUsingBounds(rrect.getBounds(), op, false);
} else {
this->INHERITED::onClipRRect(rrect, op, edgeStyle);
}
this->updateClipConservativelyUsingBounds(rrect.getBounds(), op, false);
}
size_t SkPictureRecord::recordClipRRect(const SkRRect& rrect, SkRegion::Op op, bool doAA) {
@ -899,12 +895,8 @@ void SkPictureRecord::onClipPath(const SkPath& path, SkRegion::Op op, ClipEdgeSt
this->recordClipPath(pathID, op, kSoft_ClipEdgeStyle == edgeStyle);
#endif
if (fRecordFlags & SkPicture::kUsePathBoundsForClip_RecordingFlag) {
this->updateClipConservativelyUsingBounds(path.getBounds(), op,
path.isInverseFillType());
} else {
this->INHERITED::onClipPath(path, op, edgeStyle);
}
this->updateClipConservativelyUsingBounds(path.getBounds(), op,
path.isInverseFillType());
}
size_t SkPictureRecord::recordClipPath(int pathID, SkRegion::Op op, bool doAA) {

82
tests/PictureTest.cpp
View File

@ -315,7 +315,7 @@ static SkPicture* record_bitmaps(const SkBitmap bm[],
int count,
DrawBitmapProc proc) {
SkPictureRecorder recorder;
SkCanvas* canvas = recorder.beginRecording(1000, 1000, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(1000, 1000);
for (int i = 0; i < count; ++i) {
analytic[i].rewind();
canvas->save();
@ -671,17 +671,17 @@ static void test_gatherpixelrefsandrects(skiatest::Reporter* reporter) {
static void test_deleting_empty_playback() {
SkPictureRecorder recorder;
// Creates an SkPictureRecord
recorder.beginRecording(0, 0, NULL, 0);
recorder.beginRecording(0, 0);
// Turns that into an SkPicturePlayback
SkAutoTUnref<SkPicture> picture(recorder.endRecording());
// Deletes the old SkPicturePlayback, and creates a new SkPictureRecord
recorder.beginRecording(0, 0, NULL, 0);
recorder.beginRecording(0, 0);
}
// Ensure that serializing an empty picture does not assert. Likewise only runs in debug mode.
static void test_serializing_empty_picture() {
SkPictureRecorder recorder;
recorder.beginRecording(0, 0, NULL, 0);
recorder.beginRecording(0, 0);
SkAutoTUnref<SkPicture> picture(recorder.endRecording());
SkDynamicMemoryWStream stream;
picture->serialize(&stream);
@ -713,7 +713,7 @@ static void test_gpu_veto(skiatest::Reporter* reporter) {
SkPictureRecorder recorder;
SkCanvas* canvas = recorder.beginRecording(100, 100, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(100, 100);
{
SkPath path;
path.moveTo(0, 0);
@ -735,7 +735,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, NULL, 0);
canvas = recorder.beginRecording(100, 100);
{
SkPath path;
@ -757,7 +757,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, NULL, 0);
canvas = recorder.beginRecording(100, 100);
{
SkPath path;
@ -811,7 +811,7 @@ static void test_gpu_picture_optimization(skiatest::Reporter* reporter,
{
SkPictureRecorder recorder;
SkCanvas* c = recorder.beginRecording(kWidth, kHeight, NULL, 0);
SkCanvas* c = recorder.beginRecording(kWidth, kHeight);
// 1)
c->saveLayer(NULL, NULL);
c->restore();
@ -978,7 +978,7 @@ public:
static SkPicture* Copy(SkPictureRecorder* recorder) {
SkPictureRecorder recorder2;
SkCanvas* canvas = recorder2.beginRecording(10, 10, NULL, 0);
SkCanvas* canvas = recorder2.beginRecording(10, 10);
recorder->partialReplay(canvas);
@ -992,7 +992,7 @@ DEF_TEST(PictureRecorder_replay, reporter) {
{
SkPictureRecorder recorder;
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->saveLayer(NULL, NULL);
@ -1015,7 +1015,7 @@ DEF_TEST(PictureRecorder_replay, reporter) {
{
SkPictureRecorder recorder;
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(10, 10);
SkRect r = SkRect::MakeWH(5, 5);
SkPaint p;
@ -1055,7 +1055,7 @@ static void test_unbalanced_save_restores(skiatest::Reporter* reporter) {
{
// Create picture with 2 unbalanced saves
SkCanvas* canvas = recorder.beginRecording(100, 100, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(100, 100);
canvas->save();
canvas->translate(10, 10);
canvas->drawRect(rect, paint);
@ -1072,7 +1072,7 @@ static void test_unbalanced_save_restores(skiatest::Reporter* reporter) {
{
// Create picture with 2 unbalanced restores
SkCanvas* canvas = recorder.beginRecording(100, 100, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(100, 100);
canvas->save();
canvas->translate(10, 10);
canvas->drawRect(rect, paint);
@ -1092,7 +1092,7 @@ static void test_unbalanced_save_restores(skiatest::Reporter* reporter) {
set_canvas_to_save_count_4(&testCanvas);
{
SkCanvas* canvas = recorder.beginRecording(100, 100, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(100, 100);
canvas->translate(10, 10);
canvas->drawRect(rect, paint);
SkAutoTUnref<SkPicture> noSavePicture(recorder.endRecording());
@ -1111,7 +1111,7 @@ static void test_peephole() {
for (int j = 0; j < 100; j++) {
SkRandom rand2(rand); // remember the seed
SkCanvas* canvas = recorder.beginRecording(100, 100, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(100, 100);
for (int i = 0; i < 1000; ++i) {
rand_op(canvas, rand);
@ -1122,7 +1122,7 @@ static void test_peephole() {
}
{
SkCanvas* canvas = recorder.beginRecording(100, 100, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(100, 100);
SkRect rect = SkRect::MakeWH(50, 50);
for (int i = 0; i < 100; ++i) {
@ -1145,7 +1145,7 @@ static void test_bad_bitmap() {
SkBitmap bm;
bm.setInfo(SkImageInfo::MakeN32Premul(100, 100));
SkPictureRecorder recorder;
SkCanvas* recordingCanvas = recorder.beginRecording(100, 100, NULL, 0);
SkCanvas* recordingCanvas = recorder.beginRecording(100, 100);
recordingCanvas->drawBitmap(bm, 0, 0);
SkAutoTUnref<SkPicture> picture(recorder.endRecording());
@ -1160,7 +1160,7 @@ static SkData* encode_bitmap_to_data(size_t*, const SkBitmap& bm) {
static SkData* serialized_picture_from_bitmap(const SkBitmap& bitmap) {
SkPictureRecorder recorder;
SkCanvas* canvas = recorder.beginRecording(bitmap.width(), bitmap.height(), NULL, 0);
SkCanvas* canvas = recorder.beginRecording(bitmap.width(), bitmap.height());
canvas->drawBitmap(bitmap, 0, 0);
SkAutoTUnref<SkPicture> picture(recorder.endRecording());
@ -1226,7 +1226,7 @@ static void test_clone_empty(skiatest::Reporter* reporter) {
// had a picture with no paints. This test passes by not crashing.
{
SkPictureRecorder recorder;
recorder.beginRecording(1, 1, NULL, 0);
recorder.beginRecording(1, 1);
SkAutoTUnref<SkPicture> picture(recorder.endRecording());
SkAutoTUnref<SkPicture> destPicture(picture->clone());
REPORTER_ASSERT(reporter, NULL != destPicture);
@ -1242,7 +1242,7 @@ static void test_draw_empty(skiatest::Reporter* reporter) {
{
// stock SkPicture
SkPictureRecorder recorder;
recorder.beginRecording(1, 1, NULL, 0);
recorder.beginRecording(1, 1);
SkAutoTUnref<SkPicture> picture(recorder.endRecording());
canvas.drawPicture(picture);
@ -1257,7 +1257,7 @@ static void test_draw_empty(skiatest::Reporter* reporter) {
SkTileGridFactory factory(gridInfo);
SkPictureRecorder recorder;
recorder.beginRecording(1, 1, &factory, 0);
recorder.beginRecording(1, 1, &factory);
SkAutoTUnref<SkPicture> picture(recorder.endRecording());
canvas.drawPicture(picture);
@ -1267,7 +1267,7 @@ static void test_draw_empty(skiatest::Reporter* reporter) {
// RTree
SkRTreeFactory factory;
SkPictureRecorder recorder;
recorder.beginRecording(1, 1, &factory, 0);
recorder.beginRecording(1, 1, &factory);
SkAutoTUnref<SkPicture> picture(recorder.endRecording());
canvas.drawPicture(picture);
@ -1277,7 +1277,7 @@ static void test_draw_empty(skiatest::Reporter* reporter) {
// quad tree
SkQuadTreeFactory factory;
SkPictureRecorder recorder;
recorder.beginRecording(1, 1, &factory, 0);
recorder.beginRecording(1, 1, &factory);
SkAutoTUnref<SkPicture> picture(recorder.endRecording());
canvas.drawPicture(picture);
@ -1305,8 +1305,7 @@ static void test_clip_bound_opt(skiatest::Reporter* reporter) {
// Minimalist test set for 100% code coverage of
// SkPictureRecord::updateClipConservativelyUsingBounds
{
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL,
SkPicture::kUsePathBoundsForClip_RecordingFlag);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->clipPath(invPath, SkRegion::kIntersect_Op);
bool nonEmpty = canvas->getClipDeviceBounds(&clipBounds);
REPORTER_ASSERT(reporter, true == nonEmpty);
@ -1316,8 +1315,7 @@ static void test_clip_bound_opt(skiatest::Reporter* reporter) {
REPORTER_ASSERT(reporter, 10 == clipBounds.fRight);
}
{
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL,
SkPicture::kUsePathBoundsForClip_RecordingFlag);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->clipPath(path, SkRegion::kIntersect_Op);
canvas->clipPath(invPath, SkRegion::kIntersect_Op);
bool nonEmpty = canvas->getClipDeviceBounds(&clipBounds);
@ -1328,8 +1326,7 @@ static void test_clip_bound_opt(skiatest::Reporter* reporter) {
REPORTER_ASSERT(reporter, 8 == clipBounds.fRight);
}
{
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL,
SkPicture::kUsePathBoundsForClip_RecordingFlag);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->clipPath(path, SkRegion::kIntersect_Op);
canvas->clipPath(invPath, SkRegion::kUnion_Op);
bool nonEmpty = canvas->getClipDeviceBounds(&clipBounds);
@ -1340,8 +1337,7 @@ static void test_clip_bound_opt(skiatest::Reporter* reporter) {
REPORTER_ASSERT(reporter, 10 == clipBounds.fRight);
}
{
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL,
SkPicture::kUsePathBoundsForClip_RecordingFlag);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->clipPath(path, SkRegion::kDifference_Op);
bool nonEmpty = canvas->getClipDeviceBounds(&clipBounds);
REPORTER_ASSERT(reporter, true == nonEmpty);
@ -1351,8 +1347,7 @@ static void test_clip_bound_opt(skiatest::Reporter* reporter) {
REPORTER_ASSERT(reporter, 10 == clipBounds.fRight);
}
{
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL,
SkPicture::kUsePathBoundsForClip_RecordingFlag);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->clipPath(path, SkRegion::kReverseDifference_Op);
bool nonEmpty = canvas->getClipDeviceBounds(&clipBounds);
// True clip is actually empty in this case, but the best
@ -1365,8 +1360,7 @@ static void test_clip_bound_opt(skiatest::Reporter* reporter) {
REPORTER_ASSERT(reporter, 8 == clipBounds.fRight);
}
{
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL,
SkPicture::kUsePathBoundsForClip_RecordingFlag);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->clipPath(path, SkRegion::kIntersect_Op);
canvas->clipPath(path2, SkRegion::kXOR_Op);
bool nonEmpty = canvas->getClipDeviceBounds(&clipBounds);
@ -1424,7 +1418,7 @@ private:
static void test_clip_expansion(skiatest::Reporter* reporter) {
SkPictureRecorder recorder;
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->clipRect(SkRect::MakeEmpty(), SkRegion::kReplace_Op);
// The following expanding clip should not be skipped.
@ -1448,35 +1442,35 @@ static void test_hierarchical(skiatest::Reporter* reporter) {
SkPictureRecorder recorder;
recorder.beginRecording(10, 10, NULL, 0);
recorder.beginRecording(10, 10);
SkAutoTUnref<SkPicture> childPlain(recorder.endRecording());
REPORTER_ASSERT(reporter, !childPlain->willPlayBackBitmaps()); // 0
recorder.beginRecording(10, 10, NULL, 0)->drawBitmap(bm, 0, 0);
recorder.beginRecording(10, 10)->drawBitmap(bm, 0, 0);
SkAutoTUnref<SkPicture> childWithBitmap(recorder.endRecording());
REPORTER_ASSERT(reporter, childWithBitmap->willPlayBackBitmaps()); // 1
{
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->drawPicture(childPlain);
SkAutoTUnref<SkPicture> parentPP(recorder.endRecording());
REPORTER_ASSERT(reporter, !parentPP->willPlayBackBitmaps()); // 0
}
{
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->drawPicture(childWithBitmap);
SkAutoTUnref<SkPicture> parentPWB(recorder.endRecording());
REPORTER_ASSERT(reporter, parentPWB->willPlayBackBitmaps()); // 1
}
{
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->drawBitmap(bm, 0, 0);
canvas->drawPicture(childPlain);
SkAutoTUnref<SkPicture> parentWBP(recorder.endRecording());
REPORTER_ASSERT(reporter, parentWBP->willPlayBackBitmaps()); // 1
}
{
SkCanvas* canvas = recorder.beginRecording(10, 10, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(10, 10);
canvas->drawBitmap(bm, 0, 0);
canvas->drawPicture(childWithBitmap);
SkAutoTUnref<SkPicture> parentWBWB(recorder.endRecording());
@ -1493,7 +1487,7 @@ static void test_gen_id(skiatest::Reporter* reporter) {
SkPictureRecorder recorder;
SkCanvas* canvas = recorder.beginRecording(1, 1, NULL, 0);
SkCanvas* canvas = recorder.beginRecording(1, 1);
canvas->drawARGB(255, 255, 255, 255);
SkAutoTUnref<SkPicture> hasData(recorder.endRecording());
// picture should have a non-zero id after recording
@ -1580,7 +1574,7 @@ static void test_draw_bitmaps(SkCanvas* canvas) {
DEF_TEST(Picture_EmptyBitmap, r) {
SkPictureRecorder recorder;
test_draw_bitmaps(recorder.beginRecording(10, 10, NULL, 0));
test_draw_bitmaps(recorder.beginRecording(10, 10));
SkAutoTUnref<SkPicture> picture(recorder.endRecording());
}

6
tools/PictureRenderer.cpp
View File

@ -256,12 +256,6 @@ void PictureRenderer::purgeTextures() {
#endif
}
uint32_t PictureRenderer::recordFlags() {
return (kNone_BBoxHierarchyType == fBBoxHierarchyType)
? 0
: SkPicture::kUsePathBoundsForClip_RecordingFlag;
}
/**
* Write the canvas to an image file and/or JSON summary.
*

2
tools/PictureRenderer.h
View File

@ -413,7 +413,7 @@ protected:
void scaleToScaleFactor(SkCanvas*);
SkBBHFactory* getFactory();
uint32_t recordFlags();
uint32_t recordFlags() const { return 0; }
SkCanvas* setupCanvas();
virtual SkCanvas* setupCanvas(int width, int height);

3
tools/bench_playback.cpp
View File

@ -39,8 +39,7 @@ static SkPicture* rerecord_with_tilegrid(SkPicture& src) {
SkTileGridFactory factory(info);
SkPictureRecorder recorder;
src.draw(recorder.beginRecording(src.width(), src.height(), &factory,
SkPicture::kUsePathBoundsForClip_RecordingFlag));
src.draw(recorder.beginRecording(src.width(), src.height(), &factory));
return recorder.endRecording();
}

2
tools/bench_record.cpp
View File

@ -25,7 +25,7 @@ __SK_FORCE_IMAGE_DECODER_LINKING;
DEFINE_string2(skps, r, "skps", "Directory containing SKPs to read and re-record.");
DEFINE_int32(loops, 900, "Number of times to re-record each SKP.");
DEFINE_int32(flags, SkPicture::kUsePathBoundsForClip_RecordingFlag, "RecordingFlags to use.");
DEFINE_int32(flags, 0, "RecordingFlags to use.");
DEFINE_bool(endRecording, true, "If false, don't time SkPicture::endRecording()");
DEFINE_int32(nullSize, 1000, "Pretend dimension of null source picture.");
DEFINE_int32(tileGridSize, 512, "Set the tile grid size. Has no effect if bbh is not set to tilegrid.");