From 4f56f99d467d8dca8c4dbe7c7263d0ab2f14058e Mon Sep 17 00:00:00 2001 From: John Reck Date: Thu, 29 Jul 2021 15:34:05 -0400 Subject: [PATCH] Remove NewWeakGlobalRef from RenderNode Bug: 194893628 Test: make && CtsUiRenderingTestCases Change-Id: I90d9d96ed940812c9dca1385f4a71d0a0cd2936f --- .../java/android/graphics/RenderNode.java | 58 +++++++++++- libs/hwui/jni/android_graphics_RenderNode.cpp | 94 +++++++++---------- 2 files changed, 97 insertions(+), 55 deletions(-) diff --git a/graphics/java/android/graphics/RenderNode.java b/graphics/java/android/graphics/RenderNode.java index 01fd231d011fb..5fd53adc409aa 100644 --- a/graphics/java/android/graphics/RenderNode.java +++ b/graphics/java/android/graphics/RenderNode.java @@ -35,6 +35,7 @@ import libcore.util.NativeAllocationRegistry; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.lang.ref.WeakReference; /** *

RenderNode is used to build hardware accelerated rendering hierarchies. Each RenderNode @@ -263,7 +264,6 @@ public final class RenderNode { * @hide */ public interface PositionUpdateListener { - /** * Called by native by a Rendering Worker thread to update window position * @@ -271,6 +271,21 @@ public final class RenderNode { */ void positionChanged(long frameNumber, int left, int top, int right, int bottom); + /** + * Called by JNI + * + * @hide */ + static boolean callPositionChanged(WeakReference weakListener, + long frameNumber, int left, int top, int right, int bottom) { + final PositionUpdateListener listener = weakListener.get(); + if (listener != null) { + listener.positionChanged(frameNumber, left, top, right, bottom); + return true; + } else { + return false; + } + } + /** * Call to apply a stretch effect to any child SurfaceControl layers * @@ -285,6 +300,26 @@ public final class RenderNode { float maxStretchX, float maxStretchY, float childRelativeLeft, float childRelativeTop, float childRelativeRight, float childRelativeBottom) { } + /** + * Called by JNI + * + * @hide */ + static boolean callApplyStretch(WeakReference weakListener, + long frameNumber, float width, float height, + float vecX, float vecY, + float maxStretchX, float maxStretchY, float childRelativeLeft, + float childRelativeTop, float childRelativeRight, float childRelativeBottom) { + final PositionUpdateListener listener = weakListener.get(); + if (listener != null) { + listener.applyStretch(frameNumber, width, height, vecX, vecY, maxStretchX, + maxStretchY, childRelativeLeft, childRelativeTop, childRelativeRight, + childRelativeBottom); + return true; + } else { + return false; + } + } + /** * Called by native on RenderThread to notify that the view is no longer in the * draw tree. UI thread is blocked at this point. @@ -293,6 +328,21 @@ public final class RenderNode { */ void positionLost(long frameNumber); + /** + * Called by JNI + * + * @hide */ + static boolean callPositionLost(WeakReference weakListener, + long frameNumber) { + final PositionUpdateListener listener = weakListener.get(); + if (listener != null) { + listener.positionLost(frameNumber); + return true; + } else { + return false; + } + } + } private static final class CompositePositionUpdateListener implements PositionUpdateListener { @@ -353,7 +403,7 @@ public final class RenderNode { comp = comp.with(listener); } mCompositePositionUpdateListener = comp; - nRequestPositionUpdates(mNativeRenderNode, comp); + nRequestPositionUpdates(mNativeRenderNode, new WeakReference<>(comp)); } /** @@ -368,7 +418,7 @@ public final class RenderNode { if (comp != null) { comp = comp.without(listener); mCompositePositionUpdateListener = comp; - nRequestPositionUpdates(mNativeRenderNode, comp); + nRequestPositionUpdates(mNativeRenderNode, new WeakReference<>(comp)); } } @@ -1575,7 +1625,7 @@ public final class RenderNode { private static native int nGetAllocatedSize(long renderNode); private static native void nRequestPositionUpdates(long renderNode, - PositionUpdateListener callback); + WeakReference callback); // Animations diff --git a/libs/hwui/jni/android_graphics_RenderNode.cpp b/libs/hwui/jni/android_graphics_RenderNode.cpp index e1da1690518ac..944393c63ad6a 100644 --- a/libs/hwui/jni/android_graphics_RenderNode.cpp +++ b/libs/hwui/jni/android_graphics_RenderNode.cpp @@ -547,9 +547,12 @@ static void android_view_RenderNode_endAllAnimators(JNIEnv* env, jobject clazz, // SurfaceView position callback // ---------------------------------------------------------------------------- -jmethodID gPositionListener_PositionChangedMethod; -jmethodID gPositionListener_ApplyStretchMethod; -jmethodID gPositionListener_PositionLostMethod; +struct { + jclass clazz; + jmethodID callPositionChanged; + jmethodID callApplyStretch; + jmethodID callPositionLost; +} gPositionListener; static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, jlong renderNodePtr, jobject listener) { @@ -557,16 +560,16 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, public: PositionListenerTrampoline(JNIEnv* env, jobject listener) { env->GetJavaVM(&mVm); - mWeakRef = env->NewWeakGlobalRef(listener); + mListener = env->NewGlobalRef(listener); } virtual ~PositionListenerTrampoline() { - jnienv()->DeleteWeakGlobalRef(mWeakRef); - mWeakRef = nullptr; + jnienv()->DeleteGlobalRef(mListener); + mListener = nullptr; } virtual void onPositionUpdated(RenderNode& node, const TreeInfo& info) override { - if (CC_UNLIKELY(!mWeakRef || !info.updateWindowPositions)) return; + if (CC_UNLIKELY(!mListener || !info.updateWindowPositions)) return; Matrix4 transform; info.damageAccumulator->computeCurrentTransform(&transform); @@ -609,7 +612,7 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, } virtual void onPositionLost(RenderNode& node, const TreeInfo* info) override { - if (CC_UNLIKELY(!mWeakRef || (info && !info->updateWindowPositions))) return; + if (CC_UNLIKELY(!mListener || (info && !info->updateWindowPositions))) return; if (mPreviousPosition.isEmpty()) { return; @@ -618,18 +621,16 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, ATRACE_NAME("SurfaceView position lost"); JNIEnv* env = jnienv(); - jobject localref = env->NewLocalRef(mWeakRef); - if (CC_UNLIKELY(!localref)) { - env->DeleteWeakGlobalRef(mWeakRef); - mWeakRef = nullptr; - return; - } #ifdef __ANDROID__ // Layoutlib does not support CanvasContext // TODO: Remember why this is synchronous and then make a comment - env->CallVoidMethod(localref, gPositionListener_PositionLostMethod, + jboolean keepListening = env->CallStaticBooleanMethod( + gPositionListener.clazz, gPositionListener.callPositionLost, mListener, info ? info->canvasContext.getFrameNumber() : 0); + if (!keepListening) { + env->DeleteGlobalRef(mListener); + mListener = nullptr; + } #endif - env->DeleteLocalRef(localref); } private: @@ -684,28 +685,20 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, StretchEffectBehavior::Shader) { JNIEnv* env = jnienv(); - jobject localref = env->NewLocalRef(mWeakRef); - if (CC_UNLIKELY(!localref)) { - env->DeleteWeakGlobalRef(mWeakRef); - mWeakRef = nullptr; - return; - } #ifdef __ANDROID__ // Layoutlib does not support CanvasContext SkVector stretchDirection = effect->getStretchDirection(); - env->CallVoidMethod(localref, gPositionListener_ApplyStretchMethod, - info.canvasContext.getFrameNumber(), - result.width, - result.height, - stretchDirection.fX, - stretchDirection.fY, - effect->maxStretchAmountX, - effect->maxStretchAmountY, - childRelativeBounds.left(), - childRelativeBounds.top(), - childRelativeBounds.right(), - childRelativeBounds.bottom()); + jboolean keepListening = env->CallStaticBooleanMethod( + gPositionListener.clazz, gPositionListener.callApplyStretch, mListener, + info.canvasContext.getFrameNumber(), result.width, result.height, + stretchDirection.fX, stretchDirection.fY, effect->maxStretchAmountX, + effect->maxStretchAmountY, childRelativeBounds.left(), + childRelativeBounds.top(), childRelativeBounds.right(), + childRelativeBounds.bottom()); + if (!keepListening) { + env->DeleteGlobalRef(mListener); + mListener = nullptr; + } #endif - env->DeleteLocalRef(localref); } } @@ -714,14 +707,12 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, ATRACE_NAME("Update SurfaceView position"); JNIEnv* env = jnienv(); - jobject localref = env->NewLocalRef(mWeakRef); - if (CC_UNLIKELY(!localref)) { - env->DeleteWeakGlobalRef(mWeakRef); - mWeakRef = nullptr; - } else { - env->CallVoidMethod(localref, gPositionListener_PositionChangedMethod, - frameNumber, left, top, right, bottom); - env->DeleteLocalRef(localref); + jboolean keepListening = env->CallStaticBooleanMethod( + gPositionListener.clazz, gPositionListener.callPositionChanged, mListener, + frameNumber, left, top, right, bottom); + if (!keepListening) { + env->DeleteGlobalRef(mListener); + mListener = nullptr; } // We need to release ourselves here @@ -729,7 +720,7 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, } JavaVM* mVm; - jobject mWeakRef; + jobject mListener; uirenderer::Rect mPreviousPosition; }; @@ -754,7 +745,7 @@ static const JNINativeMethod gMethods[] = { {"nGetAllocatedSize", "(J)I", (void*)android_view_RenderNode_getAllocatedSize}, {"nAddAnimator", "(JJ)V", (void*)android_view_RenderNode_addAnimator}, {"nEndAllAnimators", "(J)V", (void*)android_view_RenderNode_endAllAnimators}, - {"nRequestPositionUpdates", "(JLandroid/graphics/RenderNode$PositionUpdateListener;)V", + {"nRequestPositionUpdates", "(JLjava/lang/ref/WeakReference;)V", (void*)android_view_RenderNode_requestPositionUpdates}, // ---------------------------------------------------------------------------- @@ -852,12 +843,13 @@ static const JNINativeMethod gMethods[] = { int register_android_view_RenderNode(JNIEnv* env) { jclass clazz = FindClassOrDie(env, "android/graphics/RenderNode$PositionUpdateListener"); - gPositionListener_PositionChangedMethod = GetMethodIDOrDie(env, clazz, - "positionChanged", "(JIIII)V"); - gPositionListener_ApplyStretchMethod = - GetMethodIDOrDie(env, clazz, "applyStretch", "(JFFFFFFFFFF)V"); - gPositionListener_PositionLostMethod = GetMethodIDOrDie(env, clazz, - "positionLost", "(J)V"); + gPositionListener.clazz = MakeGlobalRefOrDie(env, clazz); + gPositionListener.callPositionChanged = GetStaticMethodIDOrDie( + env, clazz, "callPositionChanged", "(Ljava/lang/ref/WeakReference;JIIII)Z"); + gPositionListener.callApplyStretch = GetStaticMethodIDOrDie( + env, clazz, "callApplyStretch", "(Ljava/lang/ref/WeakReference;JFFFFFFFFFF)Z"); + gPositionListener.callPositionLost = GetStaticMethodIDOrDie( + env, clazz, "callPositionLost", "(Ljava/lang/ref/WeakReference;J)Z"); return RegisterMethodsOrDie(env, kClassPathName, gMethods, NELEM(gMethods)); }