From 6ee411010ea270351d495bf357fc294304286a70 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Tue, 15 Jan 2019 09:34:37 -0500 Subject: [PATCH] Add Paint#get(ShadowLayer)ColorLong Bug: 120904891 Test: I6de01bd94fade605643af590f8d6909f09a5831e Store Paint's colors (both the ShadowLayerColor, and the previously uncached color that is stored in the native SkPaint) as @ColorLongs. When setting or retrieving the int version, convert. This allows returning the long value that was manually set, so that we return the color in the ColorSpace that was requested. Since the color is already stored in Java, remove nGetAlpha, and return the alpha from the ColorLong. When setting alpha, update the cached value, too. Make setShadowLayer(..., @ColorInt) and setColor(@ColorInt) call the @ColorLong versions, so they can share single JNI entry points. Change-Id: Ifc559893dd4db2629c59b6e53f0b2166d43e6049 --- api/test-current.txt | 2 + core/jni/android/graphics/Paint.cpp | 49 +++-------- graphics/java/android/graphics/Paint.java | 99 +++++++++++++++-------- 3 files changed, 77 insertions(+), 73 deletions(-) diff --git a/api/test-current.txt b/api/test-current.txt index 6ddb341191cbc..7a0bdf019549e 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -510,6 +510,8 @@ package android.graphics { } public class Paint { + method public long getColorLong(); + method public long getShadowLayerColorLong(); method public void setColor(long); method public void setShadowLayer(float, float, float, long); } diff --git a/core/jni/android/graphics/Paint.cpp b/core/jni/android/graphics/Paint.cpp index bc49771c380b6..e2e3042ee5b9d 100644 --- a/core/jni/android/graphics/Paint.cpp +++ b/core/jni/android/graphics/Paint.cpp @@ -557,8 +557,9 @@ namespace PaintGlue { return result; } - // FIXME: Should this be FastNative? - static void setColorLong(JNIEnv* env, jobject clazz, jlong paintHandle, jobject jColorSpace, + // FIXME: Make this CriticalNative when we no longer need to use JNIEnv. b/122514935 will allow + // passing the SkColorSpace directly from JNI. + static void setColor(JNIEnv* env, jobject clazz, jlong paintHandle, jobject jColorSpace, jfloat r, jfloat g, jfloat b, jfloat a) { sk_sp cs = GraphicsJNI::getNativeColorSpace(env, jColorSpace); if (GraphicsJNI::hasException(env)) { @@ -569,9 +570,11 @@ namespace PaintGlue { reinterpret_cast(paintHandle)->setColor4f(color, cs.get()); } - static void setShadowLayerLong(JNIEnv* env, jobject clazz, jlong paintHandle, jfloat radius, - jfloat dx, jfloat dy, jobject jColorSpace, - jfloat r, jfloat g, jfloat b, jfloat a) { + // FIXME: Make this CriticalNative when we no longer need to use JNIEnv. b/122514935 will allow + // passing the SkColorSpace directly from JNI. + static void setShadowLayer(JNIEnv* env, jobject clazz, jlong paintHandle, jfloat radius, + jfloat dx, jfloat dy, jobject jColorSpace, + jfloat r, jfloat g, jfloat b, jfloat a) { sk_sp cs = GraphicsJNI::getNativeColorSpace(env, jColorSpace); if (GraphicsJNI::hasException(env)) { return; @@ -784,22 +787,6 @@ namespace PaintGlue { obj->setStyle(style); } - static jint getColor(jlong paintHandle) { - int color; - color = reinterpret_cast(paintHandle)->getColor(); - return static_cast(color); - } - - static jint getAlpha(jlong paintHandle) { - int alpha; - alpha = reinterpret_cast(paintHandle)->getAlpha(); - return static_cast(alpha); - } - - static void setColor(jlong paintHandle, jint color) { - reinterpret_cast(paintHandle)->setColor(color); - } - static void setAlpha(jlong paintHandle, jint a) { reinterpret_cast(paintHandle)->setAlpha(a); } @@ -1047,18 +1034,6 @@ namespace PaintGlue { return SkScalarToFloat(Paint::kStdStrikeThru_Thickness * textSize); } - static void setShadowLayer(jlong paintHandle, jfloat radius, - jfloat dx, jfloat dy, jint color) { - Paint* paint = reinterpret_cast(paintHandle); - if (radius <= 0) { - paint->setLooper(nullptr); - } - else { - SkScalar sigma = android::uirenderer::Blur::convertRadiusToSigma(radius); - paint->setLooper(SkBlurDrawLooper::Make((SkColor)color, sigma, dx, dy)); - } - } - static jboolean hasShadowLayer(jlong paintHandle) { Paint* paint = reinterpret_cast(paintHandle); return paint->getLooper() && paint->getLooper()->asABlurShadow(nullptr); @@ -1107,9 +1082,9 @@ static const JNINativeMethod methods[] = { {"nGetRunAdvance", "(J[CIIIIZI)F", (void*) PaintGlue::getRunAdvance___CIIIIZI_F}, {"nGetOffsetForAdvance", "(J[CIIIIZF)I", (void*) PaintGlue::getOffsetForAdvance___CIIIIZF_I}, - {"nSetColor","(JLandroid/graphics/ColorSpace;FFFF)V", (void*) PaintGlue::setColorLong}, + {"nSetColor","(JLandroid/graphics/ColorSpace;FFFF)V", (void*) PaintGlue::setColor}, {"nSetShadowLayer", "(JFFFLandroid/graphics/ColorSpace;FFFF)V", - (void*)PaintGlue::setShadowLayerLong}, + (void*)PaintGlue::setShadowLayer}, // --------------- @FastNative ---------------------- @@ -1139,9 +1114,6 @@ static const JNINativeMethod methods[] = { {"nSetDither","(JZ)V", (void*) PaintGlue::setDither}, {"nGetStyle","(J)I", (void*) PaintGlue::getStyle}, {"nSetStyle","(JI)V", (void*) PaintGlue::setStyle}, - {"nGetColor","(J)I", (void*) PaintGlue::getColor}, - {"nSetColor","(JI)V", (void*) PaintGlue::setColor}, - {"nGetAlpha","(J)I", (void*) PaintGlue::getAlpha}, {"nSetAlpha","(JI)V", (void*) PaintGlue::setAlpha}, {"nGetStrokeWidth","(J)F", (void*) PaintGlue::getStrokeWidth}, {"nSetStrokeWidth","(JF)V", (void*) PaintGlue::setStrokeWidth}, @@ -1182,7 +1154,6 @@ static const JNINativeMethod methods[] = { {"nGetUnderlineThickness","(J)F", (void*) PaintGlue::getUnderlineThickness}, {"nGetStrikeThruPosition","(J)F", (void*) PaintGlue::getStrikeThruPosition}, {"nGetStrikeThruThickness","(J)F", (void*) PaintGlue::getStrikeThruThickness}, - {"nSetShadowLayer", "(JFFFI)V", (void*)PaintGlue::setShadowLayer}, {"nHasShadowLayer", "(J)Z", (void*)PaintGlue::hasShadowLayer}, {"nEqualsForTextMeasurement", "(JJ)Z", (void*)PaintGlue::equalsForTextMeasurement}, }; diff --git a/graphics/java/android/graphics/Paint.java b/graphics/java/android/graphics/Paint.java index cbb780deac8a0..54e1abcaf1b7e 100644 --- a/graphics/java/android/graphics/Paint.java +++ b/graphics/java/android/graphics/Paint.java @@ -68,26 +68,27 @@ public class Paint { Paint.class.getClassLoader(), nGetNativeFinalizer(), NATIVE_PAINT_SIZE); } - private ColorFilter mColorFilter; - private MaskFilter mMaskFilter; - private PathEffect mPathEffect; - private Shader mShader; + @ColorLong private long mColor; + private ColorFilter mColorFilter; + private MaskFilter mMaskFilter; + private PathEffect mPathEffect; + private Shader mShader; @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023) - private Typeface mTypeface; - private Xfermode mXfermode; + private Typeface mTypeface; + private Xfermode mXfermode; - private boolean mHasCompatScaling; - private float mCompatScaling; - private float mInvCompatScaling; + private boolean mHasCompatScaling; + private float mCompatScaling; + private float mInvCompatScaling; - private LocaleList mLocales; - private String mFontFeatureSettings; - private String mFontVariationSettings; + private LocaleList mLocales; + private String mFontFeatureSettings; + private String mFontVariationSettings; - private float mShadowLayerRadius; - private float mShadowLayerDx; - private float mShadowLayerDy; - private int mShadowLayerColor; + private float mShadowLayerRadius; + private float mShadowLayerDx; + private float mShadowLayerDy; + @ColorLong private long mShadowLayerColor; private static final Object sCacheLock = new Object(); @@ -505,6 +506,7 @@ public class Paint { // ? HINTING_OFF : HINTING_ON); mCompatScaling = mInvCompatScaling = 1; setTextLocales(LocaleList.getAdjustedDefault()); + mColor = Color.pack(Color.BLACK); } /** @@ -530,6 +532,7 @@ public class Paint { // setHinting(DisplayMetrics.DENSITY_DEVICE >= DisplayMetrics.DENSITY_TV // ? HINTING_OFF : HINTING_ON); + mColor = Color.pack(Color.BLACK); mColorFilter = null; mMaskFilter = null; mPathEffect = null; @@ -551,7 +554,7 @@ public class Paint { mShadowLayerRadius = 0.0f; mShadowLayerDx = 0.0f; mShadowLayerDy = 0.0f; - mShadowLayerColor = 0; + mShadowLayerColor = Color.pack(0); } /** @@ -572,6 +575,7 @@ public class Paint { * {@link Paint}. */ private void setClassVariablesFrom(Paint paint) { + mColor = paint.mColor; mColorFilter = paint.mColorFilter; mMaskFilter = paint.mMaskFilter; mPathEffect = paint.mPathEffect; @@ -949,7 +953,7 @@ public class Paint { } /** - * Return the paint's color. Note that the color is a 32bit value + * Return the paint's color in sRGB. Note that the color is a 32bit value * containing alpha as well as r,g,b. This 32bit value is not premultiplied, * meaning that its alpha can be any value, regardless of the values of * r,g,b. See the Color class for more details. @@ -958,7 +962,25 @@ public class Paint { */ @ColorInt public int getColor() { - return nGetColor(mNativePaint); + return Color.toArgb(mColor); + } + + /** + * Return the paint's color. Note that the color is a long with an encoded + * {@link ColorSpace} as well as alpha and r,g,b. These values are not + * premultiplied, meaning that alpha can be any value, regardless of the + * values of r,g,b. See the {@link Color} class for more details. + * + * @see Color for APIs that help manipulate a color long. + * + * @return the paint's color (and alpha). + * + * @hide pending API approval + */ + @TestApi + @ColorLong + public long getColorLong() { + return mColor; } /** @@ -970,7 +992,7 @@ public class Paint { * @param color The new color (including alpha) to set in the paint. */ public void setColor(@ColorInt int color) { - nSetColor(mNativePaint, color); + setColor(Color.pack(color)); } /** @@ -996,6 +1018,7 @@ public class Paint { float a = Color.alpha(color); nSetColor(mNativePaint, cs, r, g, b, a); + mColor = color; } /** @@ -1006,7 +1029,7 @@ public class Paint { * @return the alpha component of the paint's color. */ public int getAlpha() { - return nGetAlpha(mNativePaint); + return Math.round(Color.alpha(mColor) * 255.0f); } /** @@ -1017,6 +1040,13 @@ public class Paint { * @param a set the alpha component [0..255] of the paint's color. */ public void setAlpha(int a) { + // FIXME: No need to unpack this. Instead, just update the alpha bits. + // b/122959599 + ColorSpace cs = Color.colorSpace(mColor); + float r = Color.red(mColor); + float g = Color.green(mColor); + float b = Color.blue(mColor); + mColor = Color.pack(r, g, b, a * (1.0f / 255), cs); nSetAlpha(mNativePaint, a); } @@ -1398,12 +1428,7 @@ public class Paint { * opaque, or the alpha from the shadow color if not. */ public void setShadowLayer(float radius, float dx, float dy, @ColorInt int shadowColor) { - mShadowLayerRadius = radius; - mShadowLayerDx = dx; - mShadowLayerDy = dy; - mShadowLayerColor = shadowColor; - // FIXME: Share a single native method with the ColorLong version. - nSetShadowLayer(mNativePaint, radius, dx, dy, shadowColor); + setShadowLayer(radius, dx, dy, Color.pack(shadowColor)); } /** @@ -1435,7 +1460,7 @@ public class Paint { mShadowLayerRadius = radius; mShadowLayerDx = dx; mShadowLayerDy = dy; - mShadowLayerColor = Color.toArgb(shadowColor); + mShadowLayerColor = shadowColor; } /** @@ -1482,8 +1507,20 @@ public class Paint { /** * Returns the color of the shadow layer. * @see #setShadowLayer(float,float,float,int) + * @see #setShadowLayer(float,float,float,long) */ public @ColorInt int getShadowLayerColor() { + return Color.toArgb(mShadowLayerColor); + } + + /** + * Returns the color of the shadow layer. + * @see #setShadowLayer(float,float,float,int) + * @see #setShadowLayer(float,float,float,long) + * @hide pending API approval + */ + @TestApi + public @ColorLong long getShadowLayerColorLong() { return mShadowLayerColor; } @@ -3074,12 +3111,6 @@ public class Paint { @CriticalNative private static native void nSetFilterBitmap(long paintPtr, boolean filter); @CriticalNative - private static native int nGetColor(long paintPtr); - @CriticalNative - private static native void nSetColor(long paintPtr, @ColorInt int color); - @CriticalNative - private static native int nGetAlpha(long paintPtr); - @CriticalNative private static native void nSetStrikeThruText(long paintPtr, boolean strikeThruText); @CriticalNative private static native boolean nIsElegantTextHeight(long paintPtr);