From 460a23e6fd6b71c80d5515300c6b989cd3383029 Mon Sep 17 00:00:00 2001 From: "scroggo@google.com" Date: Thu, 16 Aug 2012 17:56:49 +0000 Subject: 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 --- src/pipe/SkGPipePriv.h | 8 +++++--- src/pipe/SkGPipeRead.cpp | 18 +++++++++--------- src/pipe/SkGPipeWrite.cpp | 17 +++++++++-------- 3 files changed, 23 insertions(+), 20 deletions(-) (limited to 'src/pipe') 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(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(reader); const SkRect* dst = skip(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(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(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; -- cgit v1.2.3