From 0d31791092e7005037bca0a86395cf1c074fc90d Mon Sep 17 00:00:00 2001 From: Jarvis Huang Date: Tue, 4 Oct 2022 10:58:50 +0800 Subject: [PATCH] comp/android: Refine surface creation flow Reuse MonadoView when "Display over other apps" is enabled. Move surface creation logic to compositor for consistency. With this approach, compositor implementer controls the way surface is created. --- doc/changes/compositor/mr.1742.md | 1 + .../android/android_custom_surface.cpp | 9 ++++ .../android/android_custom_surface.h | 3 ++ src/xrt/compositor/main/comp_window_android.c | 10 ++-- .../org/freedesktop/monado/ipc/IMonado.aidl | 5 -- .../org/freedesktop/monado/ipc/Client.java | 32 +++--------- .../freedesktop/monado/ipc/MonadoImpl.java | 51 +++++++------------ .../freedesktop/monado/ipc/MonadoService.kt | 9 +--- .../MonadoOpenXrApplication.java | 18 +------ .../targets/service-lib/service_target.cpp | 25 ++++----- 10 files changed, 57 insertions(+), 106 deletions(-) create mode 100644 doc/changes/compositor/mr.1742.md diff --git a/doc/changes/compositor/mr.1742.md b/doc/changes/compositor/mr.1742.md new file mode 100644 index 000000000..3590b5f45 --- /dev/null +++ b/doc/changes/compositor/mr.1742.md @@ -0,0 +1 @@ +Android: Refactor surface creation flow. diff --git a/src/xrt/auxiliary/android/android_custom_surface.cpp b/src/xrt/auxiliary/android/android_custom_surface.cpp index 1b1eaf094..598626f49 100644 --- a/src/xrt/auxiliary/android/android_custom_surface.cpp +++ b/src/xrt/auxiliary/android/android_custom_surface.cpp @@ -17,6 +17,7 @@ #include "wrap/android.content.h" #include "wrap/android.hardware.display.h" +#include "wrap/android.provider.h" #include "wrap/android.view.h" #include "org.freedesktop.monado.auxiliary.hpp" @@ -24,6 +25,7 @@ using wrap::android::content::Context; using wrap::android::hardware::display::DisplayManager; +using wrap::android::provider::Settings; using wrap::android::view::Display; using wrap::android::view::SurfaceHolder; using wrap::android::view::WindowManager_LayoutParams; @@ -198,3 +200,10 @@ android_custom_surface_get_display_metrics(struct _JavaVM *vm, return false; } } + +bool +android_custom_surface_can_draw_overlays(struct _JavaVM *vm, void *context) +{ + jni::init(vm); + return Settings::canDrawOverlays(Context{(jobject)context}); +} diff --git a/src/xrt/auxiliary/android/android_custom_surface.h b/src/xrt/auxiliary/android/android_custom_surface.h index b57e6f839..b8a9089a4 100644 --- a/src/xrt/auxiliary/android/android_custom_surface.h +++ b/src/xrt/auxiliary/android/android_custom_surface.h @@ -93,6 +93,9 @@ android_custom_surface_get_display_metrics(struct _JavaVM *vm, void *activity, struct xrt_android_display_metrics *out_metrics); +bool +android_custom_surface_can_draw_overlays(struct _JavaVM *vm, void *context); + #ifdef __cplusplus } #endif diff --git a/src/xrt/compositor/main/comp_window_android.c b/src/xrt/compositor/main/comp_window_android.c index e3d96fe56..fff1cd420 100644 --- a/src/xrt/compositor/main/comp_window_android.c +++ b/src/xrt/compositor/main/comp_window_android.c @@ -89,7 +89,7 @@ _create_android_window(struct comp_window_android *cwa) { // 0 means default display cwa->custom_surface = - android_custom_surface_async_start(android_globals_get_vm(), android_globals_get_activity(), 0); + android_custom_surface_async_start(android_globals_get_vm(), android_globals_get_context(), 0); if (cwa->custom_surface == NULL) { COMP_ERROR(cwa->base.base.c, "comp_window_android_create_surface: could not " @@ -138,11 +138,14 @@ comp_window_android_init_swapchain(struct comp_target *ct, uint32_t width, uint3 if (android_globals_get_activity() != NULL) { /* In process: Creating surface from activity */ window = _create_android_window(cwa); + } else if (android_custom_surface_can_draw_overlays(android_globals_get_vm(), android_globals_get_context())) { + /* Out of process: Create surface */ + window = _create_android_window(cwa); } else { /* Out of process: Getting cached surface. * This loop polls for a surface created by Client.java in blockingConnect. - * TODO: change java code to callback native code to notify Session lifecycle progress, instead of - * polling here + * TODO: change java code to callback native code to notify Session lifecycle progress, instead + * of polling here */ for (int i = 0; i < 100; i++) { window = (struct ANativeWindow *)android_globals_get_window(); @@ -152,6 +155,7 @@ comp_window_android_init_swapchain(struct comp_target *ct, uint32_t width, uint3 } } + if (window == NULL) { COMP_ERROR(cwa->base.base.c, "could not get ANativeWindow"); return false; diff --git a/src/xrt/ipc/android/src/main/aidl/org/freedesktop/monado/ipc/IMonado.aidl b/src/xrt/ipc/android/src/main/aidl/org/freedesktop/monado/ipc/IMonado.aidl index e06a56a4a..4b5496053 100644 --- a/src/xrt/ipc/android/src/main/aidl/org/freedesktop/monado/ipc/IMonado.aidl +++ b/src/xrt/ipc/android/src/main/aidl/org/freedesktop/monado/ipc/IMonado.aidl @@ -23,11 +23,6 @@ interface IMonado { */ void passAppSurface(in Surface surface); - /*! - * Asking service to create surface and attach it to the display matches given display id. - */ - boolean createSurface(int displayId, boolean focusable); - /*! * Asking service whether it has the capbility to draw over other apps or not. */ diff --git a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Client.java b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Client.java index 380ab5db4..db8bf5984 100644 --- a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Client.java +++ b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Client.java @@ -22,7 +22,6 @@ import android.os.RemoteException; import android.util.Log; import android.view.Surface; import android.view.SurfaceHolder; -import android.view.WindowManager; import androidx.annotation.Keep; import androidx.annotation.Nullable; @@ -155,41 +154,26 @@ public class Client implements ServiceConnection { // (ready ... synchronized ... visible ... focused) new Thread( () -> { - boolean surfaceCreated = false; Activity activity = null; if (context_ instanceof Activity) { activity = (Activity) context_; } try { - // Determine whether runtime or client should create surface - if (monado.canDrawOverOtherApps()) { - WindowManager wm = - (WindowManager) - context_.getSystemService( - Context.WINDOW_SERVICE); - surfaceCreated = - monado.createSurface( - wm.getDefaultDisplay().getDisplayId(), false); - } else { - if (activity != null) { - Surface surface = attachViewAndGetSurface(activity); - surfaceCreated = (surface != null); - if (surfaceCreated) { - monado.passAppSurface(surface); - } + if (!monado.canDrawOverOtherApps() && activity != null) { + Surface surface = attachViewAndGetSurface(activity); + if (surface == null) { + Log.e(TAG, "Failed to create surface"); + handleFailure(); + return; } + + monado.passAppSurface(surface); } } catch (RemoteException e) { e.printStackTrace(); } - if (!surfaceCreated) { - Log.e(TAG, "Failed to create surface"); - handleFailure(); - return; - } - if (activity != null) { systemUiController = new SystemUiController(activity.getWindow().getDecorView()); diff --git a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java index 1803f0c80..aa5e2e9c8 100644 --- a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java +++ b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java @@ -9,11 +9,12 @@ package org.freedesktop.monado.ipc; +import android.content.Context; import android.os.ParcelFileDescriptor; import android.os.RemoteException; +import android.provider.Settings; import android.util.Log; import android.view.Surface; -import android.view.SurfaceHolder; import androidx.annotation.Keep; import androidx.annotation.NonNull; @@ -40,30 +41,15 @@ public class MonadoImpl extends IMonado.Stub { System.loadLibrary("monado-service"); } - private SurfaceManager surfaceManager; + private final Context context; - public MonadoImpl(@NonNull SurfaceManager surfaceManager) { - this.surfaceManager = surfaceManager; - this.surfaceManager.setCallback( - new SurfaceHolder.Callback() { - @Override - public void surfaceCreated(@NonNull SurfaceHolder holder) { - Log.i(TAG, "surfaceCreated"); - passAppSurface(holder.getSurface()); - } - - @Override - public void surfaceChanged( - @NonNull SurfaceHolder holder, int format, int width, int height) {} - - @Override - public void surfaceDestroyed(@NonNull SurfaceHolder holder) {} - }); + public MonadoImpl(@NonNull Context context) { + this.context = context.getApplicationContext(); + nativeStartServer(this.context); } @Override public void connect(@NonNull ParcelFileDescriptor parcelFileDescriptor) throws RemoteException { - nativeStartServer(); int fd = parcelFileDescriptor.getFd(); Log.i(TAG, "connect: given fd " + fd); if (nativeAddClient(fd) != 0) { @@ -90,19 +76,12 @@ public class MonadoImpl extends IMonado.Stub { return; } nativeAppSurface(surface); - nativeStartServer(); - } - - @Override - public boolean createSurface(int displayId, boolean focusable) { - Log.i(TAG, "createSurface"); - return surfaceManager.createSurfaceOnDisplay(displayId, focusable); } @Override public boolean canDrawOverOtherApps() { Log.i(TAG, "canDrawOverOtherApps"); - return surfaceManager.canDrawOverlays(); + return Settings.canDrawOverlays(context); } public void shutdown() { @@ -110,9 +89,17 @@ public class MonadoImpl extends IMonado.Stub { nativeShutdownServer(); } - /** Native method that starts server. */ + /** + * Native method that starts server. + * + *

