From 8790be6de3644e332ec6a17c855da89ffc13a9bf Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Tue, 3 Dec 2013 16:26:51 -0500 Subject: [PATCH] Remove calls to deprecated SkBitmap::setIsOpaque() setIsOpaque() has been removed from ToT Skia. Update setters for mIsPremultiplied and hasAlpha to take the other into consideration. Change-Id: I1b36b0b0ce7126031eb7b769b563c17dcd4b306a --- core/jni/android/graphics/Bitmap.cpp | 14 +++++-- core/jni/android/graphics/BitmapFactory.cpp | 7 +++- core/jni/android/graphics/Graphics.cpp | 20 ++++++++++ core/jni/android/graphics/GraphicsJNI.h | 4 ++ core/jni/android_view_Surface.cpp | 6 ++- core/jni/android_view_SurfaceControl.cpp | 5 ++- core/jni/android_view_TextureView.cpp | 3 +- graphics/java/android/graphics/Bitmap.java | 40 +++++++++++++++---- .../java/android/graphics/BitmapFactory.java | 7 ++++ libs/hwui/TextureCache.cpp | 3 +- .../tests/omxjpegdecoder/omx_jpeg_decoder.cpp | 4 +- 11 files changed, 91 insertions(+), 22 deletions(-) diff --git a/core/jni/android/graphics/Bitmap.cpp b/core/jni/android/graphics/Bitmap.cpp index 094129b6e9742..2125763287643 100644 --- a/core/jni/android/graphics/Bitmap.cpp +++ b/core/jni/android/graphics/Bitmap.cpp @@ -442,9 +442,15 @@ static jboolean Bitmap_hasAlpha(JNIEnv* env, jobject, SkBitmap* bitmap) { return !bitmap->isOpaque(); } -static void Bitmap_setHasAlpha(JNIEnv* env, jobject, SkBitmap* bitmap, - jboolean hasAlpha) { - bitmap->setIsOpaque(!hasAlpha); +static void Bitmap_setAlphaAndPremultiplied(JNIEnv* env, jobject, SkBitmap* bitmap, + jboolean hasAlpha, jboolean isPremul) { + if (!hasAlpha) { + bitmap->setAlphaType(kOpaque_SkAlphaType); + } else if (isPremul) { + bitmap->setAlphaType(kPremul_SkAlphaType); + } else { + bitmap->setAlphaType(kUnpremul_SkAlphaType); + } } static jboolean Bitmap_hasMipMap(JNIEnv* env, jobject, SkBitmap* bitmap) { @@ -770,7 +776,7 @@ static JNINativeMethod gBitmapMethods[] = { { "nativeRowBytes", "(I)I", (void*)Bitmap_rowBytes }, { "nativeConfig", "(I)I", (void*)Bitmap_config }, { "nativeHasAlpha", "(I)Z", (void*)Bitmap_hasAlpha }, - { "nativeSetHasAlpha", "(IZ)V", (void*)Bitmap_setHasAlpha }, + { "nativeSetAlphaAndPremultiplied", "(IZZ)V", (void*)Bitmap_setAlphaAndPremultiplied}, { "nativeHasMipMap", "(I)Z", (void*)Bitmap_hasMipMap }, { "nativeSetHasMipMap", "(IZ)V", (void*)Bitmap_setHasMipMap }, { "nativeCreateFromParcel", diff --git a/core/jni/android/graphics/BitmapFactory.cpp b/core/jni/android/graphics/BitmapFactory.cpp index 944b732dfb823..efa8ce7a1e142 100644 --- a/core/jni/android/graphics/BitmapFactory.cpp +++ b/core/jni/android/graphics/BitmapFactory.cpp @@ -401,8 +401,11 @@ static jobject doDecode(JNIEnv* env, SkStreamRewindable* stream, jobject padding // TODO: avoid copying when scaled size equals decodingBitmap size SkBitmap::Config config = configForScaledOutput(decodingBitmap.config()); - outputBitmap->setConfig(config, scaledWidth, scaledHeight); - outputBitmap->setIsOpaque(decodingBitmap.isOpaque()); + // FIXME: If the alphaType is kUnpremul and the image has alpha, the + // colors may not be correct, since Skia does not yet support drawing + // to/from unpremultiplied bitmaps. + outputBitmap->setConfig(config, scaledWidth, scaledHeight, 0, + decodingBitmap.alphaType()); if (!outputBitmap->allocPixels(outputAllocator, NULL)) { return nullObjectReturn("allocation failed for scaled bitmap"); } diff --git a/core/jni/android/graphics/Graphics.cpp b/core/jni/android/graphics/Graphics.cpp index 38a9ba32781aa..c5c0eeec8d2d9 100644 --- a/core/jni/android/graphics/Graphics.cpp +++ b/core/jni/android/graphics/Graphics.cpp @@ -352,6 +352,18 @@ SkRegion* GraphicsJNI::getNativeRegion(JNIEnv* env, jobject region) /////////////////////////////////////////////////////////////////////////////////////////// +// Assert that bitmap's SkAlphaType is consistent with isPremultiplied. +static void assert_premultiplied(const SkBitmap& bitmap, bool isPremultiplied) { + // kOpaque_SkAlphaType and kIgnore_SkAlphaType mean that isPremultiplied is + // irrelevant. This just tests to ensure that the SkAlphaType is not + // opposite of isPremultiplied. + if (isPremultiplied) { + SkASSERT(bitmap.alphaType() != kUnpremul_SkAlphaType); + } else { + SkASSERT(bitmap.alphaType() != kPremul_SkAlphaType); + } +} + jobject GraphicsJNI::createBitmap(JNIEnv* env, SkBitmap* bitmap, jbyteArray buffer, int bitmapCreateFlags, jbyteArray ninepatch, jintArray layoutbounds, int density) { @@ -360,6 +372,10 @@ jobject GraphicsJNI::createBitmap(JNIEnv* env, SkBitmap* bitmap, jbyteArray buff bool isMutable = bitmapCreateFlags & kBitmapCreateFlag_Mutable; bool isPremultiplied = bitmapCreateFlags & kBitmapCreateFlag_Premultiplied; + // The caller needs to have already set the alpha type properly, so the + // native SkBitmap stays in sync with the Java Bitmap. + assert_premultiplied(*bitmap, isPremultiplied); + jobject obj = env->NewObject(gBitmap_class, gBitmap_constructorMethodID, static_cast(reinterpret_cast(bitmap)), buffer, bitmap->width(), bitmap->height(), density, isMutable, isPremultiplied, @@ -377,6 +393,10 @@ jobject GraphicsJNI::createBitmap(JNIEnv* env, SkBitmap* bitmap, int bitmapCreat void GraphicsJNI::reinitBitmap(JNIEnv* env, jobject javaBitmap, SkBitmap* bitmap, bool isPremultiplied) { + // The caller needs to have already set the alpha type properly, so the + // native SkBitmap stays in sync with the Java Bitmap. + assert_premultiplied(*bitmap, isPremultiplied); + env->CallVoidMethod(javaBitmap, gBitmap_reinitMethodID, bitmap->width(), bitmap->height(), isPremultiplied); } diff --git a/core/jni/android/graphics/GraphicsJNI.h b/core/jni/android/graphics/GraphicsJNI.h index f4590b956d78a..5f2960426382c 100644 --- a/core/jni/android/graphics/GraphicsJNI.h +++ b/core/jni/android/graphics/GraphicsJNI.h @@ -57,6 +57,7 @@ public: /** Create a java Bitmap object given the native bitmap (required) and optional storage array (may be null). + bitmap's SkAlphaType must already be in sync with bitmapCreateFlags. */ static jobject createBitmap(JNIEnv* env, SkBitmap* bitmap, jbyteArray buffer, int bitmapCreateFlags, jbyteArray ninepatch, jintArray layoutbounds, int density = -1); @@ -64,6 +65,9 @@ public: static jobject createBitmap(JNIEnv* env, SkBitmap* bitmap, int bitmapCreateFlags, jbyteArray ninepatch, int density = -1); + /** Reinitialize a bitmap. bitmap must already have its SkAlphaType set in + sync with isPremultiplied + */ static void reinitBitmap(JNIEnv* env, jobject javaBitmap, SkBitmap* bitmap, bool isPremultiplied); diff --git a/core/jni/android_view_Surface.cpp b/core/jni/android_view_Surface.cpp index 1868469f8ebb6..dd178d8297f34 100644 --- a/core/jni/android_view_Surface.cpp +++ b/core/jni/android_view_Surface.cpp @@ -39,6 +39,7 @@ #include #include +#include #include #include @@ -178,7 +179,8 @@ static jboolean nativeIsConsumerRunningBehind(JNIEnv* env, jclass clazz, jint na static inline SkBitmap::Config convertPixelFormat(PixelFormat format) { /* note: if PIXEL_FORMAT_RGBX_8888 means that all alpha bytes are 0xFF, then we can map to SkBitmap::kARGB_8888_Config, and optionally call - bitmap.setIsOpaque(true) on the resulting SkBitmap (as an accelerator) + bitmap.setAlphaType(kOpaque_SkAlphaType) on the resulting SkBitmap + (as an accelerator) */ switch (format) { case PIXEL_FORMAT_RGBX_8888: return SkBitmap::kARGB_8888_Config; @@ -234,7 +236,7 @@ static jint nativeLockCanvas(JNIEnv* env, jclass clazz, ssize_t bpr = outBuffer.stride * bytesPerPixel(outBuffer.format); bitmap.setConfig(convertPixelFormat(outBuffer.format), outBuffer.width, outBuffer.height, bpr); if (outBuffer.format == PIXEL_FORMAT_RGBX_8888) { - bitmap.setIsOpaque(true); + bitmap.setAlphaType(kOpaque_SkAlphaType); } if (outBuffer.width > 0 && outBuffer.height > 0) { bitmap.setPixels(outBuffer.bits); diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp index 67eade83cdc4b..12acfe19cd9a9 100644 --- a/core/jni/android_view_SurfaceControl.cpp +++ b/core/jni/android_view_SurfaceControl.cpp @@ -150,7 +150,8 @@ static void nativeDestroy(JNIEnv* env, jclass clazz, jint nativeObject) { static inline SkBitmap::Config convertPixelFormat(PixelFormat format) { /* note: if PIXEL_FORMAT_RGBX_8888 means that all alpha bytes are 0xFF, then we can map to SkBitmap::kARGB_8888_Config, and optionally call - bitmap.setIsOpaque(true) on the resulting SkBitmap (as an accelerator) + bitmap.setAlphaType(kOpaque_SkAlphaType) on the resulting SkBitmap + (as an accelerator) */ switch (format) { case PIXEL_FORMAT_RGBX_8888: return SkBitmap::kARGB_8888_Config; @@ -183,7 +184,7 @@ static jobject nativeScreenshotBitmap(JNIEnv* env, jclass clazz, jobject display SkBitmap* bitmap = new SkBitmap(); bitmap->setConfig(convertPixelFormat(f), w, h, bpr); if (f == PIXEL_FORMAT_RGBX_8888) { - bitmap->setIsOpaque(true); + bitmap->setAlphaType(kOpaque_SkAlphaType); } if (w > 0 && h > 0) { diff --git a/core/jni/android_view_TextureView.cpp b/core/jni/android_view_TextureView.cpp index 0f4290059b89b..7a4a20a97cab3 100644 --- a/core/jni/android_view_TextureView.cpp +++ b/core/jni/android_view_TextureView.cpp @@ -27,6 +27,7 @@ #include #include +#include namespace android { @@ -156,7 +157,7 @@ static jboolean android_view_TextureView_lockCanvas(JNIEnv* env, jobject, bitmap.setConfig(convertPixelFormat(buffer.format), buffer.width, buffer.height, bytesCount); if (buffer.format == WINDOW_FORMAT_RGBX_8888) { - bitmap.setIsOpaque(true); + bitmap.setAlphaType(kOpaque_SkAlphaType); } if (buffer.width > 0 && buffer.height > 0) { diff --git a/graphics/java/android/graphics/Bitmap.java b/graphics/java/android/graphics/Bitmap.java index 3c2468341efd3..4d82264639a86 100644 --- a/graphics/java/android/graphics/Bitmap.java +++ b/graphics/java/android/graphics/Bitmap.java @@ -67,6 +67,16 @@ public final class Bitmap implements Parcelable { * setPremultiplied() aren't order dependent, despite being setters. */ private boolean mIsPremultiplied; + + /** + * Whether the Bitmap's content is expected to have alpha. Note that hasAlpha() + * does not directly return this value, because hasAlpha() may never return true + * for a 565 Bitmap. + * + * Any time this or mIsPremultiplied is changed, both are passed to native so they + * are not order dependent. + */ + private boolean mHasAlpha; private byte[] mNinePatchChunk; // may be null private int[] mLayoutBounds; // may be null private int mWidth; @@ -554,7 +564,7 @@ public final class Bitmap implements Parcelable { checkRecycled("Can't copy a recycled bitmap"); Bitmap b = nativeCopy(mNativeBitmap, config.nativeInt, isMutable); if (b != null) { - b.mIsPremultiplied = mIsPremultiplied; + b.setAlphaAndPremultiplied(mHasAlpha, mIsPremultiplied); b.mDensity = mDensity; } return b; @@ -727,12 +737,12 @@ public final class Bitmap implements Parcelable { paint.setAntiAlias(true); } } - + // The new bitmap was created from a known bitmap source so assume that // they use the same density bitmap.mDensity = source.mDensity; - bitmap.mIsPremultiplied = source.mIsPremultiplied; - + bitmap.setAlphaAndPremultiplied(source.mHasAlpha, source.mIsPremultiplied); + canvas.setBitmap(bitmap); canvas.drawBitmap(source, srcR, dstR, paint); canvas.setBitmap(null); @@ -810,9 +820,9 @@ public final class Bitmap implements Parcelable { if (display != null) { bm.mDensity = display.densityDpi; } + bm.setHasAlpha(hasAlpha); if (config == Config.ARGB_8888 && !hasAlpha) { nativeErase(bm.mNativeBitmap, 0xff000000); - nativeSetHasAlpha(bm.mNativeBitmap, hasAlpha); } // No need to initialize the bitmap to zeroes with other configs; // it is backed by a VM byte array which is by definition preinitialized @@ -884,6 +894,7 @@ public final class Bitmap implements Parcelable { if (display != null) { bm.mDensity = display.densityDpi; } + bm.mHasAlpha = true; return bm; } @@ -1041,11 +1052,24 @@ public final class Bitmap implements Parcelable { *

This method will not affect the behavior of a bitmap without an alpha * channel, or if {@link #hasAlpha()} returns false.

* + *

Calling {@link createBitmap()} or {@link createScaledBitmap()} with a source + * Bitmap whose colors are not pre-multiplied may result in a RuntimeException, + * since those functions require drawing the source, which is not supported for + * un-pre-multiplied Bitmaps.

+ * * @see Bitmap#isPremultiplied() * @see BitmapFactory.Options#inPremultiplied */ public final void setPremultiplied(boolean premultiplied) { mIsPremultiplied = premultiplied; + nativeSetAlphaAndPremultiplied(mNativeBitmap, mHasAlpha, premultiplied); + } + + /** Helper function to set both alpha and premultiplied. **/ + private final void setAlphaAndPremultiplied(boolean hasAlpha, boolean premultiplied) { + mHasAlpha = hasAlpha; + mIsPremultiplied = premultiplied; + nativeSetAlphaAndPremultiplied(mNativeBitmap, hasAlpha, premultiplied); } /** Returns the bitmap's width */ @@ -1206,7 +1230,8 @@ public final class Bitmap implements Parcelable { * non-opaque per-pixel alpha values. */ public void setHasAlpha(boolean hasAlpha) { - nativeSetHasAlpha(mNativeBitmap, hasAlpha); + mHasAlpha = hasAlpha; + nativeSetAlphaAndPremultiplied(mNativeBitmap, hasAlpha, mIsPremultiplied); } /** @@ -1611,7 +1636,8 @@ public final class Bitmap implements Parcelable { private static native void nativePrepareToDraw(int nativeBitmap); private static native boolean nativeHasAlpha(int nativeBitmap); - private static native void nativeSetHasAlpha(int nBitmap, boolean hasAlpha); + private static native void nativeSetAlphaAndPremultiplied(int nBitmap, boolean hasAlpha, + boolean isPremul); private static native boolean nativeHasMipMap(int nativeBitmap); private static native void nativeSetHasMipMap(int nBitmap, boolean hasMipMap); private static native boolean nativeSameAs(int nb0, int nb1); diff --git a/graphics/java/android/graphics/BitmapFactory.java b/graphics/java/android/graphics/BitmapFactory.java index b714fab6febc1..2b69a155284f3 100644 --- a/graphics/java/android/graphics/BitmapFactory.java +++ b/graphics/java/android/graphics/BitmapFactory.java @@ -153,8 +153,12 @@ public class BitmapFactory { * *

This does not affect bitmaps without an alpha channel.

* + *

Setting this flag to false while setting {@link #inScaled} to true + * may result in incorrect colors.

+ * * @see Bitmap#hasAlpha() * @see Bitmap#isPremultiplied() + * @see #inScaled */ public boolean inPremultiplied; @@ -249,6 +253,9 @@ public class BitmapFactory { *

This flag is turned on by default and should be turned off if you need * a non-scaled version of the bitmap. Nine-patch bitmaps ignore this * flag and are always scaled. + * + *

If {@link #inPremultiplied} is set to false, and the image has alpha, + * setting this flag to true may result in incorrect colors. */ public boolean inScaled; diff --git a/libs/hwui/TextureCache.cpp b/libs/hwui/TextureCache.cpp index 457ca59900ebd..467f6ca94a065 100644 --- a/libs/hwui/TextureCache.cpp +++ b/libs/hwui/TextureCache.cpp @@ -285,10 +285,9 @@ void TextureCache::generateTexture(const SkBitmap* bitmap, Texture* texture, boo void TextureCache::uploadLoFiTexture(bool resize, const SkBitmap* bitmap, uint32_t width, uint32_t height) { SkBitmap rgbaBitmap; - rgbaBitmap.setConfig(SkBitmap::kARGB_8888_Config, width, height); + rgbaBitmap.setConfig(SkBitmap::kARGB_8888_Config, width, height, 0, bitmap->alphaType()); rgbaBitmap.allocPixels(); rgbaBitmap.eraseColor(0); - rgbaBitmap.setIsOpaque(bitmap->isOpaque()); SkCanvas canvas(rgbaBitmap); canvas.drawBitmap(*bitmap, 0.0f, 0.0f, NULL); diff --git a/media/tests/omxjpegdecoder/omx_jpeg_decoder.cpp b/media/tests/omxjpegdecoder/omx_jpeg_decoder.cpp index 6424744a43316..53f04bc6079c9 100644 --- a/media/tests/omxjpegdecoder/omx_jpeg_decoder.cpp +++ b/media/tests/omxjpegdecoder/omx_jpeg_decoder.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include "omx_jpeg_decoder.h" @@ -184,8 +185,7 @@ void OmxJpegImageDecoder::installPixelRef(MediaBuffer *buffer, sp d void OmxJpegImageDecoder::configBitmapSize(SkBitmap* bm, SkBitmap::Config pref, int width, int height) { - bm->setConfig(getColorSpaceConfig(pref), width, height); - bm->setIsOpaque(true); + bm->setConfig(getColorSpaceConfig(pref), width, height, 0, kOpaque_SkAlphaType); } SkBitmap::Config OmxJpegImageDecoder::getColorSpaceConfig(