Remove the SkCanvas comment API

No longer used in Chromium/Blink.

R=reed@google.com,robertphillips@google.com,mtklein@google.com

Review URL: https://codereview.chromium.org/1153593003

debugger/QT/SkDebuggerGUI.cpp

@@ -779,10 +779,6 @@ void SkDebuggerGUI::setupListWidget() {
                      SkDrawCommand::GetCommandString(SkDrawCommand::kSaveLayer_OpType)));
     SkASSERT(!strcmp("Restore",
                      SkDrawCommand::GetCommandString(SkDrawCommand::kRestore_OpType)));
-    SkASSERT(!strcmp("BeginCommentGroup",
-                     SkDrawCommand::GetCommandString(SkDrawCommand::kBeginCommentGroup_OpType)));
-    SkASSERT(!strcmp("EndCommentGroup",
-                     SkDrawCommand::GetCommandString(SkDrawCommand::kEndCommentGroup_OpType)));
     SkASSERT(!strcmp("BeginDrawPicture",
                      SkDrawCommand::GetCommandString(SkDrawCommand::kBeginDrawPicture_OpType)));
     SkASSERT(!strcmp("EndDrawPicture",
@@ -799,7 +795,6 @@ void SkDebuggerGUI::setupListWidget() {
         item->setData(Qt::UserRole + 1, counter++);
 
         if (0 == strcmp("Restore", commandString.c_str()) ||
-            0 == strcmp("EndCommentGroup", commandString.c_str()) ||
             0 == strcmp("EndDrawPicture", commandString.c_str())) {
             indent -= 10;
         }
@@ -808,7 +803,6 @@ void SkDebuggerGUI::setupListWidget() {
 
         if (0 == strcmp("Save", commandString.c_str()) ||
             0 == strcmp("SaveLayer", commandString.c_str()) ||
-            0 == strcmp("BeginCommentGroup", commandString.c_str()) ||
             0 == strcmp("BeginDrawPicture", commandString.c_str())) {
             indent += 10;
         }

experimental/PdfViewer/SkNulCanvas.h

@@ -26,9 +26,6 @@ public:
     explicit SkNulCanvas(const SkBitmap& bitmap) : SkCanvas(bitmap) {}
     virtual ~SkNulCanvas() {}
 
-    void beginCommentGroup(const char* description) override {}
-    void addComment(const char* kywd, const char* value) override {}
-    void endCommentGroup() override {}
     SkDrawFilter* setDrawFilter(SkDrawFilter* filter) override {return NULL;}
 
     bool isClipEmpty() const override { return false; }

gm/rects.cpp

@@ -252,14 +252,10 @@ protected:
     }
 
     void onDraw(SkCanvas* canvas) override {
-        SkAutoCommentBlock acb(canvas, "onDraw");
-
         canvas->translate(20 * SK_Scalar1, 20 * SK_Scalar1);
 
         int testCount = 0;
 
-        canvas->addComment("Test", "Various Paints");
-
         for (int i = 0; i < fPaints.count(); ++i) {
             for (int j = 0; j < fRects.count(); ++j, ++testCount) {
                 canvas->save();
@@ -269,8 +265,6 @@ protected:
             }
         }
 
-        canvas->addComment("Test", "Matrices");
-
         SkPaint paint;
         paint.setColor(SK_ColorWHITE);
         paint.setAntiAlias(true);

include/core/SkCanvas.h

@@ -1033,20 +1033,6 @@ public:
      */
     void drawDrawable(SkDrawable* drawable);
 
-    /** Add comments. beginCommentGroup/endCommentGroup open/close a new group.
-        Each comment added via addComment is notionally attached to its
-        enclosing group. Top-level comments simply belong to no group.
-     */
-    virtual void beginCommentGroup(const char* /*description*/) {
-        // do nothing. Subclasses may do something
-    }
-    virtual void addComment(const char* /*kywd*/, const char* /*value*/) {
-        // do nothing. Subclasses may do something
-    }
-    virtual void endCommentGroup() {
-        // do nothing. Subclasses may do something
-    }
-
     //////////////////////////////////////////////////////////////////////////
 
     /** Get the current filter object. The filter's reference count is not
@@ -1425,28 +1411,6 @@ private:
 };
 #define SkAutoCanvasRestore(...) SK_REQUIRE_LOCAL_VAR(SkAutoCanvasRestore)
 
-/** Stack helper class to automatically open and close a comment block
- */
-class SkAutoCommentBlock : SkNoncopyable {
-public:
-    SkAutoCommentBlock(SkCanvas* canvas, const char* description) {
-        fCanvas = canvas;
-        if (fCanvas) {
-            fCanvas->beginCommentGroup(description);
-        }
-    }
-
-    ~SkAutoCommentBlock() {
-        if (fCanvas) {
-            fCanvas->endCommentGroup();
-        }
-    }
-
-private:
-    SkCanvas* fCanvas;
-};
-#define SkAutoCommentBlock(...) SK_REQUIRE_LOCAL_VAR(SkAutoCommentBlock)
-
 /**
  *  If the caller wants read-only access to the pixels in a canvas, it can just
  *  call canvas->peekPixels(), since that is the fastest way to "peek" at the

include/core/SkPicture.h

@@ -179,6 +179,9 @@ private:
     static_assert(MIN_PICTURE_VERSION <= 41,
                   "Remove kFontFileName and related code from SkFontDescriptor.cpp.");
 
+    static_assert(MIN_PICTURE_VERSION <= 42,
+                  "Remove COMMENT API handlers from SkPicturePlayback.cpp");
+
     static bool IsValidPictInfo(const SkPictInfo& info);
     static SkPicture* Forwardport(const SkPictInfo&, const SkPictureData*);
 

include/utils/SkDumpCanvas.h

@@ -49,10 +49,6 @@ public:
         kDrawPatch_Verb,
         kDrawData_Verb, // obsolete
 
-        kBeginCommentGroup_Verb,
-        kAddComment_Verb,
-        kEndCommentGroup_Verb,
-
         kCull_Verb
     };
 
@@ -75,10 +71,6 @@ public:
 
     int getNestLevel() const { return fNestLevel; }
 
-    void beginCommentGroup(const char* description) override;
-    void addComment(const char* kywd, const char* value) override;
-    void endCommentGroup() override;
-
 protected:
     void willSave() override;
     SaveLayerStrategy willSaveLayer(const SkRect*, const SkPaint*, SaveFlags) override;

include/utils/SkNWayCanvas.h

@@ -25,10 +25,6 @@ public:
 
     SkDrawFilter* setDrawFilter(SkDrawFilter*) override;
 
-    void beginCommentGroup(const char* description) override;
-    void addComment(const char* kywd, const char* value) override;
-    void endCommentGroup() override;
-
 protected:
     SkTDArray<SkCanvas*> fList;
 

src/core/SkPictureFlat.h

@@ -57,9 +57,9 @@ enum DrawType {
     SKEW,
     TRANSLATE,
     NOOP,
-    BEGIN_COMMENT_GROUP,
+    BEGIN_COMMENT_GROUP, // deprecated (M44)
-    COMMENT,
+    COMMENT,             // deprecated (M44)
-    END_COMMENT_GROUP,
+    END_COMMENT_GROUP,   // deprecated (M44)
 
     // new ops -- feel free to re-alphabetize on next version bump
     DRAW_DRRECT,

src/core/SkPicturePlayback.cpp

@@ -204,18 +204,18 @@ void SkPicturePlayback::handleOp(SkReader32* reader,
             reader->readRRect(&inner);
             canvas->drawDRRect(outer, inner, paint);
         } break;
-        case BEGIN_COMMENT_GROUP: {
+        case BEGIN_COMMENT_GROUP:
-            const char* desc = reader->readString();
+            reader->readString();
-            canvas->beginCommentGroup(desc);
+            // deprecated (M44)
-        } break;
+            break;
-        case COMMENT: {
+        case COMMENT:
-            const char* kywd = reader->readString();
+            reader->readString();
-            const char* value = reader->readString();
+            reader->readString();
-            canvas->addComment(kywd, value);
+            // deprecated (M44)
-        } break;
+            break;
-        case END_COMMENT_GROUP: {
+        case END_COMMENT_GROUP:
-            canvas->endCommentGroup();
+            // deprecated (M44)
-        } break;
+            break;
         case DRAW_OVAL: {
             const SkPaint& paint = *fPictureData->getPaint(reader);
             canvas->drawOval(reader->skipT<SkRect>(), paint);

src/core/SkPictureRecord.cpp

@@ -815,33 +815,6 @@ void SkPictureRecord::onDrawPatch(const SkPoint cubics[12], const SkColor colors
     this->validate(initialOffset, size);
 }
 
-void SkPictureRecord::beginCommentGroup(const char* description) {
-    // op/size + length of string + \0 terminated chars
-    size_t length = strlen(description);
-    size_t size = 2 * kUInt32Size + SkAlign4(length + 1);
-    size_t initialOffset = this->addDraw(BEGIN_COMMENT_GROUP, &size);
-    fWriter.writeString(description, length);
-    this->validate(initialOffset, size);
-}
-
-void SkPictureRecord::addComment(const char* kywd, const char* value) {
-    // op/size + 2x length of string + 2x \0 terminated chars
-    size_t kywdLen = strlen(kywd);
-    size_t valueLen = strlen(value);
-    size_t size = 3 * kUInt32Size + SkAlign4(kywdLen + 1) + SkAlign4(valueLen + 1);
-    size_t initialOffset = this->addDraw(COMMENT, &size);
-    fWriter.writeString(kywd, kywdLen);
-    fWriter.writeString(value, valueLen);
-    this->validate(initialOffset, size);
-}
-
-void SkPictureRecord::endCommentGroup() {
-    // op/size
-    size_t size = 1 * kUInt32Size;
-    size_t initialOffset = this->addDraw(END_COMMENT_GROUP, &size);
-    this->validate(initialOffset, size);
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 
 SkSurface* SkPictureRecord::onNewSurface(const SkImageInfo& info, const SkSurfaceProps&) {

src/core/SkPictureRecord.h

@@ -29,10 +29,6 @@ public:
     SkPictureRecord(const SkISize& dimensions, uint32_t recordFlags);
     virtual ~SkPictureRecord();
 
-    void beginCommentGroup(const char* description) override;
-    void addComment(const char* kywd, const char* value) override;
-    void endCommentGroup() override;
-
     const SkTDArray<const SkPicture* >& getPictureRefs() const {
         return fPictureRefs;
     }

src/core/SkRecordDraw.cpp

@@ -86,10 +86,6 @@ DRAW(ClipRRect, clipRRect(r.rrect, r.opAA.op, r.opAA.aa));
 DRAW(ClipRect, clipRect(r.rect, r.opAA.op, r.opAA.aa));
 DRAW(ClipRegion, clipRegion(r.region, r.op));
 
-DRAW(BeginCommentGroup, beginCommentGroup(r.description));
-DRAW(AddComment, addComment(r.key, r.value));
-DRAW(EndCommentGroup, endCommentGroup());
-
 DRAW(DrawBitmap, drawBitmap(r.bitmap.shallowCopy(), r.left, r.top, r.paint));
 DRAW(DrawBitmapNine, drawBitmapNine(r.bitmap.shallowCopy(), r.center, r.dst, r.paint));
 DRAW(DrawBitmapRectToRect,
@@ -289,9 +285,6 @@ private:
     void trackBounds(const ClipRRect&)         { this->pushControl(); }
     void trackBounds(const ClipPath&)          { this->pushControl(); }
     void trackBounds(const ClipRegion&)        { this->pushControl(); }
-    void trackBounds(const BeginCommentGroup&) { this->pushControl(); }
-    void trackBounds(const AddComment&)        { this->pushControl(); }
-    void trackBounds(const EndCommentGroup&)   { this->pushControl(); }
 
     // For all other ops, we can calculate and store the bounds directly now.
     template <typename T> void trackBounds(const T& op) {

src/core/SkRecorder.cpp

@@ -350,15 +350,3 @@ void SkRecorder::onClipRegion(const SkRegion& deviceRgn, SkRegion::Op op) {
     APPEND(ClipRegion, this->devBounds(), delay_copy(deviceRgn), op);
 }
 
-void SkRecorder::beginCommentGroup(const char* description) {
-    APPEND(BeginCommentGroup, this->copy(description));
-}
-
-void SkRecorder::addComment(const char* key, const char* value) {
-    APPEND(AddComment, this->copy(key), this->copy(value));
-}
-
-void SkRecorder::endCommentGroup() {
-    APPEND(EndCommentGroup);
-}
-

src/core/SkRecorder.h

@@ -116,10 +116,6 @@ public:
 
     void onDrawPicture(const SkPicture*, const SkMatrix*, const SkPaint*) override;
 
-    void beginCommentGroup(const char*) override;
-    void addComment(const char*, const char*) override;
-    void endCommentGroup() override;
-
     SkSurface* onNewSurface(const SkImageInfo&, const SkSurfaceProps&) override { return NULL; }
 
     void flushMiniRecorder();

src/core/SkRecords.h

@@ -36,9 +36,6 @@ namespace SkRecords {
     M(ClipRRect)                                                    \
     M(ClipRect)                                                     \
     M(ClipRegion)                                                   \
-    M(BeginCommentGroup)                                            \
-    M(AddComment)                                                   \
-    M(EndCommentGroup)                                              \
     M(DrawBitmap)                                                   \
     M(DrawBitmapNine)                                               \
     M(DrawBitmapRectToRect)                                         \
@@ -253,10 +250,6 @@ RECORD3(ClipRRect,  SkIRect, devBounds, SkRRect,       rrect, RegionOpAndAA, opA
 RECORD3(ClipRect,   SkIRect, devBounds, SkRect,         rect, RegionOpAndAA, opAA);
 RECORD3(ClipRegion, SkIRect, devBounds, SkRegion,     region, SkRegion::Op,    op);
 
-RECORD1(BeginCommentGroup, PODArray<char>, description);
-RECORD2(AddComment, PODArray<char>, key, PODArray<char>, value);
-RECORD0(EndCommentGroup);
-
 // While not strictly required, if you have an SkPaint, it's fastest to put it first.
 RECORD4(DrawBitmap, Optional<SkPaint>, paint,
                     ImmutableBitmap, bitmap,

src/pipe/SkGPipeWrite.cpp

@@ -238,10 +238,6 @@ public:
         return bytesAllocated;
     }
 
-    void beginCommentGroup(const char* description) override;
-    void addComment(const char* kywd, const char* value) override;
-    void endCommentGroup() override;
-
     /**
      * Flatten an SkBitmap to send to the reader, where it will be referenced
      * according to slot.
@@ -1146,18 +1142,6 @@ void SkGPipeCanvas::onDrawPatch(const SkPoint cubics[12], const SkColor colors[4
     }
 }
 
-void SkGPipeCanvas::beginCommentGroup(const char* description) {
-    // ignore for now
-}
-
-void SkGPipeCanvas::addComment(const char* kywd, const char* value) {
-    // ignore for now
-}
-
-void SkGPipeCanvas::endCommentGroup() {
-    // ignore for now
-}
-
 void SkGPipeCanvas::flushRecording(bool detachCurrentBlock) {
     this->doNotify();
     if (detachCurrentBlock) {

src/utils/SkDumpCanvas.cpp

@@ -490,18 +490,6 @@ void SkDumpCanvas::onDrawPatch(const SkPoint cubics[12], const SkColor colors[4]
               texCoords[2].x(), texCoords[2].y(), texCoords[3].x(), texCoords[3].y());
 }
 
-void SkDumpCanvas::beginCommentGroup(const char* description) {
-    this->dump(kBeginCommentGroup_Verb, NULL, "beginCommentGroup(%s)", description);
-}
-
-void SkDumpCanvas::addComment(const char* kywd, const char* value) {
-    this->dump(kAddComment_Verb, NULL, "addComment(%s, %s)", kywd, value);
-}
-
-void SkDumpCanvas::endCommentGroup() {
-    this->dump(kEndCommentGroup_Verb, NULL, "endCommentGroup()");
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 ///////////////////////////////////////////////////////////////////////////////
 

src/utils/SkNWayCanvas.cpp

@@ -308,24 +308,3 @@ SkDrawFilter* SkNWayCanvas::setDrawFilter(SkDrawFilter* filter) {
     }
     return this->INHERITED::setDrawFilter(filter);
 }
-
-void SkNWayCanvas::beginCommentGroup(const char* description) {
-    Iter iter(fList);
-    while (iter.next()) {
-        iter->beginCommentGroup(description);
-    }
-}
-
-void SkNWayCanvas::addComment(const char* kywd, const char* value) {
-    Iter iter(fList);
-    while (iter.next()) {
-        iter->addComment(kywd, value);
-    }
-}
-
-void SkNWayCanvas::endCommentGroup() {
-    Iter iter(fList);
-    while (iter.next()) {
-        iter->endCommentGroup();
-    }
-}

src/utils/debugger/SkDebugCanvas.cpp

@@ -432,18 +432,6 @@ void SkDebugCanvas::onDrawImageRect(const SkImage* image, const SkRect* src, con
     SkDebugf("SkDebugCanvas::onDrawImageRect unimplemented\n");
 }
 
-void SkDebugCanvas::beginCommentGroup(const char* description) {
-    this->addDrawCommand(new SkBeginCommentGroupCommand(description));
-}
-
-void SkDebugCanvas::addComment(const char* kywd, const char* value) {
-    this->addDrawCommand(new SkCommentCommand(kywd, value));
-}
-
-void SkDebugCanvas::endCommentGroup() {
-    this->addDrawCommand(new SkEndCommentGroupCommand());
-}
-
 void SkDebugCanvas::onDrawOval(const SkRect& oval, const SkPaint& paint) {
     this->addDrawCommand(new SkDrawOvalCommand(oval, paint));
 }

src/utils/debugger/SkDebugCanvas.h

@@ -142,10 +142,6 @@ public:
 // Inherited from SkCanvas
 ////////////////////////////////////////////////////////////////////////////////
 
-    void beginCommentGroup(const char* description) override;
-    void addComment(const char* kywd, const char* value) override;
-    void endCommentGroup() override;
-
     static const int kVizImageHeight = 256;
     static const int kVizImageWidth = 256;
 

src/utils/debugger/SkDrawCommand.cpp

@@ -25,13 +25,11 @@ SkDrawCommand::~SkDrawCommand() {
 
 const char* SkDrawCommand::GetCommandString(OpType type) {
     switch (type) {
-        case kBeginCommentGroup_OpType: return "BeginCommentGroup";
         case kBeginDrawPicture_OpType: return "BeginDrawPicture";
         case kClipPath_OpType: return "ClipPath";
         case kClipRegion_OpType: return "ClipRegion";
         case kClipRect_OpType: return "ClipRect";
         case kClipRRect_OpType: return "ClipRRect";
-        case kComment_OpType: return "Comment";
         case kConcat_OpType: return "Concat";
         case kDrawBitmap_OpType: return "DrawBitmap";
         case kDrawBitmapNine_OpType: return "DrawBitmapNine";
@@ -52,7 +50,6 @@ const char* SkDrawCommand::GetCommandString(OpType type) {
         case kDrawTextBlob_OpType: return "DrawTextBlob";
         case kDrawTextOnPath_OpType: return "DrawTextOnPath";
         case kDrawVertices_OpType: return "DrawVertices";
-        case kEndCommentGroup_OpType: return "EndCommentGroup";
         case kEndDrawPicture_OpType: return "EndDrawPicture";
         case kRestore_OpType: return "Restore";
         case kSave_OpType: return "Save";
@@ -361,27 +358,6 @@ bool SkDrawBitmapRectCommand::render(SkCanvas* canvas) const {
     return true;
 }
 
-SkBeginCommentGroupCommand::SkBeginCommentGroupCommand(const char* description)
-    : INHERITED(kBeginCommentGroup_OpType)
-    , fDescription(description) {
-    SkString* temp = new SkString;
-    temp->appendf("Description: %s", description);
-    fInfo.push(temp);
-}
-
-SkCommentCommand::SkCommentCommand(const char* kywd, const char* value)
-    : INHERITED(kComment_OpType)
-    , fKywd(kywd)
-    , fValue(value) {
-    SkString* temp = new SkString;
-    temp->appendf("%s: %s", kywd, value);
-    fInfo.push(temp);
-}
-
-SkEndCommentGroupCommand::SkEndCommentGroupCommand()
-    : INHERITED(kEndCommentGroup_OpType) {
-}
-
 SkDrawOvalCommand::SkDrawOvalCommand(const SkRect& oval, const SkPaint& paint)
     : INHERITED(kDrawOval_OpType) {
     fOval = oval;

src/utils/debugger/SkDrawCommand.h

@@ -15,13 +15,11 @@
 class SK_API SkDrawCommand {
 public:
     enum OpType {
-        kBeginCommentGroup_OpType,
         kBeginDrawPicture_OpType,
         kClipPath_OpType,
         kClipRegion_OpType,
         kClipRect_OpType,
         kClipRRect_OpType,
-        kComment_OpType,
         kConcat_OpType,
         kDrawBitmap_OpType,
         kDrawBitmapNine_OpType,
@@ -42,7 +40,6 @@ public:
         kDrawTextBlob_OpType,
         kDrawTextOnPath_OpType,
         kDrawVertices_OpType,
-        kEndCommentGroup_OpType,
         kEndDrawPicture_OpType,
         kRestore_OpType,
         kSave_OpType,
@@ -267,41 +264,6 @@ private:
     typedef SkDrawCommand INHERITED;
 };
 
-class SkBeginCommentGroupCommand : public SkDrawCommand {
-public:
-    SkBeginCommentGroupCommand(const char* description);
-    void execute(SkCanvas* canvas) const override {
-        canvas->beginCommentGroup(fDescription.c_str());
-    };
-private:
-    SkString fDescription;
-
-    typedef SkDrawCommand INHERITED;
-};
-
-class SkCommentCommand : public SkDrawCommand {
-public:
-    SkCommentCommand(const char* kywd, const char* value);
-    void execute(SkCanvas* canvas) const override {
-        canvas->addComment(fKywd.c_str(), fValue.c_str());
-    };
-private:
-    SkString fKywd;
-    SkString fValue;
-
-    typedef SkDrawCommand INHERITED;
-};
-
-class SkEndCommentGroupCommand : public SkDrawCommand {
-public:
-    SkEndCommentGroupCommand();
-    void execute(SkCanvas* canvas) const override {
-        canvas->endCommentGroup();
-    };
-private:
-    typedef SkDrawCommand INHERITED;
-};
-
 class SkDrawOvalCommand : public SkDrawCommand {
 public:
     SkDrawOvalCommand(const SkRect& oval, const SkPaint& paint);

tests/CanvasTest.cpp

@@ -348,9 +348,6 @@ SIMPLE_TEST_STEP(ClipRect, clipRect(d.fRect));
 SIMPLE_TEST_STEP(ClipPath, clipPath(d.fPath));
 SIMPLE_TEST_STEP(ClipRegion, clipRegion(d.fRegion, SkRegion::kReplace_Op));
 SIMPLE_TEST_STEP(Clear, clear(d.fColor));
-SIMPLE_TEST_STEP(BeginGroup, beginCommentGroup(d.fText.c_str()));
-SIMPLE_TEST_STEP(AddComment, addComment(d.fText.c_str(), d.fText.c_str()));
-SIMPLE_TEST_STEP(EndGroup, endCommentGroup());
 
 ///////////////////////////////////////////////////////////////////////////////
 // Complex test steps

tests/RecorderTest.cpp

@@ -50,25 +50,6 @@ DEF_TEST(Recorder, r) {
     REPORTER_ASSERT(r, 1 == tally.count<SkRecords::DrawRect>());
 }
 
-// All of Skia will work fine without support for comment groups, but
-// Chrome's inspector can break.  This serves as a simple regression test.
-DEF_TEST(Recorder_CommentGroups, r) {
-    SkRecord record;
-    SkRecorder recorder(&record, 1920, 1080);
-
-    recorder.beginCommentGroup("test");
-        recorder.addComment("foo", "bar");
-        recorder.addComment("baz", "quux");
-    recorder.endCommentGroup();
-
-    Tally tally;
-    tally.apply(record);
-
-    REPORTER_ASSERT(r, 1 == tally.count<SkRecords::BeginCommentGroup>());
-    REPORTER_ASSERT(r, 2 == tally.count<SkRecords::AddComment>());
-    REPORTER_ASSERT(r, 1 == tally.count<SkRecords::EndCommentGroup>());
-}
-
 // Regression test for leaking refs held by optional arguments.
 DEF_TEST(Recorder_RefLeaking, r) {
     // We use SaveLayer to test: