From cafc9f9e80e30fa75ad8a952e7a290e72f211ce7 Mon Sep 17 00:00:00 2001 From: "reed@android.com" Date: Sat, 22 Aug 2009 03:44:57 +0000 Subject: [PATCH] fixes around isOpaque and dithering - copyTo() now preserves isOpaqueness, and BitmapCopyTest tests it - bitmap shader doesn't claim to have shadespan16 if dithering is on, since its sampler doesn't auto-dither (note that gradients do auto-dither in their 16bit sampler) - blitter setup just relies on the shader to report if its 16bit sampler can be called (allowing gradients to say yes regardless of dither, but bitmaps to say no if dithering is on) git-svn-id: http://skia.googlecode.com/svn/trunk@331 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkBitmap.h | 3 ++ samplecode/SampleDitherBitmap.cpp | 27 ++++++++-- src/core/SkBitmap.cpp | 2 + src/core/SkBitmapProcShader.cpp | 18 +++++-- src/core/SkBlitter.cpp | 14 ++--- src/core/SkColorTable.cpp | 8 +++ tests/BitmapCopyTest.cpp | 52 +++++++++++++++++++ .../SampleApp.xcodeproj/project.pbxproj | 8 ++- 8 files changed, 110 insertions(+), 22 deletions(-) diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h index 3c04a310a9..1c16b780e2 100644 --- a/include/core/SkBitmap.h +++ b/include/core/SkBitmap.h @@ -508,6 +508,9 @@ public: */ void setFlags(unsigned flags); + bool isOpaque() const { return (fFlags & kColorsAreOpaque_Flag) != 0; } + void setIsOpaque(bool isOpaque); + /** Returns the number of colors in the table. */ int count() const { return fCount; } diff --git a/samplecode/SampleDitherBitmap.cpp b/samplecode/SampleDitherBitmap.cpp index bd604cf84c..35816a213a 100644 --- a/samplecode/SampleDitherBitmap.cpp +++ b/samplecode/SampleDitherBitmap.cpp @@ -10,7 +10,7 @@ static SkBitmap make_bitmap() { SkPMColor* c = ctable->lockColors(); for (int i = 0; i < 256; i++) { - c[i] = SkPackARGB32(0xFF, 0, 0, i); + c[i] = SkPackARGB32(0xFF, i, 0, 0); } ctable->unlockColors(true); bm.setConfig(SkBitmap::kIndex8_Config, 256, 32); @@ -51,12 +51,31 @@ protected: canvas->drawColor(0xFFDDDDDD); } + static void setBitmapOpaque(SkBitmap* bm, bool isOpaque) { + SkAutoLockPixels alp(*bm); // needed for ctable + bm->setIsOpaque(isOpaque); + SkColorTable* ctable = bm->getColorTable(); + if (ctable) { + ctable->setIsOpaque(isOpaque); + } + } + static void draw2(SkCanvas* canvas, const SkBitmap& bm) { SkPaint paint; - - canvas->drawBitmap(bm, 0, 0, &paint); + SkBitmap bitmap(bm); + + setBitmapOpaque(&bitmap, false); + paint.setDither(false); + canvas->drawBitmap(bitmap, 0, 0, &paint); + paint.setDither(true); + canvas->drawBitmap(bitmap, 0, SkIntToScalar(bm.height() + 10), &paint); + + setBitmapOpaque(&bitmap, true); + SkScalar x = SkIntToScalar(bm.width() + 10); + paint.setDither(false); + canvas->drawBitmap(bitmap, x, 0, &paint); paint.setDither(true); - canvas->drawBitmap(bm, 0, SkIntToScalar(bm.height() + 10), &paint); + canvas->drawBitmap(bitmap, x, SkIntToScalar(bm.height() + 10), &paint); } virtual void onDraw(SkCanvas* canvas) { diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp index d87f001ed8..9478faef52 100644 --- a/src/core/SkBitmap.cpp +++ b/src/core/SkBitmap.cpp @@ -793,6 +793,8 @@ bool SkBitmap::copyTo(SkBitmap* dst, Config dstConfig, Allocator* alloc) const { canvas.drawBitmap(*this, 0, 0, &paint); } + tmp.setIsOpaque(this->isOpaque()); + dst->swap(tmp); return true; } diff --git a/src/core/SkBitmapProcShader.cpp b/src/core/SkBitmapProcShader.cpp index 7eb8ea148b..353388b74c 100644 --- a/src/core/SkBitmapProcShader.cpp +++ b/src/core/SkBitmapProcShader.cpp @@ -102,19 +102,19 @@ bool SkBitmapProcShader::setContext(const SkBitmap& device, } // update fFlags - fFlags = 0; + uint32_t flags = 0; if (bitmapIsOpaque && (255 == this->getPaintAlpha())) { - fFlags |= kOpaqueAlpha_Flag; + flags |= kOpaqueAlpha_Flag; } switch (fState.fBitmap->config()) { case SkBitmap::kRGB_565_Config: - fFlags |= (kHasSpan16_Flag | kIntrinsicly16_Flag); + flags |= (kHasSpan16_Flag | kIntrinsicly16_Flag); break; case SkBitmap::kIndex8_Config: case SkBitmap::kARGB_8888_Config: if (bitmapIsOpaque) { - fFlags |= kHasSpan16_Flag; + flags |= kHasSpan16_Flag; } break; case SkBitmap::kA8_Config: @@ -123,11 +123,19 @@ bool SkBitmapProcShader::setContext(const SkBitmap& device, break; } + if (paint.isDither()) { + // gradients can auto-dither in their 16bit sampler, but we don't so + // we clear the flag here + flags &= ~kHasSpan16_Flag; + } + // if we're only 1-pixel heigh, and we don't rotate, then we can claim this if (1 == fState.fBitmap->height() && only_scale_and_translate(this->getTotalInverse())) { - fFlags |= kConstInY_Flag; + flags |= kConstInY_Flag; } + + fFlags = flags; return true; } diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp index faf29c1712..e60887e588 100644 --- a/src/core/SkBlitter.cpp +++ b/src/core/SkBlitter.cpp @@ -896,16 +896,8 @@ SkBlitter* SkBlitter::Choose(const SkBitmap& device, ((SkPaint*)&paint)->setShader(shader)->unref(); } - bool doDither = paint.isDither(); - - if (shader) - { - if (!shader->setContext(device, paint, matrix)) - return SkNEW(SkNullBlitter); - - // disable dither if our shader is natively 16bit (no need to upsample) - if (shader->getFlags() & SkShader::kIntrinsicly16_Flag) - doDither = false; + if (shader && !shader->setContext(device, paint, matrix)) { + return SkNEW(SkNullBlitter); } switch (device.getConfig()) { @@ -929,7 +921,7 @@ SkBlitter* SkBlitter::Choose(const SkBitmap& device, { if (mode) SK_PLACEMENT_NEW_ARGS(blitter, SkRGB16_Shader_Xfermode_Blitter, storage, storageSize, (device, paint)); - else if (SkShader::CanCallShadeSpan16(shader->getFlags()) && !doDither) + else if (shader->canCallShadeSpan16()) SK_PLACEMENT_NEW_ARGS(blitter, SkRGB16_Shader16_Blitter, storage, storageSize, (device, paint)); else SK_PLACEMENT_NEW_ARGS(blitter, SkRGB16_Shader_Blitter, storage, storageSize, (device, paint)); diff --git a/src/core/SkColorTable.cpp b/src/core/SkColorTable.cpp index 41e6895fff..308287c1ea 100644 --- a/src/core/SkColorTable.cpp +++ b/src/core/SkColorTable.cpp @@ -132,6 +132,14 @@ const uint16_t* SkColorTable::lock16BitCache() return f16BitCache; } +void SkColorTable::setIsOpaque(bool isOpaque) { + if (isOpaque) { + fFlags |= kColorsAreOpaque_Flag; + } else { + fFlags &= ~kColorsAreOpaque_Flag; + } +} + /////////////////////////////////////////////////////////////////////////////// SkColorTable::SkColorTable(SkFlattenableReadBuffer& buffer) { diff --git a/tests/BitmapCopyTest.cpp b/tests/BitmapCopyTest.cpp index e54cde3319..2ae7b0af50 100644 --- a/tests/BitmapCopyTest.cpp +++ b/tests/BitmapCopyTest.cpp @@ -11,6 +11,57 @@ static const char* gConfigName[] = { "None", "A1", "A8", "Index8", "565", "4444", "8888", "RLE_Index8" }; +static void report_opaqueness(skiatest::Reporter* reporter, const SkBitmap& src, + const SkBitmap& dst) { + SkString str; + str.printf("src %s opaque:%d, dst %s opaque:%d", + gConfigName[src.config()], src.isOpaque(), + gConfigName[dst.config()], dst.isOpaque()); + reporter->reportFailed(str); +} + +static bool canHaveAlpha(SkBitmap::Config config) { + return config != SkBitmap::kRGB_565_Config; +} + +// copyTo() should preserve isOpaque when it makes sense +static void test_isOpaque(skiatest::Reporter* reporter, const SkBitmap& src, + SkBitmap::Config dstConfig) { + SkBitmap bitmap(src); + SkBitmap dst; + + // we need the lock so that we get a valid colorTable (when available) + SkAutoLockPixels alp(bitmap); + SkColorTable* ctable = bitmap.getColorTable(); + unsigned ctableFlags = ctable ? ctable->getFlags() : 0; + + if (canHaveAlpha(bitmap.config()) && canHaveAlpha(dstConfig)) { + bitmap.setIsOpaque(false); + if (ctable) { + ctable->setFlags(ctableFlags & ~SkColorTable::kColorsAreOpaque_Flag); + } + REPORTER_ASSERT(reporter, bitmap.copyTo(&dst, dstConfig)); + REPORTER_ASSERT(reporter, dst.config() == dstConfig); + if (bitmap.isOpaque() != dst.isOpaque()) { + report_opaqueness(reporter, bitmap, dst); + } + } + + bitmap.setIsOpaque(true); + if (ctable) { + ctable->setFlags(ctableFlags | SkColorTable::kColorsAreOpaque_Flag); + } + REPORTER_ASSERT(reporter, bitmap.copyTo(&dst, dstConfig)); + REPORTER_ASSERT(reporter, dst.config() == dstConfig); + if (bitmap.isOpaque() != dst.isOpaque()) { + report_opaqueness(reporter, bitmap, dst); + } + + if (ctable) { + ctable->setFlags(ctableFlags); + } +} + static void init_src(const SkBitmap& bitmap) { SkAutoLockPixels lock(bitmap); if (bitmap.getPixels()) { @@ -83,6 +134,7 @@ static void TestBitmapCopy(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, src.width() == dst.width()); REPORTER_ASSERT(reporter, src.height() == dst.height()); REPORTER_ASSERT(reporter, dst.config() == gPairs[j].fConfig); + test_isOpaque(reporter, src, dst.config()); if (src.config() == dst.config()) { SkAutoLockPixels srcLock(src); SkAutoLockPixels dstLock(dst); diff --git a/xcode/sampleapp/SampleApp.xcodeproj/project.pbxproj b/xcode/sampleapp/SampleApp.xcodeproj/project.pbxproj index 654f361795..3edf0e461f 100644 --- a/xcode/sampleapp/SampleApp.xcodeproj/project.pbxproj +++ b/xcode/sampleapp/SampleApp.xcodeproj/project.pbxproj @@ -47,7 +47,6 @@ 0057785F0FF17CCC00582CD9 /* SampleMipMap.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2794C04E0FE72903009AD112 /* SampleMipMap.cpp */; }; 005778B40FF5616F00582CD9 /* SampleShapes.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 003145310FB9B48F00B10956 /* SampleShapes.cpp */; }; 005E92DC0FF08507008965B9 /* SampleFilter2.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0041CE290F00A12400695E8C /* SampleFilter2.cpp */; }; - 005E92DE0FF0850E008965B9 /* SampleFillType.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0041CE270F00A12400695E8C /* SampleFillType.cpp */; }; 005E92E00FF08512008965B9 /* SampleFilter.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0041CE280F00A12400695E8C /* SampleFilter.cpp */; }; 007A7CB30F01658C00A2D6EE /* SamplePicture.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 007A7CA40F01658C00A2D6EE /* SamplePicture.cpp */; }; 007A7CB40F01658C00A2D6EE /* SamplePoints.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 007A7CA50F01658C00A2D6EE /* SamplePoints.cpp */; }; @@ -71,6 +70,8 @@ 00A7295D0FD8397600D5051F /* SampleAll.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2762F6740FCCCB01002BD8B4 /* SampleAll.cpp */; }; 00AF77B00FE2EA2D007F9650 /* SampleTestGL.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 00A729630FD93ED600D5051F /* SampleTestGL.cpp */; }; 00AF787E0FE94433007F9650 /* SamplePath.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 00003C640EFC22A8000FF73A /* SamplePath.cpp */; }; + 00AF9B18103CD5EB00CBBCB3 /* SampleDitherBitmap.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 00AF9B17103CD5EB00CBBCB3 /* SampleDitherBitmap.cpp */; }; + 00C1B809103857A400FA5948 /* SampleFillType.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0041CE270F00A12400695E8C /* SampleFillType.cpp */; }; 00F53F480FFCFC4D003FA70A /* SampleGradients.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 00C55DA00F8552DC000CAC09 /* SampleGradients.cpp */; }; 00FF39140FC6ED2C00915187 /* SampleEffects.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 00FF39130FC6ED2C00915187 /* SampleEffects.cpp */; }; 0156F80407C56A3000C6122B /* Foundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 0156F80307C56A3000C6122B /* Foundation.framework */; }; @@ -197,6 +198,7 @@ 00A7282D0FD43D3700D5051F /* SkMovie.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SkMovie.cpp; path = ../../src/images/SkMovie.cpp; sourceTree = SOURCE_ROOT; }; 00A7282E0FD43D3700D5051F /* SkMovie_gif.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SkMovie_gif.cpp; path = ../../src/images/SkMovie_gif.cpp; sourceTree = SOURCE_ROOT; }; 00A729630FD93ED600D5051F /* SampleTestGL.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleTestGL.cpp; path = ../../samplecode/SampleTestGL.cpp; sourceTree = SOURCE_ROOT; }; + 00AF9B17103CD5EB00CBBCB3 /* SampleDitherBitmap.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleDitherBitmap.cpp; path = ../../samplecode/SampleDitherBitmap.cpp; sourceTree = SOURCE_ROOT; }; 00C55DA00F8552DC000CAC09 /* SampleGradients.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleGradients.cpp; path = ../../samplecode/SampleGradients.cpp; sourceTree = SOURCE_ROOT; }; 00D6B5CB0F72DC4300C466B9 /* SampleFuzz.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleFuzz.cpp; path = ../../samplecode/SampleFuzz.cpp; sourceTree = SOURCE_ROOT; }; 00FF39130FC6ED2C00915187 /* SampleEffects.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleEffects.cpp; path = ../../samplecode/SampleEffects.cpp; sourceTree = SOURCE_ROOT; }; @@ -277,6 +279,7 @@ 0041CE250F00A12400695E8C /* SampleEmboss.cpp */, 0041CE260F00A12400695E8C /* SampleEncode.cpp */, 0041CE270F00A12400695E8C /* SampleFillType.cpp */, + 00AF9B17103CD5EB00CBBCB3 /* SampleDitherBitmap.cpp */, 0041CE280F00A12400695E8C /* SampleFilter.cpp */, 0041CE290F00A12400695E8C /* SampleFilter2.cpp */, 0041CE2A0F00A12400695E8C /* SampleFontCache.cpp */, @@ -568,13 +571,14 @@ 00AF787E0FE94433007F9650 /* SamplePath.cpp in Sources */, 0088C1160FEC311C00CE52F5 /* SampleXfermodes.cpp in Sources */, 005E92DC0FF08507008965B9 /* SampleFilter2.cpp in Sources */, - 005E92DE0FF0850E008965B9 /* SampleFillType.cpp in Sources */, 005E92E00FF08512008965B9 /* SampleFilter.cpp in Sources */, 0057785F0FF17CCC00582CD9 /* SampleMipMap.cpp in Sources */, 005778B40FF5616F00582CD9 /* SampleShapes.cpp in Sources */, 00F53F480FFCFC4D003FA70A /* SampleGradients.cpp in Sources */, 27005D16100903C100E275B6 /* SampleLines.cpp in Sources */, 27005D5F10095B2B00E275B6 /* SampleCircle.cpp in Sources */, + 00C1B809103857A400FA5948 /* SampleFillType.cpp in Sources */, + 00AF9B18103CD5EB00CBBCB3 /* SampleDitherBitmap.cpp in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; -- 2.34.1