Rework Android lost/restore lifecycle again. Can autorotate screen without crashing again. Should help #9295 and maybe #8906.

GPU/GLES/DrawEngineGLES.cpp

@@ -145,7 +145,7 @@ DrawEngineGLES::DrawEngineGLES()
 	indexGen.Setup(decIndex);
 
 	InitDeviceObjects();
-	register_gl_resource_holder(this, "drawengine_gles");
+	register_gl_resource_holder(this, "drawengine_gles", 1);
 
 	tessDataTransfer = new TessellationDataTransferGLES(gl_extensions.VersionGEThan(3, 0, 0));
 }

UI/NativeApp.cpp

@@ -534,6 +534,8 @@ void NativeInit(int argc, const char *argv[], const char *savegame_dir, const ch
 }
 
 void NativeInitGraphics(GraphicsContext *graphicsContext) {
+	ILOG("NativeInitGraphics");
+
 	using namespace Draw;
 	Core_SetGraphicsContext(graphicsContext);
 	g_draw = graphicsContext->GetDrawContext();
@@ -640,16 +642,17 @@ void NativeInitGraphics(GraphicsContext *graphicsContext) {
 #endif
 
 	g_gameInfoCache = new GameInfoCache();
+	ILOG("NativeInitGraphics completed");
 }
 
 void NativeShutdownGraphics() {
+	ILOG("NativeShutdownGraphics");
+
 #ifdef _WIN32
 	delete winAudioBackend;
 	winAudioBackend = NULL;
 #endif
 
-	screenManager->deviceLost();
-
 	delete g_gameInfoCache;
 	g_gameInfoCache = nullptr;
 
@@ -664,6 +667,8 @@ void NativeShutdownGraphics() {
 
 	colorPipeline->Release();
 	texColorPipeline->Release();
+
+	ILOG("NativeShutdownGraphics done");
 }
 
 void TakeScreenshot() {
@@ -873,20 +878,21 @@ void NativeUpdate() {
 }
 
 void NativeDeviceLost() {
-	if (g_gameInfoCache)
-		g_gameInfoCache->Clear();
-	screenManager->deviceLost();
+	// We start by calling gl_lost - this lets objects zero their native GL objects
+	// so they then don't try to delete them as well.
 	if (GetGPUBackend() == GPUBackend::OPENGL) {
 		gl_lost();
 	}
+	if (g_gameInfoCache)
+		g_gameInfoCache->Clear();
+	screenManager->deviceLost();
 }
 
 void NativeDeviceRestore() {
-	NativeDeviceLost();
-	screenManager->deviceRestore();
 	if (GetGPUBackend() == GPUBackend::OPENGL) {
 		gl_restore();
 	}
+	screenManager->deviceRestore();
 }
 
 bool NativeIsAtTopLevel() {

UI/TextureUtil.h

@@ -16,7 +16,7 @@ class ManagedTexture : public GfxResourceHolder {
 public:
 	ManagedTexture(Draw::DrawContext *draw) : draw_(draw), texture_(nullptr) {
 		if (g_Config.iGPUBackend == (int)GPUBackend::OPENGL)
-			register_gl_resource_holder(this, "managed_texture");
+			register_gl_resource_holder(this, "managed_texture", 0);
 	}
 	~ManagedTexture() {
 		if (texture_)

android/jni/app-android.cpp

@@ -1,5 +1,5 @@
 // This is generic code that is included in all Android apps that use the
-// Native framework by Henrik Rydgård (https://github.com/hrydgard/native).
+// Native framework by Henrik Rydgard (https://github.com/hrydgard/native).
 
 // It calls a set of methods defined in NativeApp.h. These should be implemented
 // by your game or app.
@@ -602,6 +602,14 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_pause(JNIEnv *, jclass) {
 
 extern "C" void Java_org_ppsspp_ppsspp_NativeApp_shutdown(JNIEnv *, jclass) {
 	ILOG("NativeApp.shutdown() -- begin");
+	if (renderer_inited) {
+		NativeShutdownGraphics();
+		graphicsContext->Shutdown();
+		delete graphicsContext;
+		graphicsContext = nullptr;
+		renderer_inited = false;
+	}
+
 	NativeShutdown();
 	VFSShutdown();
 	ILOG("NativeApp.shutdown() -- end");
@@ -612,16 +620,23 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeRenderer_displayInit(JNIEnv * env,
 	if (javaGL && !graphicsContext) {
 		graphicsContext = new AndroidJavaEGLGraphicsContext();
 	}
-	ILOG("NativeApp.displayInit() (graphicsContext=%p)", graphicsContext);
 
-	if (!renderer_inited) {
+	if (renderer_inited) {
+		ILOG("NativeApp.displayInit() restoring");
+		NativeDeviceLost();
+		NativeShutdownGraphics();
+		NativeDeviceRestore();
+		NativeInitGraphics(graphicsContext);
+
+		ILOG("Restored.");
+	} else {
+		ILOG("NativeApp.displayInit() first time");
 		NativeInitGraphics(graphicsContext);
 		renderer_inited = true;
 		renderer_ever_inited = true;
-	} else {
-		NativeDeviceRestore();
-		ILOG("displayInit: NativeDeviceRestore completed.");
 	}
+
+	NativeMessageReceived("recreateviews", "");
 }
 
 // JavaEGL
@@ -695,19 +710,6 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeRenderer_displayRender(JNIEnv *env,
 	ProcessFrameCommands(env);
 }
 
-extern "C" void Java_org_ppsspp_ppsspp_NativeRenderer_displayShutdown(JNIEnv *env, jobject obj) {
-	ILOG("NativeApp.displayShutdown()");
-	if (renderer_inited) {
-		NativeDeviceLost();
-		NativeShutdownGraphics();
-		graphicsContext->Shutdown();
-		delete graphicsContext;
-		graphicsContext = nullptr;
-		renderer_inited = false;
-		NativeMessageReceived("recreateviews", "");
-	}
-}
-
 void System_AskForPermission(SystemPermission permission) {
 	switch (permission) {
 	case SYSTEM_PERMISSION_STORAGE:

android/src/org/ppsspp/ppsspp/NativeActivity.java

@@ -589,7 +589,6 @@ public class NativeActivity extends Activity implements SurfaceHolder.Callback {
 		if (javaGL) {
 			Log.i(TAG, "onDestroy");
 			mGLSurfaceView.onDestroy();
-			nativeRenderer.onDestroyed();
 			NativeApp.audioShutdown();
 			// Probably vain attempt to help the garbage collector...
 			mGLSurfaceView = null;

android/src/org/ppsspp/ppsspp/NativeRenderer.java

@@ -59,6 +59,7 @@ public class NativeRenderer implements GLSurfaceView.Renderer {
 	}
 
 	public void onSurfaceCreated(GL10 unused, EGLConfig config) {
+		Log.i(TAG, "onSurfaceCreated");
 		// Log.i(TAG, "onSurfaceCreated - EGL context is new or was lost");
 		// Actually, it seems that it is here we should recreate lost GL objects.
 		displayInit();
@@ -71,18 +72,13 @@ public class NativeRenderer implements GLSurfaceView.Renderer {
 		double actualH = sz.y;
 		dpi_scale_x = ((double)width / (double)actualW);
 		dpi_scale_y = ((double)height / (double)actualH);
-		Log.i(TAG, "onSurfaceChanged: " + dpi_scale_x + "x" + dpi_scale_y + " (width=" + width + ", actualW=" + actualW);
+		Log.i(TAG, "onSurfaceChanged: Scale: " + dpi_scale_x + "x" + dpi_scale_y + " (width=" + width + ", actualW=" + actualW);
 		int scaled_dpi = (int)((double)dpi * dpi_scale_x);
 		displayResize(width, height, scaled_dpi, refreshRate);
 		last_width = width;
 		last_height = height;
 	}
 
-	// Not override, it's custom.
-	public void onDestroyed() {
-		displayShutdown();
-	}
-	
 	// NATIVE METHODS
 
 	// Note: This also means "device lost" and you should reload
@@ -90,5 +86,4 @@ public class NativeRenderer implements GLSurfaceView.Renderer {
 	public native void displayInit(); 
 	public native void displayResize(int w, int h, int dpi, float refreshRate);
 	public native void displayRender();
-	public native void displayShutdown();
-}
+}

ext/native/gfx/gl_lost_manager.cpp

@@ -7,20 +7,24 @@
 struct Holder {
 	GfxResourceHolder *holder;
 	const char *desc;
+	int priority;
 };
 
 std::vector<Holder> *holders;
 
 static bool inLost;
 static bool inRestore;
+static int g_max_priority = 0;
 
-void register_gl_resource_holder(GfxResourceHolder *holder, const char *desc) {
+void register_gl_resource_holder(GfxResourceHolder *holder, const char *desc, int priority) {
 	if (inLost || inRestore) {
 		FLOG("BAD: Should not call register_gl_resource_holder from lost/restore path");
 		return;
 	}
 	if (holders) {
-		holders->push_back({ holder, desc });
+		holders->push_back({ holder, desc, priority });
+		if (g_max_priority < priority)
+			g_max_priority = priority;
 	} else {
 		WLOG("GL resource holder not initialized, cannot register resource");
 	}
@@ -53,9 +57,14 @@ void gl_restore() {
 	}
 
 	ILOG("gl_restore() restoring %d items:", (int)holders->size());
-	for (size_t i = 0; i < holders->size(); i++) {
-		ILOG("gl_restore(%d / %d, %s)", (int)(i + 1), (int)holders->size(), (*holders)[i].desc);
-		(*holders)[i].holder->GLRestore();
+	for (int p = 0; p <= g_max_priority; p++) {
+		for (size_t i = 0; i < holders->size(); i++) {
+			if ((*holders)[i].priority == p) {
+				ILOG("GLRestore(%d / %d, %s, prio %d)", (int)(i + 1), (int)holders->size(),
+					 (*holders)[i].desc, (*holders)[i].priority);
+				(*holders)[i].holder->GLRestore();
+			}
+		}
 	}
 	ILOG("gl_restore() completed on %d items:", (int)holders->size());
 	inRestore = false;
@@ -70,9 +79,14 @@ void gl_lost() {
 	}
 
 	ILOG("gl_lost() clearing %i items:", (int)holders->size());
-	for (size_t i = 0; i < holders->size(); i++) {
-		ILOG("gl_lost(%d / %d, %s)", (int)(i + 1), (int)holders->size(), (*holders)[i].desc);
-		(*holders)[i].holder->GLLost();
+	for (int p = g_max_priority; p >= 0; p--) {
+		for (size_t i = 0; i < holders->size(); i++) {
+			if ((*holders)[i].priority == p) {
+				ILOG("gl_lost(%d / %d, %s, prio %d)", (int) (i + 1), (int) holders->size(),
+					 (*holders)[i].desc, (*holders)[i].priority);
+				(*holders)[i].holder->GLLost();
+			}
+		}
 	}
 	ILOG("gl_lost() completed on %i items:", (int)holders->size());
 	inLost = false;
@@ -83,6 +97,7 @@ void gl_lost_manager_init() {
 		FLOG("Double GL lost manager init");
 		// Dead here (FLOG), no need to delete holders
 	}
+	g_max_priority = 0;
 	holders = new std::vector<Holder>();
 }
 

ext/native/gfx/gl_lost_manager.h

@@ -26,7 +26,7 @@ void gl_lost_manager_init();
 void gl_lost_manager_shutdown();
 
 // The string pointed to by desc must be a constant or otherwise live for the entire registered lifetime of the object.
-void register_gl_resource_holder(GfxResourceHolder *holder, const char *desc);
+void register_gl_resource_holder(GfxResourceHolder *holder, const char *desc, int priority);
 void unregister_gl_resource_holder(GfxResourceHolder *holder);
 
 // Notifies all objects it's time to forget / delete things.

ext/native/gfx_es2/glsl_program.cpp

@@ -49,7 +49,7 @@ GLSLProgram *glsl_create(const char *vshader, const char *fshader, std::string *
 		delete program;
 		return 0;
 	}
-	register_gl_resource_holder(program, "glsl_program");
+	register_gl_resource_holder(program, "glsl_program", 0);
 	return program;
 }
 
@@ -70,7 +70,7 @@ GLSLProgram *glsl_create_source(const char *vshader_src, const char *fshader_src
 		delete program;
 		return 0;
 	}
-	register_gl_resource_holder(program, "glsl_program_src");
+	register_gl_resource_holder(program, "glsl_program_src", 0);
 	return program;
 }
 
@@ -279,4 +279,4 @@ void glsl_unbind() {
 
 const GLSLProgram *glsl_get_program() {
 	return curProgram;
-}
+}

ext/native/thin3d/thin3d_gl.cpp

@@ -271,16 +271,18 @@ GLuint ShaderStageToOpenGL(ShaderStage stage) {
 	}
 }
 
-// Not registering this as a resource holder, instead Pipeline is registered. It will
-// invoke Compile again to recreate the shader then link them together.
-class OpenGLShaderModule : public ShaderModule {
+class OpenGLShaderModule : public ShaderModule, public GfxResourceHolder {
 public:
-	OpenGLShaderModule(ShaderStage stage) : stage_(stage), shader_(0) {
+	OpenGLShaderModule(ShaderStage stage) : stage_(stage) {
+		register_gl_resource_holder(this, "drawcontext_shader_module", 0);
 		glstage_ = ShaderStageToOpenGL(stage);
 	}
 
 	~OpenGLShaderModule() {
-		glDeleteShader(shader_);
+		ILOG("Shader module destroyed");
+		if (shader_)
+			glDeleteShader(shader_);
+		unregister_gl_resource_holder(this);
 	}
 
 	bool Compile(ShaderLanguage language, const uint8_t *data, size_t dataSize);
@@ -289,9 +291,6 @@ public:
 	}
 	const std::string &GetSource() const { return source_; }
 
-	void Unset() {
-		shader_ = 0;
-	}
 	ShaderLanguage GetLanguage() {
 		return language_;
 	}
@@ -299,12 +298,25 @@ public:
 		return stage_;
 	}
 
+	void GLLost() override {
+		ILOG("Shader module lost");
+		// Shader has been destroyed since the old context is gone, so let's zero it.
+		shader_ = 0;
+	}
+
+	void GLRestore() override {
+		ILOG("Shader module being restored");
+		if (!Compile(language_, (const uint8_t *)source_.data(), source_.size())) {
+			ELOG("Shader restore compilation failed: %s", source_.c_str());
+		}
+	}
+
 private:
 	ShaderStage stage_;
 	ShaderLanguage language_;
-	GLuint shader_;
-	GLuint glstage_;
-	bool ok_;
+	GLuint shader_ = 0;
+	GLuint glstage_ = 0;
+	bool ok_ = false;
 	std::string source_;  // So we can recompile in case of context loss.
 };
 
@@ -369,11 +381,12 @@ class OpenGLPipeline : public Pipeline, GfxResourceHolder {
 public:
 	OpenGLPipeline() {
 		program_ = 0;
-		register_gl_resource_holder(this, "drawcontext_pipeline");
+		// Priority 1 so this gets restored after the shaders.
+		register_gl_resource_holder(this, "drawcontext_pipeline", 1);
 	}
 	~OpenGLPipeline() {
 		unregister_gl_resource_holder(this);
-		for (auto iter : shaders) {
+		for (auto &iter : shaders) {
 			iter->Release();
 		}
 		glDeleteProgram(program_);
@@ -389,15 +402,10 @@ public:
 
 	void GLLost() override {
 		program_ = 0;
-		for (auto iter : shaders) {
-			iter->Unset();
-		}
 	}
 
 	void GLRestore() override {
-		for (auto iter : shaders) {
-			iter->Compile(iter->GetLanguage(), (const uint8_t *)iter->GetSource().c_str(), iter->GetSource().size());
-		}
+		// Shaders will have been restored before the pipeline.
 		LinkShaders();
 	}
 
@@ -890,7 +898,7 @@ public:
 		totalSize_ = size;
 		glBindBuffer(target_, buffer_);
 		glBufferData(target_, size, NULL, usage_);
-		register_gl_resource_holder(this, "drawcontext_buffer");
+		register_gl_resource_holder(this, "drawcontext_buffer", 0);
 	}
 	~OpenGLBuffer() override {
 		unregister_gl_resource_holder(this);
@@ -959,6 +967,7 @@ Pipeline *OpenGLContext::CreateGraphicsPipeline(const PipelineDesc &desc) {
 			pipeline->dynamicUniforms = *desc.uniformDesc;
 		return pipeline;
 	} else {
+		ELOG("Failed to create pipeline - shaders failed to link");
 		delete pipeline;
 		return NULL;
 	}
@@ -1048,6 +1057,8 @@ bool OpenGLPipeline::LinkShaders() {
 			OutputDebugStringUTF8(buf);
 #endif
 			delete[] buf;
+		} else {
+			ELOG("Could not link program with %d shaders for unknown reason:", (int)shaders.size());
 		}
 		return false;
 	}
@@ -1231,7 +1242,7 @@ void OpenGLInputLayout::Unapply() {
 class OpenGLFramebuffer : public Framebuffer, public GfxResourceHolder {
 public:
 	OpenGLFramebuffer() {
-		register_gl_resource_holder(this, "framebuffer");
+		register_gl_resource_holder(this, "framebuffer", 0);
 	}
 	~OpenGLFramebuffer();