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

Commit 2de950d5 authored by John Reck's avatar John Reck
Browse files

Overhaul RenderNode's DisplayList management

* Move mValid to native
* Have destroyHardwareResources destroy everything
* Remove flaky mParentCount checks in setStaging
* All tree updates have an internal observer to
  ensure onRemovedFromTree() is a reliable signal
* onRemovedFromTree() immediately releases resources
  to avoid displaylist "leaks"

Test: Unit tests for validity added & pass, manually
verified that b/34072929 doesn't repro

Bug: 34072929

Change-Id: I856534b4ed1b7f009fc4b7cd13209b97fa42a71c
parent df7f2835
Loading
Loading
Loading
Loading
+8 −11
Original line number Diff line number Diff line
@@ -139,9 +139,6 @@ public class RenderNode {
                RenderNode.class.getClassLoader(), nGetNativeFinalizer(), 1024);
    }

    // Note: written by native when display lists are detached
    private boolean mValid;

    // Do not access directly unless you are ThreadedRenderer
    final long mNativeRenderNode;
    private final View mOwningView;
@@ -233,7 +230,6 @@ public class RenderNode {
        long displayList = canvas.finishRecording();
        nSetDisplayList(mNativeRenderNode, displayList);
        canvas.recycle();
        mValid = true;
    }

    /**
@@ -242,10 +238,7 @@ public class RenderNode {
     * obsolete resources after related resources are gone.
     */
    public void discardDisplayList() {
        if (!mValid) return;

        nSetDisplayList(mNativeRenderNode, 0);
        mValid = false;
    }

    /**
@@ -254,10 +247,12 @@ public class RenderNode {
     *
     * @return boolean true if the display list is able to be replayed, false otherwise.
     */
    public boolean isValid() { return mValid; }
    public boolean isValid() {
        return nIsValid(mNativeRenderNode);
    }

    long getNativeDisplayList() {
        if (!mValid) {
        if (!isValid()) {
            throw new IllegalStateException("The display list is not valid.");
        }
        return mNativeRenderNode;
@@ -827,8 +822,7 @@ public class RenderNode {
    // Regular JNI methods
    ///////////////////////////////////////////////////////////////////////////

    // Intentionally not static because it acquires a reference to 'this'
    private native long nCreate(String name);
    private static native long nCreate(String name);

    private static native long nGetNativeFinalizer();
    private static native void nOutput(long renderNode);
@@ -853,6 +847,9 @@ public class RenderNode {
    // @CriticalNative methods
    ///////////////////////////////////////////////////////////////////////////

    @CriticalNative
    private static native boolean nIsValid(long renderNode);

    // Matrix

    @CriticalNative
+0 −9
Original line number Diff line number Diff line
@@ -496,15 +496,6 @@ public final class ThreadedRenderer {

    private static void destroyResources(View view) {
        view.destroyHardwareResources();

        if (view instanceof ViewGroup) {
            ViewGroup group = (ViewGroup) view;

            int count = group.getChildCount();
            for (int i = 0; i < count; i++) {
                destroyResources(group.getChildAt(i));
            }
        }
    }

    /**
+8 −10
Original line number Diff line number Diff line
@@ -16593,11 +16593,12 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
     */
    @CallSuper
    protected void destroyHardwareResources() {
        // Although the Layer will be destroyed by RenderNode, we want to release
        // the staging display list, which is also a signal to RenderNode that it's
        // safe to free its copy of the display list as it knows that we will
        // push an updated DisplayList if we try to draw again
        resetDisplayList();
        if (mOverlay != null) {
            mOverlay.getOverlayView().destroyHardwareResources();
        }
        if (mGhostView != null) {
            mGhostView.destroyHardwareResources();
        }
    }
    /**
@@ -16768,11 +16769,8 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
    }
    private void resetDisplayList() {
        if (mRenderNode.isValid()) {
        mRenderNode.discardDisplayList();
        }
        if (mBackgroundRenderNode != null && mBackgroundRenderNode.isValid()) {
        if (mBackgroundRenderNode != null) {
            mBackgroundRenderNode.discardDisplayList();
        }
    }
+10 −0
Original line number Diff line number Diff line
@@ -4597,6 +4597,16 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
        clearCachedLayoutMode();
    }

    /** @hide */
    @Override
    protected void destroyHardwareResources() {
        super.destroyHardwareResources();
        int count = getChildCount();
        for (int i = 0; i < count; i++) {
            getChildAt(i).destroyHardwareResources();
        }
    }

    /**
     * Adds a view during layout. This is useful if in your onLayout() method,
     * you need to add more views (as does the list view for example).
+7 −70
Original line number Diff line number Diff line
@@ -43,57 +43,6 @@ using namespace uirenderer;
        ? (reinterpret_cast<RenderNode*>(renderNodePtr)->setPropertyFieldsDirty(dirtyFlag), true) \
        : false)

static JNIEnv* getenv(JavaVM* vm) {
    JNIEnv* env;
    if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK) {
        LOG_ALWAYS_FATAL("Failed to get JNIEnv for JavaVM: %p", vm);
    }
    return env;
}

static jfieldID gRenderNode_validFieldID;

class RenderNodeContext : public VirtualLightRefBase {
public:
    RenderNodeContext(JNIEnv* env, jobject jobjRef) {
        env->GetJavaVM(&mVm);
        // This holds a weak ref because otherwise there's a cyclic global ref
        // with this holding a strong global ref to the view which holds
        // a strong ref to RenderNode which holds a strong ref to this.
        mWeakRef = env->NewWeakGlobalRef(jobjRef);
    }

    virtual ~RenderNodeContext() {
        JNIEnv* env = getenv(mVm);
        env->DeleteWeakGlobalRef(mWeakRef);
    }

    jobject acquireLocalRef(JNIEnv* env) {
        return env->NewLocalRef(mWeakRef);
    }

private:
    JavaVM* mVm;
    jweak mWeakRef;
};

// Called by ThreadedRenderer's JNI layer
void onRenderNodeRemoved(JNIEnv* env, RenderNode* node) {
    auto context = reinterpret_cast<RenderNodeContext*>(node->getUserContext());
    if (!context) return;
    jobject jnode = context->acquireLocalRef(env);
    if (!jnode) {
        // The owning node has been GC'd, release the context
        node->setUserContext(nullptr);
        return;
    }

    // Update the valid field, since native has already removed
    // the staging DisplayList
    env->SetBooleanField(jnode, gRenderNode_validFieldID, false);
    env->DeleteLocalRef(jnode);
}

// ----------------------------------------------------------------------------
// DisplayList view properties
// ----------------------------------------------------------------------------
@@ -108,8 +57,7 @@ static jint android_view_RenderNode_getDebugSize(JNIEnv* env, jobject clazz, jlo
    return renderNode->getDebugSize();
}

static jlong android_view_RenderNode_create(JNIEnv* env, jobject thiz,
        jstring name) {
static jlong android_view_RenderNode_create(JNIEnv* env, jobject, jstring name) {
    RenderNode* renderNode = new RenderNode();
    renderNode->incStrong(0);
    if (name != NULL) {
@@ -117,7 +65,6 @@ static jlong android_view_RenderNode_create(JNIEnv* env, jobject thiz,
        renderNode->setName(textArray);
        env->ReleaseStringUTFChars(name, textArray);
    }
    renderNode->setUserContext(new RenderNodeContext(env, thiz));
    return reinterpret_cast<jlong>(renderNode);
}

@@ -132,22 +79,13 @@ static jlong android_view_RenderNode_getNativeFinalizer(JNIEnv* env,

static void android_view_RenderNode_setDisplayList(JNIEnv* env,
        jobject clazz, jlong renderNodePtr, jlong displayListPtr) {
    class RemovedObserver : public TreeObserver {
    public:
        virtual void onMaybeRemovedFromTree(RenderNode* node) override {
            maybeRemovedNodes.insert(sp<RenderNode>(node));
        }
        std::set< sp<RenderNode> > maybeRemovedNodes;
    };

    RenderNode* renderNode = reinterpret_cast<RenderNode*>(renderNodePtr);
    DisplayList* newData = reinterpret_cast<DisplayList*>(displayListPtr);
    RemovedObserver observer;
    renderNode->setStagingDisplayList(newData, &observer);
    for (auto& node : observer.maybeRemovedNodes) {
        if (node->hasParents()) continue;
        onRenderNodeRemoved(env, node.get());
    renderNode->setStagingDisplayList(newData);
}

static jboolean android_view_RenderNode_isValid(jlong renderNodePtr) {
    return reinterpret_cast<RenderNode*>(renderNodePtr)->isValid();
}

// ----------------------------------------------------------------------------
@@ -621,6 +559,7 @@ static const JNINativeMethod gMethods[] = {
// ----------------------------------------------------------------------------
// Critical JNI via @CriticalNative annotation in RenderNode.java
// ----------------------------------------------------------------------------
    { "nIsValid",              "(J)Z",   (void*) android_view_RenderNode_isValid },
    { "nSetLayerType",         "(JI)Z",  (void*) android_view_RenderNode_setLayerType },
    { "nSetLayerPaint",        "(JJ)Z",  (void*) android_view_RenderNode_setLayerPaint },
    { "nSetStaticMatrix",      "(JJ)Z",  (void*) android_view_RenderNode_setStaticMatrix },
@@ -691,8 +630,6 @@ int register_android_view_RenderNode(JNIEnv* env) {
            "updateWindowPosition_renderWorker", "(JIIII)V");
    gSurfaceViewPositionLostMethod = GetMethodIDOrDie(env, clazz,
            "windowPositionLost_uiRtSync", "(J)V");
    clazz = FindClassOrDie(env, "android/view/RenderNode");
    gRenderNode_validFieldID = GetFieldIDOrDie(env, clazz, "mValid", "Z");
    return RegisterMethodsOrDie(env, kClassPathName, gMethods, NELEM(gMethods));
}

Loading