Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit bf3e4647 authored by Derek Sollenberger's avatar Derek Sollenberger Committed by Leon Scroggins III
Browse files

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
parent ac4bccb2

core/jni/android/graphics/Bitmap.cpp

+21 −62
Original line number Diff line number Diff line
@@ -355,9 +355,16 @@ static jobject Bitmap_creator(JNIEnv* env, jobject, jintArray jColors, 
        colorType = kN32_SkColorType;

    }



    sk_sp<SkColorSpace> 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<Bitmap> 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<jint>(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<SkColorSpace> 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<SkData> 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;



    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);

    if (colorSpace == nullptr) return nullptr;



    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 ----------------

core/jni/android/graphics/BitmapFactory.cpp

+1 −1
Original line number Diff line number Diff line
@@ -307,7 +307,7 @@ static jobject doDecode(JNIEnv* env, std::unique_ptr<SkStreamRewindable> 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;

core/jni/android/graphics/BitmapRegionDecoder.cpp

+1 −1
Original line number Diff line number Diff line
@@ -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.

core/jni/android/graphics/Graphics.cpp

+72 −55
Original line number Diff line number Diff line
@@ -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,24 +414,37 @@ jobject GraphicsJNI::createRegion(JNIEnv* env, SkRegion* region) 


///////////////////////////////////////////////////////////////////////////////



jobject GraphicsJNI::getColorSpace(JNIEnv* env, sk_sp<SkColorSpace>& decodeColorSpace,

jobject GraphicsJNI::getColorSpace(JNIEnv* env, SkColorSpace* decodeColorSpace,

        SkColorType decodeColorType) {

    jobject colorSpace = nullptr;

    if (!decodeColorSpace || decodeColorType == kAlpha_8_SkColorType) {

        return nullptr;

    }



    // No need to match, we know what the output color space will be

    // Special checks for the common sRGB cases and their extended variants.

    jobject namedCS = nullptr;

    sk_sp<SkColorSpace> srgbLinear = SkColorSpace::MakeSRGBLinear();

    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

        // An F16 Bitmap will always report that it is EXTENDED if

        // it matches a ColorSpace that has an EXTENDED variant.

        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) {

            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;
@@ -459,7 +474,7 @@ jobject GraphicsJNI::getColorSpace(JNIEnv* env, sk_sp<SkColorSpace>& decodeColor 
    };

    env->SetFloatArrayRegion(xyzArray, 0, 9, xyz);



            colorSpace = env->CallStaticObjectMethod(gColorSpace_class,

    jobject colorSpace = env->CallStaticObjectMethod(gColorSpace_class,

            gColorSpace_matchMethodID, xyzArray, params);



    if (colorSpace == nullptr) {
@@ -471,8 +486,6 @@ jobject GraphicsJNI::getColorSpace(JNIEnv* env, sk_sp<SkColorSpace>& decodeColor 
    }



    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;");

core/jni/android/graphics/GraphicsJNI.h

+7 −1
Original line number Diff line number Diff line
@@ -109,7 +109,13 @@ public: 
     */

    static sk_sp<SkColorSpace> getNativeColorSpace(jlong colorSpaceHandle);



    static jobject getColorSpace(JNIEnv* env, sk_sp<SkColorSpace>& 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);



    /**