From ce1a64848444719eef38f1c65a725ff2b5dbbbd2 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Thu, 22 Oct 2020 17:41:30 -0700 Subject: [PATCH] Allow creating child surfaces from BlastBufferQueue App such as Chrome create child surfaces and parent them to surfaces provided by SurfaceView. When we enable the blast adapter for SurfaceView, the IGBP returned to the app is created in the client and SurfaceFlinger does not know about it. When the app creates a child surface and provides the IGBP as the parent surface identifier, SF fails to validate the IGBP and the surface is not created. This can be avoid if the client creates the child surface from the SV SurfaceControl but we still need to support existing APIs. To fix this, when we create a Surface from the adapter, pass in the handle of the Blast SurfaceControl. When calling ASurfaceControl_createFromWindow, use this handle to identify the parent. Bug: 168917217 Test: adb shell settings put global use_blast_adapter_sv 1 & launch chrome Change-Id: I879b411c47e8558397516bd7b7278813e79e005f --- PREUPLOAD.cfg | 1 + core/java/android/view/SurfaceView.java | 2 +- .../jni/android_graphics_BLASTBufferQueue.cpp | 8 ++++--- core/jni/android_view_Surface.cpp | 6 +++-- core/jni/android_view_SurfaceControl.cpp | 9 +++++-- .../android/graphics/BLASTBufferQueue.java | 12 ++++++++-- native/android/surface_control.cpp | 24 ++++++++++++++----- 7 files changed, 46 insertions(+), 16 deletions(-) diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg index f66d12a695944..cdf5df6c6bd32 100644 --- a/PREUPLOAD.cfg +++ b/PREUPLOAD.cfg @@ -9,6 +9,7 @@ clang_format = --commit ${PREUPLOAD_COMMIT} --style file --extensions c,h,cc,cpp cmds/uinput/ core/jni/ libs/input/ + native/ services/core/jni/ services/incremental/ tests/ diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java index 432d9279c48de..14748f03e9341 100644 --- a/core/java/android/view/SurfaceView.java +++ b/core/java/android/view/SurfaceView.java @@ -1207,7 +1207,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall // Therefore, we must explicitly recreate the {@link Surface} in these // cases. if (mUseBlastAdapter) { - mSurface.transferFrom(mBlastBufferQueue.createSurface()); + mSurface.transferFrom(mBlastBufferQueue.createSurfaceWithHandle()); } else { mSurface.createFrom(mSurfaceControl); } diff --git a/core/jni/android_graphics_BLASTBufferQueue.cpp b/core/jni/android_graphics_BLASTBufferQueue.cpp index 0fc0451c6d4bb..3e87cb5003f6a 100644 --- a/core/jni/android_graphics_BLASTBufferQueue.cpp +++ b/core/jni/android_graphics_BLASTBufferQueue.cpp @@ -53,9 +53,11 @@ static void nativeDestroy(JNIEnv* env, jclass clazz, jlong ptr) { queue->decStrong((void*)nativeCreate); } -static jobject nativeGetSurface(JNIEnv* env, jclass clazz, jlong ptr) { +static jobject nativeGetSurface(JNIEnv* env, jclass clazz, jlong ptr, + jboolean includeSurfaceControlHandle) { sp queue = reinterpret_cast(ptr); - return android_view_Surface_createFromSurface(env, queue->getSurface()); + return android_view_Surface_createFromSurface(env, + queue->getSurface(includeSurfaceControlHandle)); } static void nativeSetNextTransaction(JNIEnv* env, jclass clazz, jlong ptr, jlong transactionPtr) { @@ -77,7 +79,7 @@ static void nativeFlushShadowQueue(JNIEnv* env, jclass clazz, jlong ptr) { static const JNINativeMethod gMethods[] = { /* name, signature, funcPtr */ {"nativeCreate", "(Ljava/lang/String;JJJZ)J", (void*)nativeCreate}, - {"nativeGetSurface", "(J)Landroid/view/Surface;", (void*)nativeGetSurface}, + {"nativeGetSurface", "(JZ)Landroid/view/Surface;", (void*)nativeGetSurface}, {"nativeDestroy", "(J)V", (void*)nativeDestroy}, {"nativeSetNextTransaction", "(JJ)V", (void*)nativeSetNextTransaction}, {"nativeUpdate", "(JJJJ)V", (void*)nativeUpdate}, diff --git a/core/jni/android_view_Surface.cpp b/core/jni/android_view_Surface.cpp index 5c87f1973bed7..3a1ccd9de79e7 100644 --- a/core/jni/android_view_Surface.cpp +++ b/core/jni/android_view_Surface.cpp @@ -313,7 +313,7 @@ static jlong nativeGetFromBlastBufferQueue(JNIEnv* env, jclass clazz, jlong nati return nativeObject; } - sp surface = queue->getSurface(); + sp surface = queue->getSurface(true /* includeSurfaceControlHandle */); if (surface != NULL) { surface->incStrong(&sRefBaseOwner); } @@ -349,7 +349,8 @@ static jlong nativeReadFromParcel(JNIEnv* env, jclass clazz, sp sur; if (surfaceShim.graphicBufferProducer != nullptr) { // we have a new IGraphicBufferProducer, create a new Surface for it - sur = new Surface(surfaceShim.graphicBufferProducer, true); + sur = new Surface(surfaceShim.graphicBufferProducer, true, + surfaceShim.surfaceControlHandle); // and keep a reference before passing to java sur->incStrong(&sRefBaseOwner); } @@ -373,6 +374,7 @@ static void nativeWriteToParcel(JNIEnv* env, jclass clazz, android::view::Surface surfaceShim; if (self != nullptr) { surfaceShim.graphicBufferProducer = self->getIGraphicBufferProducer(); + surfaceShim.surfaceControlHandle = self->getSurfaceControlHandle(); } // Calling code in Surface.java has already written the name of the Surface // to the Parcel diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp index a61903dcb7c83..62f844eb59a85 100644 --- a/core/jni/android_view_SurfaceControl.cpp +++ b/core/jni/android_view_SurfaceControl.cpp @@ -318,8 +318,13 @@ static jlong nativeCreate(JNIEnv* env, jclass clazz, jobject sessionObj, } } - status_t err = client->createSurfaceChecked( - String8(name.c_str()), w, h, format, &surface, flags, parent, std::move(metadata)); + sp parentHandle; + if (parent != nullptr) { + parentHandle = parent->getHandle(); + } + + status_t err = client->createSurfaceChecked(String8(name.c_str()), w, h, format, &surface, + flags, parentHandle, std::move(metadata)); if (err == NAME_NOT_FOUND) { jniThrowException(env, "java/lang/IllegalArgumentException", NULL); return 0; diff --git a/graphics/java/android/graphics/BLASTBufferQueue.java b/graphics/java/android/graphics/BLASTBufferQueue.java index 8284042c4dca8..94bfdc9dbad63 100644 --- a/graphics/java/android/graphics/BLASTBufferQueue.java +++ b/graphics/java/android/graphics/BLASTBufferQueue.java @@ -29,7 +29,7 @@ public final class BLASTBufferQueue { private static native long nativeCreate(String name, long surfaceControl, long width, long height, boolean tripleBufferingEnabled); private static native void nativeDestroy(long ptr); - private static native Surface nativeGetSurface(long ptr); + private static native Surface nativeGetSurface(long ptr, boolean includeSurfaceControlHandle); private static native void nativeSetNextTransaction(long ptr, long transactionPtr); private static native void nativeUpdate(long ptr, long surfaceControl, long width, long height); private static native void nativeFlushShadowQueue(long ptr); @@ -49,7 +49,15 @@ public final class BLASTBufferQueue { * @return a new Surface instance from the IGraphicsBufferProducer of the adapter. */ public Surface createSurface() { - return nativeGetSurface(mNativeObject); + return nativeGetSurface(mNativeObject, false /* includeSurfaceControlHandle */); + } + + /** + * @return a new Surface instance from the IGraphicsBufferProducer of the adapter and + * the SurfaceControl handle. + */ + public Surface createSurfaceWithHandle() { + return nativeGetSurface(mNativeObject, true /* includeSurfaceControlHandle */); } public void setNextTransaction(SurfaceControl.Transaction t) { diff --git a/native/android/surface_control.cpp b/native/android/surface_control.cpp index 0a466f424cece..c503721319fb1 100644 --- a/native/android/surface_control.cpp +++ b/native/android/surface_control.cpp @@ -137,12 +137,24 @@ ASurfaceControl* ASurfaceControl_createFromWindow(ANativeWindow* window, const c return nullptr; } + Surface* surface = static_cast(window); + sp parentHandle = surface->getSurfaceControlHandle(); + uint32_t flags = ISurfaceComposerClient::eFXSurfaceBufferState; - sp surfaceControl = - client->createWithSurfaceParent(String8(debug_name), 0 /* width */, 0 /* height */, - // Format is only relevant for buffer queue layers. - PIXEL_FORMAT_UNKNOWN /* format */, flags, - static_cast(window)); + sp surfaceControl; + if (parentHandle) { + surfaceControl = + client->createSurface(String8(debug_name), 0 /* width */, 0 /* height */, + // Format is only relevant for buffer queue layers. + PIXEL_FORMAT_UNKNOWN /* format */, flags, parentHandle); + } else { + surfaceControl = + client->createWithSurfaceParent(String8(debug_name), 0 /* width */, 0 /* height */, + // Format is only relevant for buffer queue layers. + PIXEL_FORMAT_UNKNOWN /* format */, flags, + static_cast(window)); + } + if (!surfaceControl) { return nullptr; } @@ -164,7 +176,7 @@ ASurfaceControl* ASurfaceControl_create(ASurfaceControl* parent, const char* deb client->createSurface(String8(debug_name), 0 /* width */, 0 /* height */, // Format is only relevant for buffer queue layers. PIXEL_FORMAT_UNKNOWN /* format */, flags, - surfaceControlParent); + surfaceControlParent->getHandle()); if (!surfaceControl) { return nullptr; }