Make a flipped fDirtyBit always mean "this field is not the default".

Haven't been able to measure any perf effect, so I figure I'll let the bots have at it.

BUG=skia:
R=mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/595583003
This commit is contained in:
mtklein 2014-09-22 11:40:33 -07:00 committed by Commit bot
parent baae2f1aa2
commit ee902cd4f4
2 changed files with 33 additions and 31 deletions

View File

@ -190,8 +190,10 @@ SkPaint& SkPaint::operator=(const SkPaint& src) {
bool operator==(const SkPaint& a, const SkPaint& b) { bool operator==(const SkPaint& a, const SkPaint& b) {
#define EQUAL(field) (a.field == b.field) #define EQUAL(field) (a.field == b.field)
// Don't check fGenerationID or fDirtyBits, which can be different for logically equal paints. // Don't check fGenerationID, which can be different for logically equal paints.
return EQUAL(fTypeface) // fDirtyBits is a very quick check for non-equality, so check it first.
return EQUAL(fDirtyBits)
&& EQUAL(fTypeface)
&& EQUAL(fPathEffect) && EQUAL(fPathEffect)
&& EQUAL(fShader) && EQUAL(fShader)
&& EQUAL(fXfermode) && EQUAL(fXfermode)
@ -315,7 +317,7 @@ void SkPaint::setStyle(Style style) {
void SkPaint::setColor(SkColor color) { void SkPaint::setColor(SkColor color) {
GEN_ID_INC_EVAL(color != fColor); GEN_ID_INC_EVAL(color != fColor);
fColor = color; fColor = color;
fDirtyBits |= kColor_DirtyBit; fDirtyBits = SkSetClearMask(fDirtyBits, color != SK_ColorBLACK, kColor_DirtyBit);
} }
void SkPaint::setAlpha(U8CPU a) { void SkPaint::setAlpha(U8CPU a) {
@ -331,7 +333,7 @@ void SkPaint::setStrokeWidth(SkScalar width) {
if (width >= 0) { if (width >= 0) {
GEN_ID_INC_EVAL(width != fWidth); GEN_ID_INC_EVAL(width != fWidth);
fWidth = width; fWidth = width;
fDirtyBits |= kStrokeWidth_DirtyBit; fDirtyBits = SkSetClearMask(fDirtyBits, width != 0, kStrokeWidth_DirtyBit);
} else { } else {
#ifdef SK_REPORT_API_RANGE_CHECK #ifdef SK_REPORT_API_RANGE_CHECK
SkDebugf("SkPaint::setStrokeWidth() called with negative value\n"); SkDebugf("SkPaint::setStrokeWidth() called with negative value\n");
@ -343,7 +345,9 @@ void SkPaint::setStrokeMiter(SkScalar limit) {
if (limit >= 0) { if (limit >= 0) {
GEN_ID_INC_EVAL(limit != fMiterLimit); GEN_ID_INC_EVAL(limit != fMiterLimit);
fMiterLimit = limit; fMiterLimit = limit;
fDirtyBits |= kStrokeMiter_DirtyBit; fDirtyBits = SkSetClearMask(fDirtyBits,
limit != SkPaintDefaults_MiterLimit,
kStrokeMiter_DirtyBit);
} else { } else {
#ifdef SK_REPORT_API_RANGE_CHECK #ifdef SK_REPORT_API_RANGE_CHECK
SkDebugf("SkPaint::setStrokeMiter() called with negative value\n"); SkDebugf("SkPaint::setStrokeMiter() called with negative value\n");
@ -390,7 +394,7 @@ void SkPaint::setTextSize(SkScalar ts) {
if (ts >= 0) { if (ts >= 0) {
GEN_ID_INC_EVAL(ts != fTextSize); GEN_ID_INC_EVAL(ts != fTextSize);
fTextSize = ts; fTextSize = ts;
fDirtyBits |= kTextSize_DirtyBit; fDirtyBits = SkSetClearMask(fDirtyBits, ts != SkPaintDefaults_TextSize, kTextSize_DirtyBit);
} else { } else {
#ifdef SK_REPORT_API_RANGE_CHECK #ifdef SK_REPORT_API_RANGE_CHECK
SkDebugf("SkPaint::setTextSize() called with negative value\n"); SkDebugf("SkPaint::setTextSize() called with negative value\n");
@ -401,13 +405,13 @@ void SkPaint::setTextSize(SkScalar ts) {
void SkPaint::setTextScaleX(SkScalar scaleX) { void SkPaint::setTextScaleX(SkScalar scaleX) {
GEN_ID_INC_EVAL(scaleX != fTextScaleX); GEN_ID_INC_EVAL(scaleX != fTextScaleX);
fTextScaleX = scaleX; fTextScaleX = scaleX;
fDirtyBits |= kTextScaleX_DirtyBit; fDirtyBits = SkSetClearMask(fDirtyBits, scaleX != SK_Scalar1, kTextScaleX_DirtyBit);
} }
void SkPaint::setTextSkewX(SkScalar skewX) { void SkPaint::setTextSkewX(SkScalar skewX) {
GEN_ID_INC_EVAL(skewX != fTextSkewX); GEN_ID_INC_EVAL(skewX != fTextSkewX);
fTextSkewX = skewX; fTextSkewX = skewX;
fDirtyBits |= kTextSkewX_DirtyBit; fDirtyBits = SkSetClearMask(fDirtyBits, skewX != 0, kTextSkewX_DirtyBit);
} }
void SkPaint::setTextEncoding(TextEncoding encoding) { void SkPaint::setTextEncoding(TextEncoding encoding) {
@ -423,43 +427,38 @@ void SkPaint::setTextEncoding(TextEncoding encoding) {
/////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////
// Returns dst with the given bitmask enabled or disabled, depending on value.
inline static uint32_t set_mask(uint32_t dst, uint32_t bitmask, bool value) {
return value ? (dst | bitmask) : (dst & ~bitmask);
}
SkTypeface* SkPaint::setTypeface(SkTypeface* font) { SkTypeface* SkPaint::setTypeface(SkTypeface* font) {
SkRefCnt_SafeAssign(fTypeface, font); SkRefCnt_SafeAssign(fTypeface, font);
GEN_ID_INC; GEN_ID_INC;
fDirtyBits = set_mask(fDirtyBits, kTypeface_DirtyBit, font != NULL); fDirtyBits = SkSetClearMask(fDirtyBits, font != NULL, kTypeface_DirtyBit);
return font; return font;
} }
SkRasterizer* SkPaint::setRasterizer(SkRasterizer* r) { SkRasterizer* SkPaint::setRasterizer(SkRasterizer* r) {
SkRefCnt_SafeAssign(fRasterizer, r); SkRefCnt_SafeAssign(fRasterizer, r);
GEN_ID_INC; GEN_ID_INC;
fDirtyBits = set_mask(fDirtyBits, kRasterizer_DirtyBit, r != NULL); fDirtyBits = SkSetClearMask(fDirtyBits, r != NULL, kRasterizer_DirtyBit);
return r; return r;
} }
SkDrawLooper* SkPaint::setLooper(SkDrawLooper* looper) { SkDrawLooper* SkPaint::setLooper(SkDrawLooper* looper) {
SkRefCnt_SafeAssign(fLooper, looper); SkRefCnt_SafeAssign(fLooper, looper);
GEN_ID_INC; GEN_ID_INC;
fDirtyBits = set_mask(fDirtyBits, kLooper_DirtyBit, looper != NULL); fDirtyBits = SkSetClearMask(fDirtyBits, looper != NULL, kLooper_DirtyBit);
return looper; return looper;
} }
SkImageFilter* SkPaint::setImageFilter(SkImageFilter* imageFilter) { SkImageFilter* SkPaint::setImageFilter(SkImageFilter* imageFilter) {
SkRefCnt_SafeAssign(fImageFilter, imageFilter); SkRefCnt_SafeAssign(fImageFilter, imageFilter);
GEN_ID_INC; GEN_ID_INC;
fDirtyBits = set_mask(fDirtyBits, kImageFilter_DirtyBit, imageFilter != NULL); fDirtyBits = SkSetClearMask(fDirtyBits, imageFilter != NULL, kImageFilter_DirtyBit);
return imageFilter; return imageFilter;
} }
SkAnnotation* SkPaint::setAnnotation(SkAnnotation* annotation) { SkAnnotation* SkPaint::setAnnotation(SkAnnotation* annotation) {
SkRefCnt_SafeAssign(fAnnotation, annotation); SkRefCnt_SafeAssign(fAnnotation, annotation);
GEN_ID_INC; GEN_ID_INC;
fDirtyBits = set_mask(fDirtyBits, kAnnotation_DirtyBit, annotation != NULL); fDirtyBits = SkSetClearMask(fDirtyBits, annotation != NULL, kAnnotation_DirtyBit);
return annotation; return annotation;
} }
@ -2206,21 +2205,21 @@ void SkPaint::unflatten(SkReadBuffer& buffer) {
SkShader* SkPaint::setShader(SkShader* shader) { SkShader* SkPaint::setShader(SkShader* shader) {
GEN_ID_INC_EVAL(shader != fShader); GEN_ID_INC_EVAL(shader != fShader);
SkRefCnt_SafeAssign(fShader, shader); SkRefCnt_SafeAssign(fShader, shader);
fDirtyBits = set_mask(fDirtyBits, kShader_DirtyBit, shader != NULL); fDirtyBits = SkSetClearMask(fDirtyBits, shader != NULL, kShader_DirtyBit);
return shader; return shader;
} }
SkColorFilter* SkPaint::setColorFilter(SkColorFilter* filter) { SkColorFilter* SkPaint::setColorFilter(SkColorFilter* filter) {
GEN_ID_INC_EVAL(filter != fColorFilter); GEN_ID_INC_EVAL(filter != fColorFilter);
SkRefCnt_SafeAssign(fColorFilter, filter); SkRefCnt_SafeAssign(fColorFilter, filter);
fDirtyBits = set_mask(fDirtyBits, kColorFilter_DirtyBit, filter != NULL); fDirtyBits = SkSetClearMask(fDirtyBits, filter != NULL, kColorFilter_DirtyBit);
return filter; return filter;
} }
SkXfermode* SkPaint::setXfermode(SkXfermode* mode) { SkXfermode* SkPaint::setXfermode(SkXfermode* mode) {
GEN_ID_INC_EVAL(mode != fXfermode); GEN_ID_INC_EVAL(mode != fXfermode);
SkRefCnt_SafeAssign(fXfermode, mode); SkRefCnt_SafeAssign(fXfermode, mode);
fDirtyBits = set_mask(fDirtyBits, kXfermode_DirtyBit, mode != NULL); fDirtyBits = SkSetClearMask(fDirtyBits, mode != NULL, kXfermode_DirtyBit);
return mode; return mode;
} }
@ -2228,21 +2227,21 @@ SkXfermode* SkPaint::setXfermodeMode(SkXfermode::Mode mode) {
SkSafeUnref(fXfermode); SkSafeUnref(fXfermode);
fXfermode = SkXfermode::Create(mode); fXfermode = SkXfermode::Create(mode);
GEN_ID_INC; GEN_ID_INC;
fDirtyBits = set_mask(fDirtyBits, kXfermode_DirtyBit, fXfermode != NULL); fDirtyBits = SkSetClearMask(fDirtyBits, fXfermode != NULL, kXfermode_DirtyBit);
return fXfermode; return fXfermode;
} }
SkPathEffect* SkPaint::setPathEffect(SkPathEffect* effect) { SkPathEffect* SkPaint::setPathEffect(SkPathEffect* effect) {
GEN_ID_INC_EVAL(effect != fPathEffect); GEN_ID_INC_EVAL(effect != fPathEffect);
SkRefCnt_SafeAssign(fPathEffect, effect); SkRefCnt_SafeAssign(fPathEffect, effect);
fDirtyBits = set_mask(fDirtyBits, kPathEffect_DirtyBit, effect != NULL); fDirtyBits = SkSetClearMask(fDirtyBits, effect != NULL, kPathEffect_DirtyBit);
return effect; return effect;
} }
SkMaskFilter* SkPaint::setMaskFilter(SkMaskFilter* filter) { SkMaskFilter* SkPaint::setMaskFilter(SkMaskFilter* filter) {
GEN_ID_INC_EVAL(filter != fMaskFilter); GEN_ID_INC_EVAL(filter != fMaskFilter);
SkRefCnt_SafeAssign(fMaskFilter, filter); SkRefCnt_SafeAssign(fMaskFilter, filter);
fDirtyBits = set_mask(fDirtyBits, kMaskFilter_DirtyBit, filter != NULL); fDirtyBits = SkSetClearMask(fDirtyBits, filter != NULL, kMaskFilter_DirtyBit);
return filter; return filter;
} }

View File

@ -318,19 +318,22 @@ DEF_TEST(Paint_regression_measureText, reporter) {
DEF_TEST(Paint_FlatteningTraits, r) { DEF_TEST(Paint_FlatteningTraits, r) {
SkPaint paint; SkPaint paint;
paint.setColor(0x00AABBCC); paint.setColor(0x00AABBCC);
paint.setTextScaleX(1.0f); // Encoded despite being the default value. paint.setTextScaleX(1.0f); // Default value, ignored.
paint.setTextSize(19); paint.setTextSize(19);
paint.setXfermode(SkXfermode::Create(SkXfermode::kModulate_Mode))->unref(); paint.setXfermode(SkXfermode::Create(SkXfermode::kModulate_Mode))->unref();
paint.setLooper(NULL); // Ignored. paint.setLooper(NULL); // Default value, ignored.
SkWriteBuffer writer; SkWriteBuffer writer;
SkPaint::FlatteningTraits::Flatten(writer, paint); SkPaint::FlatteningTraits::Flatten(writer, paint);
const size_t expectedBytesWritten = sizeof(void*) == 8 ? 36 : 32;
ASSERT(expectedBytesWritten == writer.bytesWritten());
const uint32_t* written = writer.getWriter32()->contiguousArray(); // BEGIN white box asserts: if the impl changes, these asserts may change
SkASSERT(written != NULL); const size_t expectedBytesWritten = sizeof(void*) == 8 ? 32 : 28;
ASSERT(*written == ((1<<0) | (1<<1) | (1<<2) | (1<<8))); // Dirty bits for our 4. ASSERT(expectedBytesWritten == writer.bytesWritten());
const uint32_t* written = writer.getWriter32()->contiguousArray();
SkASSERT(written != NULL);
ASSERT(*written == ((1<<0) | (1<<1) | (1<<8))); // Dirty bits for our 3.
// END white box asserts
SkReadBuffer reader(written, writer.bytesWritten()); SkReadBuffer reader(written, writer.bytesWritten());
SkPaint other; SkPaint other;