This is essentially the entry point for the monado service on Android. + * + *

+ * + * @param context Context object. + */ @SuppressWarnings("JavaJniMissingFunction") - private native void nativeStartServer(); + private native void nativeStartServer(@NonNull Context context); /** * Native handling of receiving a surface: should convert it to an ANativeWindow then do stuff @@ -131,10 +118,6 @@ public class MonadoImpl extends IMonado.Stub { * Native handling of receiving an FD for a new client: the FD should be used to start up the * rest of the native IPC code on that socket. * - *

This is essentially the entry point for the monado service on Android: if it's already - * running, this will be called in it. If it's not already running, a process will be created, - * and this will be the first native code executed in that process. - * *

Ignore warnings that this function is missing: it is not, it is just in a different * module. See `src/xrt/targets/service-lib/service_target.cpp` for the implementation. * diff --git a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoService.kt b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoService.kt index 68e3c6a53..757c681c7 100644 --- a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoService.kt +++ b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoService.kt @@ -32,19 +32,15 @@ class MonadoService : Service(), Watchdog.ShutdownListener { @Inject lateinit var serviceNotification: IServiceNotification - private lateinit var surfaceManager: SurfaceManager - override fun onCreate() { super.onCreate() - surfaceManager = SurfaceManager(this) - binder = MonadoImpl(surfaceManager) + binder = MonadoImpl(this) watchdog = Watchdog( // If the surface comes from client, just stop the service when client disconnected // because the surface belongs to the client. - if (surfaceManager.canDrawOverlays()) BuildConfig.WATCHDOG_TIMEOUT_MILLISECONDS - else 0, + if (binder.canDrawOverOtherApps()) BuildConfig.WATCHDOG_TIMEOUT_MILLISECONDS else 0, this ) watchdog.startMonitor() @@ -61,7 +57,6 @@ class MonadoService : Service(), Watchdog.ShutdownListener { binder.shutdown() watchdog.stopMonitor() - surfaceManager.destroySurface() } override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { diff --git a/src/xrt/targets/openxr_android/src/main/java/org/freedesktop/monado/openxr_runtime/MonadoOpenXrApplication.java b/src/xrt/targets/openxr_android/src/main/java/org/freedesktop/monado/openxr_runtime/MonadoOpenXrApplication.java index fc8e8b2b3..43484f272 100644 --- a/src/xrt/targets/openxr_android/src/main/java/org/freedesktop/monado/openxr_runtime/MonadoOpenXrApplication.java +++ b/src/xrt/targets/openxr_android/src/main/java/org/freedesktop/monado/openxr_runtime/MonadoOpenXrApplication.java @@ -8,25 +8,9 @@ package org.freedesktop.monado.openxr_runtime; import android.app.Application; -import android.content.Context; - -import androidx.annotation.NonNull; import dagger.hilt.android.HiltAndroidApp; /** Subclass required for Hilt usage. */ @HiltAndroidApp -public class MonadoOpenXrApplication extends Application { - - @Override - public void onCreate() { - super.onCreate(); - - if (!BuildConfig.inProcess) { - System.loadLibrary("monado-service"); - nativeStoreContext(getApplicationContext()); - } - } - - private native void nativeStoreContext(@NonNull Context context); -} +public class MonadoOpenXrApplication extends Application {} diff --git a/src/xrt/targets/service-lib/service_target.cpp b/src/xrt/targets/service-lib/service_target.cpp index 9564c527b..1a27d6818 100644 --- a/src/xrt/targets/service-lib/service_target.cpp +++ b/src/xrt/targets/service-lib/service_target.cpp @@ -133,13 +133,20 @@ private: }; } // namespace -extern "C" void -Java_org_freedesktop_monado_ipc_MonadoImpl_nativeStartServer(JNIEnv *env, jobject thiz) +extern "C" JNIEXPORT void JNICALL +Java_org_freedesktop_monado_ipc_MonadoImpl_nativeStartServer(JNIEnv *env, jobject thiz, jobject context) { + JavaVM *jvm = nullptr; + jint result = env->GetJavaVM(&jvm); + assert(result == JNI_OK); + assert(jvm); + jni::init(env); jni::Object monadoImpl(thiz); U_LOG_D("service: Called nativeStartServer"); + android_globals_store_vm_and_context(jvm, context); + IpcServerHelper::instance().startServer(); } @@ -174,17 +181,3 @@ Java_org_freedesktop_monado_ipc_MonadoImpl_nativeShutdownServer(JNIEnv *env, job return IpcServerHelper::instance().shutdownServer(); } - -extern "C" JNIEXPORT void JNICALL -Java_org_freedesktop_monado_openxr_1runtime_MonadoOpenXrApplication_nativeStoreContext(JNIEnv *env, - jobject thiz, - jobject context) -{ - JavaVM *jvm = nullptr; - jint result = env->GetJavaVM(&jvm); - assert(result == JNI_OK); - assert(jvm); - - jni::init(env); - android_globals_store_vm_and_context(jvm, context); -}