Fix a parenthesis bug.

SkGPipeCanvas::needOpBytes was being called with the wrong value due to a misplaced
parens in clipRect and clipPath. This can cause a crash if clip is called at just
the right (wrong) time. Instead of writing a boolean to the stream, I have added a
flag, which helps to avoid the parens problem.

Also renamed some flags from _DrawOpsFlag to _DrawOpFlag for consistency.

Lastly, added an assert that the size provided by the SkGPipeController is a multiple
of four.

Review URL: https://codereview.appspot.com/6453126

git-svn-id: http://skia.googlecode.com/svn/trunk@5134 2bbb7eff-a529-9590-31e7-b0007b416f81

3 files changed, 23 insertions, 20 deletions

diff --git a/src/pipe/SkGPipePriv.h b/src/pipe/SkGPipePriv.h
index c68f3ec338..edfe2b6edd 100644
--- a/src/pipe/SkGPipePriv.h
+++ b/src/pipe/SkGPipePriv.h
@@ -140,12 +140,14 @@ enum {
     kDrawVertices_HasIndices_DrawOpFlag  = 1 << 2,
 };
 enum {
-    kDrawBitmap_HasPaint_DrawOpsFlag   = 1 << 0,
+    kDrawBitmap_HasPaint_DrawOpFlag   = 1 << 0,
     // Specific to drawBitmapRect, but needs to be different from HasPaint,
     // which is used for all drawBitmap calls, so include it here.
-    kDrawBitmap_HasSrcRect_DrawOpsFlag = 1 << 1,
+    kDrawBitmap_HasSrcRect_DrawOpFlag = 1 << 1,
+};
+enum {
+    kClip_HasAntiAlias_DrawOpFlag = 1 << 0,
 };
