From bf3e464704b27ee6245a0ef985c2777766e1aac8 Mon Sep 17 00:00:00 2001 From: Derek Sollenberger Date: Wed, 30 Jan 2019 11:28:27 -0500 Subject: [PATCH] Refine ColorSpace restrictions for Bitmaps Bug: 123377741 Bug: 120870651 Bug: 121001670 Bug: 123374456 Bug: 124052364 Test: I90adb511c8fdefe016028da4fd53b079d8367bf6 RGBA_F16 is no longer restricted to particular ColorSpaces. (Previously, it was LINEAR_EXTENDED_SRGB when decoded, and EXTENDED_SRGB when created.) Instead, F16 is now EXTENDED if there is an EXTENDED variant, but is otherwise not special. It defaults to SRGB (but EXTENDED), but can be created or decoded to other ColorSpaces. Likewise, smaller Configs use the non-EXTENDED variant, when the EXTENDED variant was requested. ALPHA_8 always has a null ColorSpace. Add TransferParameters to EXTENDED_SRGB. This seems to be a relic from a time before Skia treated SkColorSpaces as non-bounded. Make it have the same parameters as SRGB, so that it can be used in native code. e.g. now we can draw to/from it. Fix a bug getting the ColorSpace for gray image. ImageDecoder's info previously reported that a gray image's ColorSpace is null. (Though it correctly decoded it with the proper ColorSpace.) Allow setColorSpace to request SRGB on an EXTENDED_SRGB F16 Bitmap. (It has no visible effect.) Do *not* allow setting a ColorSpace on an ALPHA_8 Bitmap (throw an Exception). Other attempts to create a Bitmap with ALPHA_8 and a non-null ColorSpace silently use null, for backwards compatibility. Copying *from* an ALPHA_8 to another Config returns a Bitmap with SRGB. Change-Id: Ied0426f6deff354df5998691703a18ddd33ccd3d --- core/jni/android/graphics/Bitmap.cpp | 83 +++------- core/jni/android/graphics/BitmapFactory.cpp | 2 +- .../android/graphics/BitmapRegionDecoder.cpp | 2 +- core/jni/android/graphics/Graphics.cpp | 135 +++++++++-------- core/jni/android/graphics/GraphicsJNI.h | 8 +- core/jni/android/graphics/ImageDecoder.cpp | 4 +- graphics/java/android/graphics/Bitmap.java | 142 ++++++++---------- .../java/android/graphics/BitmapFactory.java | 9 +- .../java/android/graphics/ColorSpace.java | 2 +- .../java/android/graphics/ImageDecoder.java | 9 +- 10 files changed, 177 insertions(+), 219 deletions(-) diff --git a/core/jni/android/graphics/Bitmap.cpp b/core/jni/android/graphics/Bitmap.cpp index e8172172f5d29..8f007594dd674 100755 --- a/core/jni/android/graphics/Bitmap.cpp +++ b/core/jni/android/graphics/Bitmap.cpp @@ -355,9 +355,16 @@ static jobject Bitmap_creator(JNIEnv* env, jobject, jintArray jColors, colorType = kN32_SkColorType; } + sk_sp colorSpace; + if (colorType == kAlpha_8_SkColorType) { + colorSpace = nullptr; + } else { + colorSpace = GraphicsJNI::getNativeColorSpace(colorSpacePtr); + } + SkBitmap bitmap; bitmap.setInfo(SkImageInfo::Make(width, height, colorType, kPremul_SkAlphaType, - GraphicsJNI::getNativeColorSpace(colorSpacePtr))); + colorSpace)); sk_sp nativeBitmap = Bitmap::allocateHeapBitmap(&bitmap); if (!nativeBitmap) { @@ -385,15 +392,17 @@ static bool bitmapCopyTo(SkBitmap* dst, SkColorType dstCT, const SkBitmap& src, case kRGB_565_SkColorType: dstInfo = dstInfo.makeAlphaType(kOpaque_SkAlphaType); break; - case kRGBA_F16_SkColorType: - // The caller does not have an opportunity to pass a dst color space. Assume that - // they want linear sRGB. - dstInfo = dstInfo.makeColorSpace(SkColorSpace::MakeSRGBLinear()); + case kAlpha_8_SkColorType: + dstInfo = dstInfo.makeColorSpace(nullptr); break; default: break; } + if (!dstInfo.colorSpace() && dstCT != kAlpha_8_SkColorType) { + dstInfo = dstInfo.makeColorSpace(SkColorSpace::MakeSRGB()); + } + if (!dst->setInfo(dstInfo)) { return false; } @@ -608,14 +617,6 @@ static jint Bitmap_getGenerationId(JNIEnv* env, jobject, jlong bitmapHandle) { return static_cast(bitmap->getGenerationID()); } -static jboolean Bitmap_isConfigF16(JNIEnv* env, jobject, jlong bitmapHandle) { - LocalScopedBitmap bitmap(bitmapHandle); - if (bitmap->info().colorType() == kRGBA_F16_SkColorType) { - return JNI_TRUE; - } - return JNI_FALSE; -} - static jboolean Bitmap_isPremultiplied(JNIEnv* env, jobject, jlong bitmapHandle) { LocalScopedBitmap bitmap(bitmapHandle); if (bitmap->info().alphaType() == kPremul_SkAlphaType) { @@ -684,9 +685,7 @@ static jobject Bitmap_createFromParcel(JNIEnv* env, jobject, jobject parcel) { const SkAlphaType alphaType = (SkAlphaType)p->readInt32(); const uint32_t colorSpaceSize = p->readUint32(); sk_sp colorSpace; - if (kRGBA_F16_SkColorType == colorType) { - colorSpace = SkColorSpace::MakeSRGBLinear(); - } else if (colorSpaceSize > 0) { + if (colorSpaceSize > 0) { if (colorSpaceSize > kMaxColorSpaceSerializedBytes) { ALOGD("Bitmap_createFromParcel: Serialized SkColorSpace is larger than expected: " "%d bytes\n", colorSpaceSize); @@ -811,7 +810,7 @@ static jboolean Bitmap_writeToParcel(JNIEnv* env, jobject, p->writeInt32(bitmap.colorType()); p->writeInt32(bitmap.alphaType()); SkColorSpace* colorSpace = bitmap.colorSpace(); - if (colorSpace != nullptr && bitmap.colorType() != kRGBA_F16_SkColorType) { + if (colorSpace != nullptr) { sk_sp data = colorSpace->serialize(); size_t size = data->size(); p->writeUint32(size); @@ -924,44 +923,14 @@ static jboolean Bitmap_isSRGBLinear(JNIEnv* env, jobject, jlong bitmapHandle) { return colorSpace == srgbLinear.get() ? JNI_TRUE : JNI_FALSE; } -static jboolean Bitmap_getColorSpace(JNIEnv* env, jobject, jlong bitmapHandle, - jfloatArray xyzArray, jfloatArray paramsArray) { - +static jobject Bitmap_computeColorSpace(JNIEnv* env, jobject, jlong bitmapHandle) { LocalScopedBitmap bitmapHolder(bitmapHandle); - if (!bitmapHolder.valid()) return JNI_FALSE; + if (!bitmapHolder.valid()) return nullptr; SkColorSpace* colorSpace = bitmapHolder->info().colorSpace(); - if (colorSpace == nullptr) return JNI_FALSE; + if (colorSpace == nullptr) return nullptr; - skcms_Matrix3x3 xyzMatrix; - if (!colorSpace->toXYZD50(&xyzMatrix)) return JNI_FALSE; - - jfloat* xyz = env->GetFloatArrayElements(xyzArray, NULL); - xyz[0] = xyzMatrix.vals[0][0]; - xyz[1] = xyzMatrix.vals[1][0]; - xyz[2] = xyzMatrix.vals[2][0]; - xyz[3] = xyzMatrix.vals[0][1]; - xyz[4] = xyzMatrix.vals[1][1]; - xyz[5] = xyzMatrix.vals[2][1]; - xyz[6] = xyzMatrix.vals[0][2]; - xyz[7] = xyzMatrix.vals[1][2]; - xyz[8] = xyzMatrix.vals[2][2]; - env->ReleaseFloatArrayElements(xyzArray, xyz, 0); - - skcms_TransferFunction transferParams; - if (!colorSpace->isNumericalTransferFn(&transferParams)) return JNI_FALSE; - - jfloat* params = env->GetFloatArrayElements(paramsArray, NULL); - params[0] = transferParams.a; - params[1] = transferParams.b; - params[2] = transferParams.c; - params[3] = transferParams.d; - params[4] = transferParams.e; - params[5] = transferParams.f; - params[6] = transferParams.g; - env->ReleaseFloatArrayElements(paramsArray, params, 0); - - return JNI_TRUE; + return GraphicsJNI::getColorSpace(env, colorSpace, bitmapHolder->info().colorType()); } static void Bitmap_setColorSpace(JNIEnv* env, jobject, jlong bitmapHandle, jlong colorSpacePtr) { @@ -1174,13 +1143,6 @@ static jobject Bitmap_createGraphicBufferHandle(JNIEnv* env, jobject, jlong bitm return createJavaGraphicBuffer(env, buffer); } -static void Bitmap_copyColorSpace(JNIEnv* env, jobject, jlong srcBitmapPtr, jlong dstBitmapPtr) { - LocalScopedBitmap srcBitmapHandle(srcBitmapPtr); - LocalScopedBitmap dstBitmapHandle(dstBitmapPtr); - - dstBitmapHandle->bitmap().setColorSpace(srcBitmapHandle->bitmap().info().refColorSpace()); -} - static jboolean Bitmap_isImmutable(jlong bitmapHandle) { LocalScopedBitmap bitmapHolder(bitmapHandle); if (!bitmapHolder.valid()) return JNI_FALSE; @@ -1215,7 +1177,6 @@ static const JNINativeMethod gBitmapMethods[] = { { "nativeErase", "(JJJ)V", (void*)Bitmap_eraseLong }, { "nativeRowBytes", "(J)I", (void*)Bitmap_rowBytes }, { "nativeConfig", "(J)I", (void*)Bitmap_config }, - { "nativeIsConfigF16", "(J)Z", (void*)Bitmap_isConfigF16 }, { "nativeHasAlpha", "(J)Z", (void*)Bitmap_hasAlpha }, { "nativeIsPremultiplied", "(J)Z", (void*)Bitmap_isPremultiplied}, { "nativeSetHasAlpha", "(JZZ)V", (void*)Bitmap_setHasAlpha}, @@ -1248,12 +1209,10 @@ static const JNINativeMethod gBitmapMethods[] = { (void*) Bitmap_wrapHardwareBufferBitmap }, { "nativeCreateGraphicBufferHandle", "(J)Landroid/graphics/GraphicBuffer;", (void*) Bitmap_createGraphicBufferHandle }, - { "nativeGetColorSpace", "(J[F[F)Z", (void*)Bitmap_getColorSpace }, + { "nativeComputeColorSpace", "(J)Landroid/graphics/ColorSpace;", (void*)Bitmap_computeColorSpace }, { "nativeSetColorSpace", "(JJ)V", (void*)Bitmap_setColorSpace }, { "nativeIsSRGB", "(J)Z", (void*)Bitmap_isSRGB }, { "nativeIsSRGBLinear", "(J)Z", (void*)Bitmap_isSRGBLinear}, - { "nativeCopyColorSpace", "(JJ)V", - (void*)Bitmap_copyColorSpace }, { "nativeSetImmutable", "(J)V", (void*)Bitmap_setImmutable}, // ------------ @CriticalNative ---------------- diff --git a/core/jni/android/graphics/BitmapFactory.cpp b/core/jni/android/graphics/BitmapFactory.cpp index 70e6604fddeb4..4ba4540f7dbc9 100644 --- a/core/jni/android/graphics/BitmapFactory.cpp +++ b/core/jni/android/graphics/BitmapFactory.cpp @@ -307,7 +307,7 @@ static jobject doDecode(JNIEnv* env, std::unique_ptr stream, env->SetObjectField(options, gOptions_outConfigFieldID, config); env->SetObjectField(options, gOptions_outColorSpaceFieldID, - GraphicsJNI::getColorSpace(env, decodeColorSpace, decodeColorType)); + GraphicsJNI::getColorSpace(env, decodeColorSpace.get(), decodeColorType)); if (onlyDecodeSize) { return nullptr; diff --git a/core/jni/android/graphics/BitmapRegionDecoder.cpp b/core/jni/android/graphics/BitmapRegionDecoder.cpp index d65f324d10658..9c07e2d64c6ef 100644 --- a/core/jni/android/graphics/BitmapRegionDecoder.cpp +++ b/core/jni/android/graphics/BitmapRegionDecoder.cpp @@ -215,7 +215,7 @@ static jobject nativeDecodeRegion(JNIEnv* env, jobject, jlong brdHandle, jint in env->SetObjectField(options, gOptions_outConfigFieldID, config); env->SetObjectField(options, gOptions_outColorSpaceFieldID, - GraphicsJNI::getColorSpace(env, decodeColorSpace, decodeColorType)); + GraphicsJNI::getColorSpace(env, decodeColorSpace.get(), decodeColorType)); } // If we may have reused a bitmap, we need to indicate that the pixels have changed. diff --git a/core/jni/android/graphics/Graphics.cpp b/core/jni/android/graphics/Graphics.cpp index 6570992b4b23c..2987c5ed56b5a 100644 --- a/core/jni/android/graphics/Graphics.cpp +++ b/core/jni/android/graphics/Graphics.cpp @@ -187,6 +187,8 @@ static jmethodID gColorSpaceRGB_constructorMethodID; static jclass gColorSpace_Named_class; static jfieldID gColorSpace_Named_sRGBFieldID; +static jfieldID gColorSpace_Named_ExtendedSRGBFieldID; +static jfieldID gColorSpace_Named_LinearSRGBFieldID; static jfieldID gColorSpace_Named_LinearExtendedSRGBFieldID; static jclass gTransferParameters_class; @@ -412,67 +414,78 @@ jobject GraphicsJNI::createRegion(JNIEnv* env, SkRegion* region) /////////////////////////////////////////////////////////////////////////////// -jobject GraphicsJNI::getColorSpace(JNIEnv* env, sk_sp& decodeColorSpace, +jobject GraphicsJNI::getColorSpace(JNIEnv* env, SkColorSpace* decodeColorSpace, SkColorType decodeColorType) { - jobject colorSpace = nullptr; - - // No need to match, we know what the output color space will be - if (decodeColorType == kRGBA_F16_SkColorType) { - jobject linearExtendedSRGB = env->GetStaticObjectField( - gColorSpace_Named_class, gColorSpace_Named_LinearExtendedSRGBFieldID); - colorSpace = env->CallStaticObjectMethod(gColorSpace_class, - gColorSpace_getMethodID, linearExtendedSRGB); - } else { - // Same here, no need to match - if (decodeColorSpace->isSRGB()) { - jobject sRGB = env->GetStaticObjectField( - gColorSpace_Named_class, gColorSpace_Named_sRGBFieldID); - colorSpace = env->CallStaticObjectMethod(gColorSpace_class, - gColorSpace_getMethodID, sRGB); - } else if (decodeColorSpace.get() != nullptr) { - // Try to match against known RGB color spaces using the CIE XYZ D50 - // conversion matrix and numerical transfer function parameters - skcms_Matrix3x3 xyzMatrix; - LOG_ALWAYS_FATAL_IF(!decodeColorSpace->toXYZD50(&xyzMatrix)); - - skcms_TransferFunction transferParams; - // We can only handle numerical transfer functions at the moment - LOG_ALWAYS_FATAL_IF(!decodeColorSpace->isNumericalTransferFn(&transferParams)); - - jobject params = env->NewObject(gTransferParameters_class, - gTransferParameters_constructorMethodID, - transferParams.a, transferParams.b, transferParams.c, - transferParams.d, transferParams.e, transferParams.f, - transferParams.g); - - jfloatArray xyzArray = env->NewFloatArray(9); - jfloat xyz[9] = { - xyzMatrix.vals[0][0], - xyzMatrix.vals[1][0], - xyzMatrix.vals[2][0], - xyzMatrix.vals[0][1], - xyzMatrix.vals[1][1], - xyzMatrix.vals[2][1], - xyzMatrix.vals[0][2], - xyzMatrix.vals[1][2], - xyzMatrix.vals[2][2] - }; - env->SetFloatArrayRegion(xyzArray, 0, 9, xyz); - - colorSpace = env->CallStaticObjectMethod(gColorSpace_class, - gColorSpace_matchMethodID, xyzArray, params); - - if (colorSpace == nullptr) { - // We couldn't find an exact match, let's create a new color space - // instance with the 3x3 conversion matrix and transfer function - colorSpace = env->NewObject(gColorSpaceRGB_class, - gColorSpaceRGB_constructorMethodID, - env->NewStringUTF("Unknown"), xyzArray, params); - } - - env->DeleteLocalRef(xyzArray); - } + if (!decodeColorSpace || decodeColorType == kAlpha_8_SkColorType) { + return nullptr; } + + // Special checks for the common sRGB cases and their extended variants. + jobject namedCS = nullptr; + sk_sp srgbLinear = SkColorSpace::MakeSRGBLinear(); + if (decodeColorType == kRGBA_F16_SkColorType) { + // An F16 Bitmap will always report that it is EXTENDED if + // it matches a ColorSpace that has an EXTENDED variant. + if (decodeColorSpace->isSRGB()) { + namedCS = env->GetStaticObjectField(gColorSpace_Named_class, + gColorSpace_Named_ExtendedSRGBFieldID); + } else if (decodeColorSpace == srgbLinear.get()) { + namedCS = env->GetStaticObjectField(gColorSpace_Named_class, + gColorSpace_Named_LinearExtendedSRGBFieldID); + } + } else if (decodeColorSpace->isSRGB()) { + namedCS = env->GetStaticObjectField(gColorSpace_Named_class, + gColorSpace_Named_sRGBFieldID); + } else if (decodeColorSpace == srgbLinear.get()) { + namedCS = env->GetStaticObjectField(gColorSpace_Named_class, + gColorSpace_Named_LinearSRGBFieldID); + } + + if (namedCS) { + return env->CallStaticObjectMethod(gColorSpace_class, gColorSpace_getMethodID, namedCS); + } + + // Try to match against known RGB color spaces using the CIE XYZ D50 + // conversion matrix and numerical transfer function parameters + skcms_Matrix3x3 xyzMatrix; + LOG_ALWAYS_FATAL_IF(!decodeColorSpace->toXYZD50(&xyzMatrix)); + + skcms_TransferFunction transferParams; + // We can only handle numerical transfer functions at the moment + LOG_ALWAYS_FATAL_IF(!decodeColorSpace->isNumericalTransferFn(&transferParams)); + + jobject params = env->NewObject(gTransferParameters_class, + gTransferParameters_constructorMethodID, + transferParams.a, transferParams.b, transferParams.c, + transferParams.d, transferParams.e, transferParams.f, + transferParams.g); + + jfloatArray xyzArray = env->NewFloatArray(9); + jfloat xyz[9] = { + xyzMatrix.vals[0][0], + xyzMatrix.vals[1][0], + xyzMatrix.vals[2][0], + xyzMatrix.vals[0][1], + xyzMatrix.vals[1][1], + xyzMatrix.vals[2][1], + xyzMatrix.vals[0][2], + xyzMatrix.vals[1][2], + xyzMatrix.vals[2][2] + }; + env->SetFloatArrayRegion(xyzArray, 0, 9, xyz); + + jobject colorSpace = env->CallStaticObjectMethod(gColorSpace_class, + gColorSpace_matchMethodID, xyzArray, params); + + if (colorSpace == nullptr) { + // We couldn't find an exact match, let's create a new color space + // instance with the 3x3 conversion matrix and transfer function + colorSpace = env->NewObject(gColorSpaceRGB_class, + gColorSpaceRGB_constructorMethodID, + env->NewStringUTF("Unknown"), xyzArray, params); + } + + env->DeleteLocalRef(xyzArray); return colorSpace; } @@ -658,6 +671,10 @@ int register_android_graphics_Graphics(JNIEnv* env) FindClassOrDie(env, "android/graphics/ColorSpace$Named")); gColorSpace_Named_sRGBFieldID = GetStaticFieldIDOrDie(env, gColorSpace_Named_class, "SRGB", "Landroid/graphics/ColorSpace$Named;"); + gColorSpace_Named_ExtendedSRGBFieldID = GetStaticFieldIDOrDie(env, + gColorSpace_Named_class, "EXTENDED_SRGB", "Landroid/graphics/ColorSpace$Named;"); + gColorSpace_Named_LinearSRGBFieldID = GetStaticFieldIDOrDie(env, + gColorSpace_Named_class, "LINEAR_SRGB", "Landroid/graphics/ColorSpace$Named;"); gColorSpace_Named_LinearExtendedSRGBFieldID = GetStaticFieldIDOrDie(env, gColorSpace_Named_class, "LINEAR_EXTENDED_SRGB", "Landroid/graphics/ColorSpace$Named;"); diff --git a/core/jni/android/graphics/GraphicsJNI.h b/core/jni/android/graphics/GraphicsJNI.h index dc0d022d94c05..f80651c30d64a 100644 --- a/core/jni/android/graphics/GraphicsJNI.h +++ b/core/jni/android/graphics/GraphicsJNI.h @@ -109,7 +109,13 @@ public: */ static sk_sp getNativeColorSpace(jlong colorSpaceHandle); - static jobject getColorSpace(JNIEnv* env, sk_sp& decodeColorSpace, + /** + * Return the android.graphics.ColorSpace Java object that corresponds to decodeColorSpace + * and decodeColorType. + * + * This may create a new object if none of the Named ColorSpaces match. + */ + static jobject getColorSpace(JNIEnv* env, SkColorSpace* decodeColorSpace, SkColorType decodeColorType); /** diff --git a/core/jni/android/graphics/ImageDecoder.cpp b/core/jni/android/graphics/ImageDecoder.cpp index 2d83ac3207332..9efcace06be3f 100644 --- a/core/jni/android/graphics/ImageDecoder.cpp +++ b/core/jni/android/graphics/ImageDecoder.cpp @@ -506,9 +506,9 @@ static jstring ImageDecoder_nGetMimeType(JNIEnv* env, jobject /*clazz*/, jlong n static jobject ImageDecoder_nGetColorSpace(JNIEnv* env, jobject /*clazz*/, jlong nativePtr) { auto* codec = reinterpret_cast(nativePtr)->mCodec.get(); - auto colorType = codec->computeOutputColorType(codec->getInfo().colorType()); + auto colorType = codec->computeOutputColorType(kN32_SkColorType); sk_sp colorSpace = codec->computeOutputColorSpace(colorType); - return GraphicsJNI::getColorSpace(env, colorSpace, colorType); + return GraphicsJNI::getColorSpace(env, colorSpace.get(), colorType); } static const JNINativeMethod gImageDecoderMethods[] = { diff --git a/graphics/java/android/graphics/Bitmap.java b/graphics/java/android/graphics/Bitmap.java index 18f0cae4733ce..bdb63643f6155 100644 --- a/graphics/java/android/graphics/Bitmap.java +++ b/graphics/java/android/graphics/Bitmap.java @@ -891,8 +891,10 @@ public final class Bitmap implements Parcelable { } } + ColorSpace cs = source.getColorSpace(); + if (m == null || m.isIdentity()) { - bitmap = createBitmap(neww, newh, newConfig, source.hasAlpha()); + bitmap = createBitmap(null, neww, newh, newConfig, source.hasAlpha(), cs); paint = null; // not needed } else { final boolean transformed = !m.rectStaysRect(); @@ -906,9 +908,14 @@ public final class Bitmap implements Parcelable { if (transformed) { if (transformedConfig != Config.ARGB_8888 && transformedConfig != Config.RGBA_F16) { transformedConfig = Config.ARGB_8888; + if (cs == null) { + cs = ColorSpace.get(ColorSpace.Named.SRGB); + } } } - bitmap = createBitmap(neww, newh, transformedConfig, transformed || source.hasAlpha()); + + bitmap = createBitmap(null, neww, newh, transformedConfig, + transformed || source.hasAlpha(), cs); paint = new Paint(); paint.setFilterBitmap(filter); @@ -917,8 +924,6 @@ public final class Bitmap implements Parcelable { } } - nativeCopyColorSpace(source.mNativePtr, bitmap.mNativePtr); - // The new bitmap was created from a known bitmap source so assume that // they use the same density bitmap.mDensity = source.mDensity; @@ -1000,10 +1005,10 @@ public final class Bitmap implements Parcelable { * @param hasAlpha If the bitmap is ARGB_8888 or RGBA_16F this flag can be used to * mark the bitmap as opaque. Doing so will clear the bitmap in black * instead of transparent. - * @param colorSpace The color space of the bitmap. If the config is {@link Config#RGBA_F16}, - * {@link ColorSpace.Named#EXTENDED_SRGB scRGB} is assumed, and if the - * config is not {@link Config#ARGB_8888}, {@link ColorSpace.Named#SRGB sRGB} - * is assumed. + * @param colorSpace The color space of the bitmap. If the config is {@link Config#RGBA_F16} + * and {@link ColorSpace.Named#SRGB sRGB} or + * {@link ColorSpace.Named#LINEAR_SRGB Linear sRGB} is provided then the + * corresponding extended range variant is assumed. * * @throws IllegalArgumentException if the width or height are <= 0, if * Config is Config.HARDWARE (because hardware bitmaps are always @@ -1055,10 +1060,10 @@ public final class Bitmap implements Parcelable { * @param hasAlpha If the bitmap is ARGB_8888 or RGBA_16F this flag can be used to * mark the bitmap as opaque. Doing so will clear the bitmap in black * instead of transparent. - * @param colorSpace The color space of the bitmap. If the config is {@link Config#RGBA_F16}, - * {@link ColorSpace.Named#EXTENDED_SRGB scRGB} is assumed, and if the - * config is not {@link Config#ARGB_8888}, {@link ColorSpace.Named#SRGB sRGB} - * is assumed. + * @param colorSpace The color space of the bitmap. If the config is {@link Config#RGBA_F16} + * and {@link ColorSpace.Named#SRGB sRGB} or + * {@link ColorSpace.Named#LINEAR_SRGB Linear sRGB} is provided then the + * corresponding extended range variant is assumed. * * @throws IllegalArgumentException if the width or height are <= 0, if * Config is Config.HARDWARE (because hardware bitmaps are always @@ -1075,22 +1080,12 @@ public final class Bitmap implements Parcelable { if (config == Config.HARDWARE) { throw new IllegalArgumentException("can't create mutable bitmap with Config.HARDWARE"); } - if (colorSpace == null) { + if (colorSpace == null && config != Config.ALPHA_8) { throw new IllegalArgumentException("can't create bitmap without a color space"); } - if (config != Config.ARGB_8888) { - if (config == Config.RGBA_F16) { - // FIXME: This should be LINEAR_EXTENDED_SRGB, but that would fail a CTS test. See - // b/120960866. SRGB matches the old (incorrect) behavior. - //colorSpace = ColorSpace.get(ColorSpace.Named.LINEAR_EXTENDED_SRGB); - colorSpace = ColorSpace.get(ColorSpace.Named.SRGB); - } else { - colorSpace = ColorSpace.get(ColorSpace.Named.SRGB); - } - } Bitmap bm = nativeCreate(null, 0, width, width, height, config.nativeInt, true, - colorSpace.getNativeInstance()); + colorSpace == null ? 0 : colorSpace.getNativeInstance()); if (display != null) { bm.mDensity = display.densityDpi; @@ -1701,41 +1696,9 @@ public final class Bitmap implements Parcelable { @Nullable public final ColorSpace getColorSpace() { checkRecycled("getColorSpace called on a recycled bitmap"); - // Cache the color space retrieval since it can be fairly expensive if (mColorSpace == null) { - if (nativeIsConfigF16(mNativePtr)) { - // an F16 bitmaps is intended to always be linear extended, but due to - // inconsistencies in Bitmap.create() functions it is possible to have - // rendered into a bitmap in non-linear sRGB. - if (nativeIsSRGB(mNativePtr)) { - mColorSpace = ColorSpace.get(ColorSpace.Named.EXTENDED_SRGB); - } else { - mColorSpace = ColorSpace.get(ColorSpace.Named.LINEAR_EXTENDED_SRGB); - } - } else if (nativeIsSRGB(mNativePtr)) { - mColorSpace = ColorSpace.get(ColorSpace.Named.SRGB); - } else if (nativeIsSRGBLinear(mNativePtr)) { - mColorSpace = ColorSpace.get(ColorSpace.Named.LINEAR_SRGB); - } else { - float[] xyz = new float[9]; - float[] params = new float[7]; - - boolean hasColorSpace = nativeGetColorSpace(mNativePtr, xyz, params); - if (hasColorSpace) { - ColorSpace.Rgb.TransferParameters parameters = - new ColorSpace.Rgb.TransferParameters( - params[0], params[1], params[2], - params[3], params[4], params[5], params[6]); - ColorSpace cs = ColorSpace.match(xyz, parameters); - if (cs != null) { - mColorSpace = cs; - } else { - mColorSpace = new ColorSpace.Rgb("Unknown", xyz, parameters); - } - } - } + mColorSpace = nativeComputeColorSpace(mNativePtr); } - return mColorSpace; } @@ -1749,6 +1712,9 @@ public final class Bitmap implements Parcelable { * components min/max values reduce the numerical range compared to the * previously assigned color space. * + * @throws IllegalArgumentException If the {@code Config} (returned by {@link #getConfig()}) + * is {@link Config#ALPHA_8}. + * * @param colorSpace to assign to the bitmap */ public void setColorSpace(@NonNull ColorSpace colorSpace) { @@ -1756,29 +1722,47 @@ public final class Bitmap implements Parcelable { if (colorSpace == null) { throw new IllegalArgumentException("The colorSpace cannot be set to null"); } - if (getColorSpace() != null) { - if (mColorSpace.getComponentCount() != colorSpace.getComponentCount()) { - throw new IllegalArgumentException("The new ColorSpace must have the same " - + "component count as the current ColorSpace"); - } - for (int i = 0; i < mColorSpace.getComponentCount(); i++) { - if (mColorSpace.getMinValue(i) < colorSpace.getMinValue(i)) { - throw new IllegalArgumentException("The new ColorSpace cannot increase the " - + "minimum value for any of the components compared to the current " - + "ColorSpace. To perform this type of conversion create a new Bitmap " - + "in the desired ColorSpace and draw this Bitmap into it."); - } - if (mColorSpace.getMaxValue(i) > colorSpace.getMaxValue(i)) { - throw new IllegalArgumentException("The new ColorSpace cannot decrease the " - + "maximum value for any of the components compared to the current " - + "ColorSpace/ To perform this type of conversion create a new Bitmap" - + "in the desired ColorSpace and draw this Bitmap into it."); - } - } + + if (getConfig() == Config.ALPHA_8) { + throw new IllegalArgumentException("Cannot set a ColorSpace on ALPHA_8"); } + // Keep track of the old ColorSpace for comparison, and so we can reset it in case of an + // Exception. + final ColorSpace oldColorSpace = getColorSpace(); nativeSetColorSpace(mNativePtr, colorSpace.getNativeInstance()); - mColorSpace = colorSpace; + + // This will update mColorSpace. It may not be the same as |colorSpace|, e.g. if we + // corrected it because the Bitmap is F16. + mColorSpace = null; + final ColorSpace newColorSpace = getColorSpace(); + + try { + if (oldColorSpace.getComponentCount() != newColorSpace.getComponentCount()) { + throw new IllegalArgumentException("The new ColorSpace must have the same " + + "component count as the current ColorSpace"); + } else { + for (int i = 0; i < oldColorSpace.getComponentCount(); i++) { + if (oldColorSpace.getMinValue(i) < newColorSpace.getMinValue(i)) { + throw new IllegalArgumentException("The new ColorSpace cannot increase the " + + "minimum value for any of the components compared to the current " + + "ColorSpace. To perform this type of conversion create a new " + + "Bitmap in the desired ColorSpace and draw this Bitmap into it."); + } + if (oldColorSpace.getMaxValue(i) > newColorSpace.getMaxValue(i)) { + throw new IllegalArgumentException("The new ColorSpace cannot decrease the " + + "maximum value for any of the components compared to the current " + + "ColorSpace/ To perform this type of conversion create a new " + + "Bitmap in the desired ColorSpace and draw this Bitmap into it."); + } + } + } + } catch (IllegalArgumentException e) { + // Undo the change to the ColorSpace. + mColorSpace = oldColorSpace; + nativeSetColorSpace(mNativePtr, mColorSpace.getNativeInstance()); + throw e; + } } /** @@ -2197,7 +2181,6 @@ public final class Bitmap implements Parcelable { private static native void nativeErase(long nativeBitmap, long colorSpacePtr, long color); private static native int nativeRowBytes(long nativeBitmap); private static native int nativeConfig(long nativeBitmap); - private static native boolean nativeIsConfigF16(long nativeBitmap); private static native int nativeGetPixel(long nativeBitmap, int x, int y); private static native long nativeGetColor(long nativeBitmap, int x, int y); @@ -2241,11 +2224,10 @@ public final class Bitmap implements Parcelable { private static native Bitmap nativeWrapHardwareBufferBitmap(HardwareBuffer buffer, long nativeColorSpace); private static native GraphicBuffer nativeCreateGraphicBufferHandle(long nativeBitmap); - private static native boolean nativeGetColorSpace(long nativePtr, float[] xyz, float[] params); + private static native ColorSpace nativeComputeColorSpace(long nativePtr); private static native void nativeSetColorSpace(long nativePtr, long nativeColorSpace); private static native boolean nativeIsSRGB(long nativePtr); private static native boolean nativeIsSRGBLinear(long nativePtr); - private static native void nativeCopyColorSpace(long srcBitmap, long dstBitmap); private static native void nativeSetImmutable(long nativePtr); diff --git a/graphics/java/android/graphics/BitmapFactory.java b/graphics/java/android/graphics/BitmapFactory.java index 7aff0414106a7..49c3a3ba68b8b 100644 --- a/graphics/java/android/graphics/BitmapFactory.java +++ b/graphics/java/android/graphics/BitmapFactory.java @@ -151,12 +151,9 @@ public class BitmapFactory { * the decoder will pick either the color space embedded in the image * or the color space best suited for the requested image configuration * (for instance {@link ColorSpace.Named#SRGB sRGB} for - * the {@link Bitmap.Config#ARGB_8888} configuration).

- * - *

{@link Bitmap.Config#RGBA_F16} always uses the - * {@link ColorSpace.Named#LINEAR_EXTENDED_SRGB scRGB} color space). - * Bitmaps in other configurations without an embedded color space are - * assumed to be in the {@link ColorSpace.Named#SRGB sRGB} color space.

+ * {@link Bitmap.Config#ARGB_8888} configuration and + * {@link ColorSpace.Named#EXTENDED_SRGB EXTENDED_SRGB} for + * {@link Bitmap.Config#RGBA_F16}).

* *

Only {@link ColorSpace.Model#RGB} color spaces are * currently supported. An IllegalArgumentException will diff --git a/graphics/java/android/graphics/ColorSpace.java b/graphics/java/android/graphics/ColorSpace.java index c9e46942a51a8..0d5233880674f 100644 --- a/graphics/java/android/graphics/ColorSpace.java +++ b/graphics/java/android/graphics/ColorSpace.java @@ -1475,7 +1475,7 @@ public abstract class ColorSpace { x -> absRcpResponse(x, 1 / 1.055, 0.055 / 1.055, 1 / 12.92, 0.04045, 2.4), x -> absResponse(x, 1 / 1.055, 0.055 / 1.055, 1 / 12.92, 0.04045, 2.4), -0.799f, 2.399f, - null, // FIXME: Use SRGB_TRANSFER_PARAMETERS + SRGB_TRANSFER_PARAMETERS, Named.EXTENDED_SRGB.ordinal() ); sNamedColorSpaces[Named.LINEAR_EXTENDED_SRGB.ordinal()] = new ColorSpace.Rgb( diff --git a/graphics/java/android/graphics/ImageDecoder.java b/graphics/java/android/graphics/ImageDecoder.java index 466a5fc2a7708..26c5080eead08 100644 --- a/graphics/java/android/graphics/ImageDecoder.java +++ b/graphics/java/android/graphics/ImageDecoder.java @@ -1556,12 +1556,9 @@ public final class ImageDecoder implements AutoCloseable { * decoder will pick either the color space embedded in the image or the * {@link ColorSpace} best suited for the requested image configuration * (for instance {@link ColorSpace.Named#SRGB sRGB} for the - * {@link Bitmap.Config#ARGB_8888} configuration).

- * - *

{@link Bitmap.Config#RGBA_F16} always uses the - * {@link ColorSpace.Named#LINEAR_EXTENDED_SRGB scRGB} color space. - * Bitmaps in other configurations without an embedded color space are - * assumed to be in the {@link ColorSpace.Named#SRGB sRGB} color space.

+ * {@link Bitmap.Config#ARGB_8888} configuration and + * {@link ColorSpace.Named#EXTENDED_SRGB EXTENDED_SRGB} for + * {@link Bitmap.Config#RGBA_F16}).

* *

Only {@link ColorSpace.Model#RGB} color spaces are * currently supported. An IllegalArgumentException will