From 378170d723c97f4608f1bef8f8b22fb3d1e293f8 Mon Sep 17 00:00:00 2001 From: Henrik Rydgard Date: Sat, 18 Mar 2017 13:01:08 +0100 Subject: [PATCH 1/3] Add ability to tag objects in the gl_lost_manager --- GPU/GLES/DrawEngineGLES.cpp | 5 ++--- UI/TextureUtil.h | 2 +- ext/native/gfx/gl_lost_manager.cpp | 27 ++++++++++++++++----------- ext/native/gfx/gl_lost_manager.h | 13 +++++++++++-- ext/native/gfx_es2/glsl_program.cpp | 4 ++-- ext/native/thin3d/thin3d_gl.cpp | 4 ++-- 6 files changed, 34 insertions(+), 21 deletions(-) diff --git a/GPU/GLES/DrawEngineGLES.cpp b/GPU/GLES/DrawEngineGLES.cpp index 49d51dbb51..60b344fb13 100644 --- a/GPU/GLES/DrawEngineGLES.cpp +++ b/GPU/GLES/DrawEngineGLES.cpp @@ -145,7 +145,7 @@ DrawEngineGLES::DrawEngineGLES() indexGen.Setup(decIndex); InitDeviceObjects(); - register_gl_resource_holder(this); + register_gl_resource_holder(this, "drawengine_gles"); tessDataTransfer = new TessellationDataTransferGLES(gl_extensions.VersionGEThan(3, 0, 0)); } @@ -200,7 +200,6 @@ void DrawEngineGLES::DestroyDeviceObjects() { bufferNameInfo_.clear(); freeSizedBuffers_.clear(); bufferNameCacheSize_ = 0; - if (sharedVao_ != 0) { glDeleteVertexArrays(1, &sharedVao_); } @@ -209,7 +208,7 @@ void DrawEngineGLES::DestroyDeviceObjects() { void DrawEngineGLES::GLLost() { ILOG("TransformDrawEngine::GLLost()"); - // The objects have already been deleted. + // The objects have already been deleted by losing the context, so we don't call DestroyDeviceObjects. bufferNameCache_.clear(); bufferNameInfo_.clear(); freeSizedBuffers_.clear(); diff --git a/UI/TextureUtil.h b/UI/TextureUtil.h index eaa5b14a20..df9414335c 100644 --- a/UI/TextureUtil.h +++ b/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); + register_gl_resource_holder(this, "managed_texture"); } ~ManagedTexture() { if (texture_) diff --git a/ext/native/gfx/gl_lost_manager.cpp b/ext/native/gfx/gl_lost_manager.cpp index 61e68f20dd..944ca5c144 100644 --- a/ext/native/gfx/gl_lost_manager.cpp +++ b/ext/native/gfx/gl_lost_manager.cpp @@ -4,18 +4,23 @@ #include "base/logging.h" #include "gfx/gl_lost_manager.h" -std::vector *holders; +struct Holder { + GfxResourceHolder *holder; + const char *desc; +}; + +std::vector *holders; static bool inLost; static bool inRestore; -void register_gl_resource_holder(GfxResourceHolder *holder) { +void register_gl_resource_holder(GfxResourceHolder *holder, const char *desc) { if (inLost || inRestore) { FLOG("BAD: Should not call register_gl_resource_holder from lost/restore path"); return; } if (holders) { - holders->push_back(holder); + holders->push_back({ holder, desc }); } else { WLOG("GL resource holder not initialized, cannot register resource"); } @@ -28,7 +33,7 @@ void unregister_gl_resource_holder(GfxResourceHolder *holder) { } if (holders) { for (size_t i = 0; i < holders->size(); i++) { - if ((*holders)[i] == holder) { + if ((*holders)[i].holder == holder) { holders->erase(holders->begin() + i); return; } @@ -47,12 +52,12 @@ void gl_restore() { return; } - ILOG("gl_restore() restoring %i items:", (int)holders->size()); + ILOG("gl_restore() restoring %d items:", (int)holders->size()); for (size_t i = 0; i < holders->size(); i++) { - ILOG("gl_restore(%i / %i, %p, %08x)", (int)(i + 1), (int)holders->size(), (*holders)[i], *((uint32_t *)((*holders)[i]))); - (*holders)[i]->GLRestore(); + ILOG("gl_restore(%d / %d, %s)", (int)(i + 1), (int)holders->size(), (*holders)[i].desc); + (*holders)[i].holder->GLRestore(); } - ILOG("gl_restore() completed on %i items:", (int)holders->size()); + ILOG("gl_restore() completed on %d items:", (int)holders->size()); inRestore = false; } @@ -66,8 +71,8 @@ void gl_lost() { ILOG("gl_lost() clearing %i items:", (int)holders->size()); for (size_t i = 0; i < holders->size(); i++) { - ILOG("gl_lost(%i / %i, %p, %08x)", (int)(i + 1), (int)holders->size(), (*holders)[i], *((uint32_t *)((*holders)[i]))); - (*holders)[i]->GLLost(); + ILOG("gl_lost(%d / %d, %s)", (int)(i + 1), (int)holders->size(), (*holders)[i].desc); + (*holders)[i].holder->GLLost(); } ILOG("gl_lost() completed on %i items:", (int)holders->size()); inLost = false; @@ -78,7 +83,7 @@ void gl_lost_manager_init() { FLOG("Double GL lost manager init"); // Dead here (FLOG), no need to delete holders } - holders = new std::vector(); + holders = new std::vector(); } void gl_lost_manager_shutdown() { diff --git a/ext/native/gfx/gl_lost_manager.h b/ext/native/gfx/gl_lost_manager.h index cfb09601a2..fc60609dd8 100644 --- a/ext/native/gfx/gl_lost_manager.h +++ b/ext/native/gfx/gl_lost_manager.h @@ -3,17 +3,26 @@ // On Android, even OpenGL can lose allocated resources. This is a utility to keep // track of them. +// It's important to realize that with OpenGL, there's no Lost event that can be relied upon. +// The only solid indication we get is onSurfaceCreated. That is called every time the graphics +// surface that we render upon has been recreated. When that's called, we know that any +// gl resources we owned before it was called have been killed and need to be recreated. + +// However, with D3D UWP, and potentially other platforms, there is a lost event. +// So we keep that infrastructure, but with GL we simply call both Lost and Restore when we detect a Restore. + class GfxResourceHolder { public: virtual ~GfxResourceHolder() {} - virtual void GLRestore() = 0; virtual void GLLost() = 0; + virtual void GLRestore() = 0; }; void gl_lost_manager_init(); void gl_lost_manager_shutdown(); -void register_gl_resource_holder(GfxResourceHolder *holder); +// 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 unregister_gl_resource_holder(GfxResourceHolder *holder); // Notifies all objects it's time to forget / delete things. diff --git a/ext/native/gfx_es2/glsl_program.cpp b/ext/native/gfx_es2/glsl_program.cpp index c0c08a7cd8..989e4e00eb 100644 --- a/ext/native/gfx_es2/glsl_program.cpp +++ b/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); + register_gl_resource_holder(program, "glsl_program"); 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); + register_gl_resource_holder(program, "glsl_program_src"); return program; } diff --git a/ext/native/thin3d/thin3d_gl.cpp b/ext/native/thin3d/thin3d_gl.cpp index 441e3e2b3c..e82b2ac6ee 100644 --- a/ext/native/thin3d/thin3d_gl.cpp +++ b/ext/native/thin3d/thin3d_gl.cpp @@ -369,7 +369,7 @@ class OpenGLPipeline : public Pipeline, GfxResourceHolder { public: OpenGLPipeline() { program_ = 0; - register_gl_resource_holder(this); + register_gl_resource_holder(this, "drawcontext_pipeline"); } ~OpenGLPipeline() { unregister_gl_resource_holder(this); @@ -890,7 +890,7 @@ public: totalSize_ = size; glBindBuffer(target_, buffer_); glBufferData(target_, size, NULL, usage_); - register_gl_resource_holder(this); + register_gl_resource_holder(this, "drawcontext_buffer"); } ~OpenGLBuffer() override { unregister_gl_resource_holder(this); From 85d0b89b7bbd197a1713479efeaf6179f2825fcc Mon Sep 17 00:00:00 2001 From: Henrik Rydgard Date: Sat, 18 Mar 2017 13:26:18 +0100 Subject: [PATCH 2/3] GL: Improve lost managment in DrawContext framebuffers --- ext/native/gfx/gl_lost_manager.h | 4 ++++ ext/native/thin3d/thin3d_gl.cpp | 41 +++++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/ext/native/gfx/gl_lost_manager.h b/ext/native/gfx/gl_lost_manager.h index fc60609dd8..5dda0b7d3b 100644 --- a/ext/native/gfx/gl_lost_manager.h +++ b/ext/native/gfx/gl_lost_manager.h @@ -11,6 +11,10 @@ // However, with D3D UWP, and potentially other platforms, there is a lost event. // So we keep that infrastructure, but with GL we simply call both Lost and Restore when we detect a Restore. +// For code simplicity, it may be a good idea to manually tear down and recreate everything. Even in this case, +// it's important to use this to zero out resource handles in GLLost() - gl_lost should be called before you +// tear things down, so then you can check if handles are zero and avoid deleting resources that are already gone. + class GfxResourceHolder { public: virtual ~GfxResourceHolder() {} diff --git a/ext/native/thin3d/thin3d_gl.cpp b/ext/native/thin3d/thin3d_gl.cpp index e82b2ac6ee..aa1cfa3ac7 100644 --- a/ext/native/thin3d/thin3d_gl.cpp +++ b/ext/native/thin3d/thin3d_gl.cpp @@ -1228,9 +1228,25 @@ void OpenGLInputLayout::Unapply() { } } -class OpenGLFramebuffer : public Framebuffer { +class OpenGLFramebuffer : public Framebuffer, public GfxResourceHolder { public: + OpenGLFramebuffer() { + register_gl_resource_holder(this, "framebuffer"); + } ~OpenGLFramebuffer(); + + void GLLost() override { + handle = 0; + color_texture = 0; + z_stencil_buffer = 0; + z_buffer = 0; + stencil_buffer = 0; + } + + void GLRestore() override { + ELOG("Restoring framebuffers not yet implemented"); + } + GLuint handle = 0; GLuint color_texture = 0; GLuint z_stencil_buffer = 0; // Either this is set, or the two below. @@ -1624,14 +1640,16 @@ void OpenGLContext::BindFramebufferAsTexture(Framebuffer *fbo, int binding, FBCh } OpenGLFramebuffer::~OpenGLFramebuffer() { + unregister_gl_resource_holder(this); CHECK_GL_ERROR_IF_DEBUG(); if (gl_extensions.ARB_framebuffer_object || gl_extensions.IsGLES) { - glBindFramebuffer(GL_FRAMEBUFFER, handle); - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0); - glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, 0); - glBindFramebuffer(GL_FRAMEBUFFER, 0); - if (handle) + if (handle) { + glBindFramebuffer(GL_FRAMEBUFFER, handle); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0); + glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, 0); + glBindFramebuffer(GL_FRAMEBUFFER, 0); glDeleteFramebuffers(1, &handle); + } if (z_stencil_buffer) glDeleteRenderbuffers(1, &z_stencil_buffer); if (z_buffer) @@ -1640,12 +1658,13 @@ OpenGLFramebuffer::~OpenGLFramebuffer() { glDeleteRenderbuffers(1, &stencil_buffer); } else if (gl_extensions.EXT_framebuffer_object) { #ifndef USING_GLES2 - glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, handle); - glFramebufferTexture2DEXT(GL_FRAMEBUFFER_EXT, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0); - glFramebufferRenderbufferEXT(GL_FRAMEBUFFER_EXT, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER_EXT, 0); - glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0); - if (handle) + if (handle) { + glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, handle); + glFramebufferTexture2DEXT(GL_FRAMEBUFFER_EXT, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0); + glFramebufferRenderbufferEXT(GL_FRAMEBUFFER_EXT, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER_EXT, 0); + glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0); glDeleteFramebuffersEXT(1, &handle); + } if (z_stencil_buffer) glDeleteRenderbuffers(1, &z_stencil_buffer); if (z_buffer) From 908193e894fd6b29ef270493f3e11c93daf92f14 Mon Sep 17 00:00:00 2001 From: Henrik Rydgard Date: Sat, 18 Mar 2017 15:20:36 +0100 Subject: [PATCH 3/3] Rework Android lost/restore lifecycle again. Can autorotate screen without crashing again. Should help #9295 and maybe #8906. --- GPU/GLES/DrawEngineGLES.cpp | 2 +- UI/NativeApp.cpp | 20 ++++--- UI/TextureUtil.h | 2 +- android/jni/app-android.cpp | 40 +++++++------- .../src/org/ppsspp/ppsspp/NativeActivity.java | 1 - .../src/org/ppsspp/ppsspp/NativeRenderer.java | 11 ++-- ext/native/gfx/gl_lost_manager.cpp | 31 ++++++++--- ext/native/gfx/gl_lost_manager.h | 2 +- ext/native/gfx_es2/glsl_program.cpp | 6 +-- ext/native/thin3d/thin3d_gl.cpp | 53 +++++++++++-------- 10 files changed, 98 insertions(+), 70 deletions(-) diff --git a/GPU/GLES/DrawEngineGLES.cpp b/GPU/GLES/DrawEngineGLES.cpp index 60b344fb13..4034684bc7 100644 --- a/GPU/GLES/DrawEngineGLES.cpp +++ b/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)); } diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index 5b05dd03f4..f9d6de3e7f 100644 --- a/UI/NativeApp.cpp +++ b/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() { diff --git a/UI/TextureUtil.h b/UI/TextureUtil.h index df9414335c..01e2bda11d 100644 --- a/UI/TextureUtil.h +++ b/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_) diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index 03ff9c72e8..239e687647 100644 --- a/android/jni/app-android.cpp +++ b/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: diff --git a/android/src/org/ppsspp/ppsspp/NativeActivity.java b/android/src/org/ppsspp/ppsspp/NativeActivity.java index b1d67da088..ad298c1aff 100644 --- a/android/src/org/ppsspp/ppsspp/NativeActivity.java +++ b/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; diff --git a/android/src/org/ppsspp/ppsspp/NativeRenderer.java b/android/src/org/ppsspp/ppsspp/NativeRenderer.java index ea2a021958..578116ff0c 100644 --- a/android/src/org/ppsspp/ppsspp/NativeRenderer.java +++ b/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(); -} \ No newline at end of file +} diff --git a/ext/native/gfx/gl_lost_manager.cpp b/ext/native/gfx/gl_lost_manager.cpp index 944ca5c144..3fb56f44b4 100644 --- a/ext/native/gfx/gl_lost_manager.cpp +++ b/ext/native/gfx/gl_lost_manager.cpp @@ -7,20 +7,24 @@ struct Holder { GfxResourceHolder *holder; const char *desc; + int priority; }; std::vector *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(); } diff --git a/ext/native/gfx/gl_lost_manager.h b/ext/native/gfx/gl_lost_manager.h index 5dda0b7d3b..15a7704fb3 100644 --- a/ext/native/gfx/gl_lost_manager.h +++ b/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. diff --git a/ext/native/gfx_es2/glsl_program.cpp b/ext/native/gfx_es2/glsl_program.cpp index 989e4e00eb..fadffff7b5 100644 --- a/ext/native/gfx_es2/glsl_program.cpp +++ b/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; -} \ No newline at end of file +} diff --git a/ext/native/thin3d/thin3d_gl.cpp b/ext/native/thin3d/thin3d_gl.cpp index aa1cfa3ac7..e7ebe9736e 100644 --- a/ext/native/thin3d/thin3d_gl.cpp +++ b/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();