-
 ///////////////////////////////////////////////////////////////////////////////
 
 class BitmapInfo : SkNoncopyable {
diff --git a/src/pipe/SkGPipeRead.cpp b/src/pipe/SkGPipeRead.cpp
index 651e8389ac..d217aeb85d 100644
--- a/src/pipe/SkGPipeRead.cpp
+++ b/src/pipe/SkGPipeRead.cpp
@@ -210,8 +210,8 @@ static void clipPath_rp(SkCanvas* canvas, SkReader32* reader, uint32_t op32,
                         SkGPipeState* state) {
     SkPath path;
     reader->readPath(&path);
-    canvas->clipPath(path, (SkRegion::Op)DrawOp_unpackData(op32),
-                     reader->readBool());
+    bool doAA = SkToBool(DrawOp_unpackFlags(op32) & kClip_HasAntiAlias_DrawOpFlag);
+    canvas->clipPath(path, (SkRegion::Op)DrawOp_unpackData(op32), doAA);
 }
 
 static void clipRegion_rp(SkCanvas* canvas, SkReader32* reader, uint32_t op32,
@@ -224,7 +224,7 @@ static void clipRegion_rp(SkCanvas* canvas, SkReader32* reader, uint32_t op32,
 static void clipRect_rp(SkCanvas* canvas, SkReader32* reader, uint32_t op32,
                         SkGPipeState* state) {
     const SkRect* rect = skip<SkRect>(reader);
-    bool doAA = reader->readBool();
+    bool doAA = SkToBool(DrawOp_unpackFlags(op32) & kClip_HasAntiAlias_DrawOpFlag);
     canvas->clipRect(*rect, (SkRegion::Op)DrawOp_unpackData(op32), doAA);
 }
 
@@ -443,7 +443,7 @@ BitmapHolder::BitmapHolder(SkReader32* reader, uint32_t op32,
 static void drawBitmap_rp(SkCanvas* canvas, SkReader32* reader, uint32_t op32,
                           SkGPipeState* state) {
     BitmapHolder holder(reader, op32, state);
-    bool hasPaint = SkToBool(DrawOp_unpackFlags(op32) & kDrawBitmap_HasPaint_DrawOpsFlag);
+    bool hasPaint = SkToBool(DrawOp_unpackFlags(op32) & kDrawBitmap_HasPaint_DrawOpFlag);
     SkScalar left = reader->readScalar();
     SkScalar top = reader->readScalar();
     canvas->drawBitmap(*holder.getBitmap(), left, top, hasPaint ? &state->paint() : NULL);
@@ -452,7 +452,7 @@ static void drawBitmap_rp(SkCanvas* canvas, SkReader32* reader, uint32_t op32,
 static void drawBitmapMatrix_rp(SkCanvas* canvas, SkReader32* reader, uint32_t op32,
                                 SkGPipeState* state) {
     BitmapHolder holder(reader, op32, state);
-    bool hasPaint = SkToBool(DrawOp_unpackFlags(op32) & kDrawBitmap_HasPaint_DrawOpsFlag);
+    bool hasPaint = SkToBool(DrawOp_unpackFlags(op32) & kDrawBitmap_HasPaint_DrawOpFlag);
     SkMatrix matrix;
     reader->readMatrix(&matrix);
     canvas->drawBitmapMatrix(*holder.getBitmap(), matrix,
@@ -462,7 +462,7 @@ static void drawBitmapMatrix_rp(SkCanvas* canvas, SkReader32* reader, uint32_t o
 static void drawBitmapNine_rp(SkCanvas* canvas, SkReader32* reader,
                               uint32_t op32, SkGPipeState* state) {
     BitmapHolder holder(reader, op32, state);
-    bool hasPaint = SkToBool(DrawOp_unpackFlags(op32) & kDrawBitmap_HasPaint_DrawOpsFlag);
+    bool hasPaint = SkToBool(DrawOp_unpackFlags(op32) & kDrawBitmap_HasPaint_DrawOpFlag);
     const SkIRect* center = skip<SkIRect>(reader);
     const SkRect* dst = skip<SkRect>(reader);
     canvas->drawBitmapNine(*holder.getBitmap(), *center, *dst,
@@ -473,8 +473,8 @@ static void drawBitmapRect_rp(SkCanvas* canvas, SkReader32* reader,
                               uint32_t op32, SkGPipeState* state) {
     BitmapHolder holder(reader, op32, state);
     unsigned flags = DrawOp_unpackFlags(op32);
-    bool hasPaint = SkToBool(flags & kDrawBitmap_HasPaint_DrawOpsFlag);
-    bool hasSrc = SkToBool(flags & kDrawBitmap_HasSrcRect_DrawOpsFlag);
+    bool hasPaint = SkToBool(flags & kDrawBitmap_HasPaint_DrawOpFlag);
+    bool hasSrc = SkToBool(flags & kDrawBitmap_HasSrcRect_DrawOpFlag);
     const SkIRect* src;
     if (hasSrc) {
         src = skip<SkIRect>(reader);
@@ -488,7 +488,7 @@ static void drawBitmapRect_rp(SkCanvas* canvas, SkReader32* reader,
 static void drawSprite_rp(SkCanvas* canvas, SkReader32* reader, uint32_t op32,
                           SkGPipeState* state) {
     BitmapHolder holder(reader, op32, state);
-    bool hasPaint = SkToBool(DrawOp_unpackFlags(op32) & kDrawBitmap_HasPaint_DrawOpsFlag);
+    bool hasPaint = SkToBool(DrawOp_unpackFlags(op32) & kDrawBitmap_HasPaint_DrawOpFlag);
     const SkIPoint* point = skip<SkIPoint>(reader);
     canvas->drawSprite(*holder.getBitmap(), point->fX, point->fY, hasPaint ? &state->paint() : NULL);
 }
diff --git a/src/pipe/SkGPipeWrite.cpp b/src/pipe/SkGPipeWrite.cpp
index aad4c8a9c0..42f82c9bcd 100644
--- a/src/pipe/SkGPipeWrite.cpp
+++ b/src/pipe/SkGPipeWrite.cpp
@@ -465,6 +465,7 @@ bool SkGPipeCanvas::needOpBytes(size_t needed) {
             fDone = true;
             return false;
         }
+        SkASSERT(SkIsAlign4(fBlockSize));
         fWriter.reset(block, fBlockSize);
         fBytesNotified = 0;
     }
@@ -616,10 +617,10 @@ void SkGPipeCanvas::setMatrix(const SkMatrix& matrix) {
 bool SkGPipeCanvas::clipRect(const SkRect& rect, SkRegion::Op rgnOp,
                              bool doAntiAlias) {
     NOTIFY_SETUP(this);
-    if (this->needOpBytes(sizeof(SkRect)) + sizeof(bool)) {
-        this->writeOp(kClipRect_DrawOp, 0, rgnOp);
+    if (this->needOpBytes(sizeof(SkRect))) {
+        unsigned flags = doAntiAlias & kClip_HasAntiAlias_DrawOpFlag;
+        this->writeOp(kClipRect_DrawOp, flags, rgnOp);
         fWriter.writeRect(rect);
-        fWriter.writeBool(doAntiAlias);
     }
     return this->INHERITED::clipRect(rect, rgnOp, doAntiAlias);
 }
@@ -627,10 +628,10 @@ bool SkGPipeCanvas::clipRect(const SkRect& rect, SkRegion::Op rgnOp,
 bool SkGPipeCanvas::clipPath(const SkPath& path, SkRegion::Op rgnOp,
                              bool doAntiAlias) {
     NOTIFY_SETUP(this);
-    if (this->needOpBytes(path.writeToMemory(NULL)) + sizeof(bool)) {
-        this->writeOp(kClipPath_DrawOp, 0, rgnOp);
+    if (this->needOpBytes(path.writeToMemory(NULL))) {
+        unsigned flags = doAntiAlias & kClip_HasAntiAlias_DrawOpFlag;
+        this->writeOp(kClipPath_DrawOp, flags, rgnOp);
         fWriter.writePath(path);
-        fWriter.writeBool(doAntiAlias);
     }
     // we just pass on the bounds of the path
     return this->INHERITED::clipRect(path.getBounds(), rgnOp, doAntiAlias);
@@ -705,7 +706,7 @@ bool SkGPipeCanvas::commonDrawBitmap(const SkBitmap& bm, DrawOps op,
                                      size_t opBytesNeeded,
                                      const SkPaint* paint) {
     if (paint != NULL) {
-        flags |= kDrawBitmap_HasPaint_DrawOpsFlag;
+        flags |= kDrawBitmap_HasPaint_DrawOpFlag;
         this->writePaint(*paint);
     }
     if (this->needOpBytes(opBytesNeeded)) {
@@ -738,7 +739,7 @@ void SkGPipeCanvas::drawBitmapRect(const SkBitmap& bm, const SkIRect* src,
     bool hasSrc = src != NULL;
     unsigned flags;
     if (hasSrc) {
-        flags = kDrawBitmap_HasSrcRect_DrawOpsFlag;
+        flags = kDrawBitmap_HasSrcRect_DrawOpFlag;
         opBytesNeeded += sizeof(int32_t) * 4;
     } else {
         flags = 0;