From 8622471277e872e3cd962045b3e09a87e797a517 Mon Sep 17 00:00:00 2001
From: baldurk <baldurk@baldurk.org>
Date: Fri, 9 Feb 2018 14:02:31 +0000
Subject: [PATCH] Reset android vulkan layer property as late as possible

* We can't reliably know when we can safely remove the layer property
  at capture time because it's impossible to know if an android app will
  use vulkan or not. So instead we fall back to unsetting it whenever we
  need to - before opening a capture or when shutting down the remote
  server (in case after capturing, no frame capture was actually opened)
* This means that if the UI crashes without unsetting it, we will poison
  the state, but this seems unavoidable.
---
 renderdoc/android/android.cpp    | 32 ++++++++++----------------------
 renderdoc/android/android.h      |  1 +
 renderdoc/core/remote_server.cpp | 20 +++++++++++++++++++-
 3 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/renderdoc/android/android.cpp b/renderdoc/android/android.cpp
index fe125b2e8..2d9483500 100644
--- a/renderdoc/android/android.cpp
+++ b/renderdoc/android/android.cpp
@@ -218,25 +218,8 @@ uint32_t StartAndroidPackageForCapture(const char *host, const char *package)
     ITargetControl *control = RENDERDOC_CreateTargetControl(host, ret, "testConnection", false);
     if(control)
     {
-      std::string api;
-
-      // allow a few messages to come in, to see the reported active API
-      for(int i = 0; i < 4; i++)
-      {
-        Threading::Sleep(5);
-        control->ReceiveMessage();
-
-        api = control->GetAPI();
-        if(!api.empty())
-          break;
-      }
-
       control->Shutdown();
-
-      if(!api.empty())
-        break;
-      else
-        RDCDEBUG("Connection established, but no API initialised yet. Waiting...");
+      break;
     }
 
     // check to see if the PID is still there. If it was before and isn't now, the APK has exited
@@ -253,9 +236,9 @@ uint32_t StartAndroidPackageForCapture(const char *host, const char *package)
     elapsed += 1000;
   }
 
-  // We've ensured above that the app picked up the setprop before we turn it back off for
-  // replaying.
-  adbExecCommand(deviceID, "shell setprop debug.vulkan.layers :");
+  // we leave the setprop in case the application later initialises a vulkan device. It's impossible
+  // to tell if it will or not, since many applications will init and present from GLES and then
+  // later use vulkan.
 
   return ret;
 }
@@ -437,6 +420,11 @@ bool CheckAndroidServerVersion(const string &deviceID)
 
   return false;
 }
+
+void ResetCaptureSettings(const std::string &deviceID)
+{
+  Android::adbExecCommand(deviceID, "shell setprop debug.vulkan.layers :");
+}
 };    // namespace Android
 
 extern "C" RENDERDOC_API void RENDERDOC_CC RENDERDOC_GetAndroidFriendlyName(const rdcstr &device,
@@ -523,7 +511,7 @@ extern "C" RENDERDOC_API void RENDERDOC_CC RENDERDOC_StartAndroidRemoteServer(co
     return;
 
   Android::adbForwardPorts(index, deviceID, 0, 0);
-  Android::adbExecCommand(deviceID, "shell setprop debug.vulkan.layers :");
+  Android::ResetCaptureSettings(deviceID);
 
   // launch the first ABI, as the default 'most compatible' package
   Android::adbExecCommand(deviceID, "shell am start -n " + GetRenderDocPackageForABI(abis[0]) +
diff --git a/renderdoc/android/android.h b/renderdoc/android/android.h
index ff0dc0ad7..fb534ea98 100644
--- a/renderdoc/android/android.h
+++ b/renderdoc/android/android.h
@@ -32,6 +32,7 @@ namespace Android
 {
 bool IsHostADB(const char *hostname);
 uint32_t StartAndroidPackageForCapture(const char *host, const char *package);
+void ResetCaptureSettings(const std::string &deviceID);
 void ExtractDeviceIDAndIndex(const std::string &hostname, int &index, std::string &deviceID);
 Process::ProcessResult adbExecCommand(const std::string &deviceID, const std::string &args,
                                       const string &workDir = ".", bool silent = false);
diff --git a/renderdoc/core/remote_server.cpp b/renderdoc/core/remote_server.cpp
index 137e4a086..71cd2f39f 100644
--- a/renderdoc/core/remote_server.cpp
+++ b/renderdoc/core/remote_server.cpp
@@ -1020,9 +1020,14 @@ public:
   }
   const std::string &hostname() const { return m_hostname; }
   virtual ~RemoteServer() { SAFE_DELETE(m_Socket); }
-  void ShutdownConnection() { delete this; }
+  void ShutdownConnection()
+  {
+    ResetAndroidSettings();
+    delete this;
+  }
   void ShutdownServerAndConnection()
   {
+    ResetAndroidSettings();
     {
       WRITE_DATA_SCOPE();
       SCOPED_SERIALISE_CHUNK(eRemoteServer_ShutdownServer);
@@ -1360,6 +1365,8 @@ public:
     ret.first = ReplayStatus::InternalError;
     ret.second = NULL;
 
+    ResetAndroidSettings();
+
     if(proxyid != ~0U && proxyid >= m_Proxies.size())
     {
       RDCERR("Invalid proxy driver id %d specified for remote renderer", proxyid);
@@ -1453,6 +1460,17 @@ public:
     return ret;
   }
 
+  void ResetAndroidSettings()
+  {
+    if(Android::IsHostADB(m_hostname.c_str()))
+    {
+      int index = 0;
+      std::string deviceID;
+      Android::ExtractDeviceIDAndIndex(m_hostname.c_str(), index, deviceID);
+      Android::ResetCaptureSettings(deviceID);
+    }
+  }
+
   void CloseCapture(IReplayController *rend)
   {
     {