diff --git a/core/jni/android_graphics_drawable_VectorDrawable.cpp b/core/jni/android_graphics_drawable_VectorDrawable.cpp index e882876445559..7314fbcd92d2a 100644 --- a/core/jni/android_graphics_drawable_VectorDrawable.cpp +++ b/core/jni/android_graphics_drawable_VectorDrawable.cpp @@ -138,11 +138,6 @@ static jlong createGroup(JNIEnv*, jobject, jlong srcGroupPtr) { return reinterpret_cast(newGroup); } -static void deleteNode(JNIEnv*, jobject, jlong nodePtr) { - VectorDrawable::Node* node = reinterpret_cast(nodePtr); - delete node; -} - static void setNodeName(JNIEnv* env, jobject, jlong nodePtr, jstring nameStr) { VectorDrawable::Node* node = reinterpret_cast(nodePtr); const char* nodeName = env->GetStringUTFChars(nameStr, NULL); @@ -346,7 +341,6 @@ static const JNINativeMethod gMethods[] = { {"nCreateClipPath", "!(J)J", (void*)createClipPath}, {"nCreateGroup", "!()J", (void*)createEmptyGroup}, {"nCreateGroup", "!(J)J", (void*)createGroup}, - {"nDestroy", "!(J)V", (void*)deleteNode}, {"nSetName", "(JLjava/lang/String;)V", (void*)setNodeName}, {"nUpdateGroupProperties", "!(JFFFFFFF)V", (void*)updateGroupProperties}, diff --git a/graphics/java/android/graphics/drawable/AnimatedVectorDrawable.java b/graphics/java/android/graphics/drawable/AnimatedVectorDrawable.java index 0a3e27e94c064..af8ccf5018b9c 100644 --- a/graphics/java/android/graphics/drawable/AnimatedVectorDrawable.java +++ b/graphics/java/android/graphics/drawable/AnimatedVectorDrawable.java @@ -25,6 +25,8 @@ import android.animation.ValueAnimator; import android.animation.ObjectAnimator; import android.annotation.NonNull; import android.annotation.Nullable; +import android.app.ActivityThread; +import android.app.Application; import android.content.res.ColorStateList; import android.content.res.Resources; import android.content.res.Resources.Theme; @@ -35,6 +37,7 @@ import android.graphics.Insets; import android.graphics.Outline; import android.graphics.PorterDuff; import android.graphics.Rect; +import android.os.Build; import android.util.ArrayMap; import android.util.AttributeSet; import android.util.Log; @@ -200,6 +203,24 @@ public class AnimatedVectorDrawable extends Drawable implements Animatable2 { mMutated = false; } + /** + * In order to avoid breaking old apps, we only throw exception on invalid VectorDrawable + * animations * for apps targeting N and later. For older apps, we ignore (i.e. quietly skip) + * these animations. + * + * @return whether invalid animations for vector drawable should be ignored. + */ + private static boolean shouldIgnoreInvalidAnimation() { + Application app = ActivityThread.currentApplication(); + if (app == null || app.getApplicationInfo() == null) { + return true; + } + if (app.getApplicationInfo().targetSdkVersion < Build.VERSION_CODES.N) { + return true; + } + return false; + } + @Override public ConstantState getConstantState() { mAnimatedVectorState.mChangingConfigurations = getChangingConfigurations(); @@ -763,6 +784,8 @@ public class AnimatedVectorDrawable extends Drawable implements Animatable2 { private boolean mInitialized = false; private boolean mAnimationPending = false; private boolean mIsReversible = false; + // This needs to be set before parsing starts. + private boolean mShouldIgnoreInvalidAnim; // TODO: Consider using NativeAllocationRegistery to track native allocation private final VirtualRefBasePtr mSetRefBasePtr; private WeakReference mTarget = null; @@ -782,6 +805,7 @@ public class AnimatedVectorDrawable extends Drawable implements Animatable2 { throw new UnsupportedOperationException("VectorDrawableAnimator cannot be " + "re-initialized"); } + mShouldIgnoreInvalidAnim = shouldIgnoreInvalidAnimation(); parseAnimatorSet(set, 0); mInitialized = true; @@ -841,7 +865,7 @@ public class AnimatedVectorDrawable extends Drawable implements Animatable2 { } else if (target instanceof VectorDrawable.VFullPath) { createRTAnimatorForFullPath(animator, (VectorDrawable.VFullPath) target, startTime); - } else { + } else if (!mShouldIgnoreInvalidAnim) { throw new IllegalArgumentException("ClipPath only supports PathData " + "property"); } @@ -850,10 +874,11 @@ public class AnimatedVectorDrawable extends Drawable implements Animatable2 { } else if (target instanceof VectorDrawable.VectorDrawableState) { createRTAnimatorForRootGroup(values, animator, (VectorDrawable.VectorDrawableState) target, startTime); - } else { + } else if (!mShouldIgnoreInvalidAnim) { // Should never get here throw new UnsupportedOperationException("Target should be either VGroup, VPath, " + - "or ConstantState, " + target.getClass() + " is not supported"); + "or ConstantState, " + target == null ? "Null target" : target.getClass() + + " is not supported"); } } @@ -912,8 +937,12 @@ public class AnimatedVectorDrawable extends Drawable implements Animatable2 { long nativePtr = target.getNativePtr(); if (mTmpValues.type == Float.class || mTmpValues.type == float.class) { if (propertyId < 0) { - throw new IllegalArgumentException("Property: " + mTmpValues - .propertyName + " is not supported for FullPath"); + if (mShouldIgnoreInvalidAnim) { + return; + } else { + throw new IllegalArgumentException("Property: " + mTmpValues.propertyName + + " is not supported for FullPath"); + } } propertyPtr = nCreatePathPropertyHolder(nativePtr, propertyId, (Float) mTmpValues.startValue, (Float) mTmpValues.endValue); @@ -922,9 +951,13 @@ public class AnimatedVectorDrawable extends Drawable implements Animatable2 { propertyPtr = nCreatePathColorPropertyHolder(nativePtr, propertyId, (Integer) mTmpValues.startValue, (Integer) mTmpValues.endValue); } else { - throw new UnsupportedOperationException("Unsupported type: " + - mTmpValues.type + ". Only float, int or PathData value is " + - "supported for Paths."); + if (mShouldIgnoreInvalidAnim) { + return; + } else { + throw new UnsupportedOperationException("Unsupported type: " + + mTmpValues.type + ". Only float, int or PathData value is " + + "supported for Paths."); + } } if (mTmpValues.dataSource != null) { float[] dataPoints = createDataPoints(mTmpValues.dataSource, animator @@ -939,8 +972,12 @@ public class AnimatedVectorDrawable extends Drawable implements Animatable2 { long startTime) { long nativePtr = target.getNativeRenderer(); if (!animator.getPropertyName().equals("alpha")) { - throw new UnsupportedOperationException("Only alpha is supported for root " + - "group"); + if (mShouldIgnoreInvalidAnim) { + return; + } else { + throw new UnsupportedOperationException("Only alpha is supported for root " + + "group"); + } } Float startValue = null; Float endValue = null; @@ -953,7 +990,11 @@ public class AnimatedVectorDrawable extends Drawable implements Animatable2 { } } if (startValue == null && endValue == null) { - throw new UnsupportedOperationException("No alpha values are specified"); + if (mShouldIgnoreInvalidAnim) { + return; + } else { + throw new UnsupportedOperationException("No alpha values are specified"); + } } long propertyPtr = nCreateRootAlphaPropertyHolder(nativePtr, startValue, endValue); createNativeChildAnimator(propertyPtr, startTime, animator); diff --git a/graphics/java/android/graphics/drawable/VectorDrawable.java b/graphics/java/android/graphics/drawable/VectorDrawable.java index f4bbc8c43d08e..bdbf3c04b0004 100644 --- a/graphics/java/android/graphics/drawable/VectorDrawable.java +++ b/graphics/java/android/graphics/drawable/VectorDrawable.java @@ -924,8 +924,11 @@ public class VectorDrawable extends Drawable { private int mChangingConfigurations; private int[] mThemeAttrs; private String mGroupName = null; - private long mNativePtr = 0; + // The native object will be created in the constructor and will be destroyed in native + // when the neither java nor native has ref to the tree. This pointer should be valid + // throughout this VGroup Java object's life. + private final long mNativePtr; public VGroup(VGroup copy, ArrayMap targetsMap) { mIsStateful = copy.mIsStateful; @@ -1064,16 +1067,6 @@ public class VectorDrawable extends Drawable { return false; } - @Override - protected void finalize() throws Throwable { - if (mNativePtr != 0) { - nDestroy(mNativePtr); - mNativePtr = 0; - } - super.finalize(); - } - - @Override public void applyTheme(Theme t) { if (mThemeAttrs != null) { @@ -1208,10 +1201,10 @@ public class VectorDrawable extends Drawable { * Clip path, which only has name and pathData. */ private static class VClipPath extends VPath { - long mNativePtr = 0; + private final long mNativePtr; + public VClipPath() { mNativePtr = nCreateClipPath(); - // Empty constructor. } public VClipPath(VClipPath copy) { @@ -1224,14 +1217,6 @@ public class VectorDrawable extends Drawable { return mNativePtr; } - @Override - protected void finalize() throws Throwable { - if (mNativePtr != 0) { - nDestroy(mNativePtr); - mNativePtr = 0; - } - super.finalize(); - } @Override public void inflate(Resources r, AttributeSet attrs, Theme theme) { final TypedArray a = obtainAttributes(r, theme, attrs, @@ -1317,10 +1302,9 @@ public class VectorDrawable extends Drawable { ComplexColor mStrokeColors = null; ComplexColor mFillColors = null; - private long mNativePtr = 0; + private final long mNativePtr; public VFullPath() { - // Empty constructor. mNativePtr = nCreateFullPath(); } @@ -1384,15 +1368,6 @@ public class VectorDrawable extends Drawable { a.recycle(); } - @Override - protected void finalize() throws Throwable { - if (mNativePtr != 0) { - nDestroy(mNativePtr); - mNativePtr = 0; - } - super.finalize(); - } - private void updateStateFromTypedArray(TypedArray a) { int byteCount = TOTAL_PROPERTY_COUNT * 4; if (mPropertyData == null) { @@ -1647,7 +1622,7 @@ public class VectorDrawable extends Drawable { private static native void nDraw(long rendererPtr, long canvasWrapperPtr, long colorFilterPtr, Rect bounds, boolean needsMirroring, boolean canReuseCache); private static native long nCreateFullPath(); - private static native long nCreateFullPath(long mNativeFullPathPtr); + private static native long nCreateFullPath(long nativeFullPathPtr); private static native boolean nGetFullPathProperties(long pathPtr, byte[] properties, int length); @@ -1663,7 +1638,6 @@ public class VectorDrawable extends Drawable { private static native long nCreateGroup(); private static native long nCreateGroup(long groupPtr); - private static native void nDestroy(long nodePtr); private static native void nSetName(long nodePtr, String name); private static native boolean nGetGroupProperties(long groupPtr, float[] properties, int length); diff --git a/libs/hwui/VectorDrawable.cpp b/libs/hwui/VectorDrawable.cpp index 541c799f0149a..2e3856fafb606 100644 --- a/libs/hwui/VectorDrawable.cpp +++ b/libs/hwui/VectorDrawable.cpp @@ -324,7 +324,7 @@ void Group::draw(SkCanvas* outCanvas, const SkMatrix& currentMatrix, float scale // Save the current clip information, which is local to this group. outCanvas->save(); // Draw the group tree in the same order as the XML file. - for (Node* child : mChildren) { + for (auto& child : mChildren) { child->draw(outCanvas, stackedMatrix, scaleX, scaleY); } // Restore the previous clip information. @@ -361,7 +361,7 @@ void Group::getLocalMatrix(SkMatrix* outMatrix) { } void Group::addChild(Node* child) { - mChildren.push_back(child); + mChildren.emplace_back(child); } bool Group::getProperties(float* outProperties, int length) { diff --git a/libs/hwui/VectorDrawable.h b/libs/hwui/VectorDrawable.h index f8f1ea62a6248..36a8aebeaa332 100644 --- a/libs/hwui/VectorDrawable.h +++ b/libs/hwui/VectorDrawable.h @@ -316,7 +316,7 @@ private: // Count of the properties, must be at the end. Count, }; - std::vector mChildren; + std::vector< std::unique_ptr > mChildren; Properties mProperties; }; @@ -360,7 +360,7 @@ private: float mViewportHeight = 0; float mRootAlpha = 1.0f; - Group* mRootNode; + std::unique_ptr mRootNode; SkRect mBounds; SkMatrix mCanvasMatrix; SkPaint mPaint;