From 4fb2e97afaaa2bd0b7d322545c79a65d8927ac8d Mon Sep 17 00:00:00 2001 From: Henrik Rydgard Date: Sun, 10 Mar 2013 17:49:01 +0100 Subject: [PATCH] Fix bugs causing infinite loop in gllost, add more logging. --- android/app-android.cpp | 40 ++++++++++++++----- .../henrikrydgard/libnative/NativeApp.java | 2 +- gfx/gl_lost_manager.cpp | 33 ++++++++++++--- gfx/texture.cpp | 18 ++++++--- gfx_es2/draw_buffer.cpp | 7 ++-- gfx_es2/draw_buffer.h | 2 +- 6 files changed, 75 insertions(+), 27 deletions(-) diff --git a/android/app-android.cpp b/android/app-android.cpp index 6dc9843454..6fdc010e8a 100644 --- a/android/app-android.cpp +++ b/android/app-android.cpp @@ -46,6 +46,8 @@ static JNIEnv *jniEnvUI; std::string frameCommand; std::string frameCommandParam; +const bool extraLog = true; + static uint32_t pad_buttons_async_set; static uint32_t pad_buttons_async_clear; @@ -120,6 +122,9 @@ extern "C" jboolean Java_com_henrikrydgard_libnative_NativeApp_isLandscape(JNIEn // For the Back button to work right. extern "C" jboolean Java_com_henrikrydgard_libnative_NativeApp_isAtTopLevel(JNIEnv *env, jclass) { + if (extraLog) { + ILOG("isAtTopLevel"); + } return NativeIsAtTopLevel(); } @@ -128,6 +133,8 @@ extern "C" void Java_com_henrikrydgard_libnative_NativeApp_init jstring jdataDir, jstring jexternalDir, jstring jlibraryDir, jstring jinstallID, jboolean juseNativeAudio) { jniEnvUI = env; + ILOG("NativeApp.init() -- begin"); + memset(&input_state, 0, sizeof(input_state)); renderer_inited = false; first_lost = true; @@ -136,7 +143,7 @@ extern "C" void Java_com_henrikrydgard_libnative_NativeApp_init pad_buttons_async_clear = 0; std::string apkPath = GetJavaString(env, japkpath); - ILOG("APK path: %s", apkPath.c_str()); + ILOG("NativeApp::Init: APK path: %s", apkPath.c_str()); VFSRegister("", new ZipAssetReader(apkPath.c_str(), "assets/")); std::string externalDir = GetJavaString(env, jexternalDir); @@ -144,7 +151,7 @@ extern "C" void Java_com_henrikrydgard_libnative_NativeApp_init std::string library_path = GetJavaString(env, jlibraryDir) + "/"; std::string installID = GetJavaString(env, jinstallID); - ILOG("External storage path: %s", externalDir.c_str()); + ILOG("NativeApp.init(): External storage path: %s", externalDir.c_str()); std::string app_name; std::string app_nice_name; @@ -167,24 +174,25 @@ extern "C" void Java_com_henrikrydgard_libnative_NativeApp_init if (use_native_audio) { AndroidAudio_Init(&NativeMix, library_path); } + ILOG("NativeApp.init() -- end"); } extern "C" void Java_com_henrikrydgard_libnative_NativeApp_resume(JNIEnv *, jclass) { - ILOG("NativeResume"); + ILOG("NativeApp.resume() - resuming audio"); if (use_native_audio) { AndroidAudio_Resume(); } } extern "C" void Java_com_henrikrydgard_libnative_NativeApp_pause(JNIEnv *, jclass) { - ILOG("NativePause"); + ILOG("NativeApp.pause() - pausing audio"); if (use_native_audio) { AndroidAudio_Pause(); } } extern "C" void Java_com_henrikrydgard_libnative_NativeApp_shutdown(JNIEnv *, jclass) { - ILOG("NativeShutdown."); + ILOG("NativeApp.shutdown() -- begin"); if (use_native_audio) { AndroidAudio_Shutdown(); } @@ -196,21 +204,21 @@ extern "C" void Java_com_henrikrydgard_libnative_NativeApp_shutdown(JNIEnv *, jc ILOG("VFSShutdown."); VFSShutdown(); net::Shutdown(); + ILOG("NativeApp.shutdown() -- end"); } static jmethodID postCommand; extern "C" void Java_com_henrikrydgard_libnative_NativeRenderer_displayInit(JNIEnv * env, jobject obj) { - ILOG("displayInit()"); + ILOG("NativeApp.displayInit()"); if (!renderer_inited) { - // We default to 240 dpi and all UI code is written to assume it. (DENSITY_HIGH, like Nexus S). // Note that we don't compute dp_xscale and dp_yscale until later! This is so that NativeGetAppInfo // can change the dp resolution if it feels like it. dp_xres = pixel_xres * g_dpi_scale; dp_yres = pixel_yres * g_dpi_scale; - ILOG("Calling NativeInitGraphics(); dpi = %i, dp_xres = %i, dp_yres = %i", g_dpi, dp_xres, dp_yres); + ILOG("Calling NativeInitGraphics(): dpi = %i, dp_xres = %i, dp_yres = %i", g_dpi, dp_xres, dp_yres); NativeInitGraphics(); dp_xscale = (float)dp_xres / pixel_xres; @@ -219,18 +227,24 @@ extern "C" void Java_com_henrikrydgard_libnative_NativeRenderer_displayInit(JNIE } else { ILOG("Calling NativeDeviceLost();"); NativeDeviceLost(); + ILOG("NativeDeviceLost completed.;"); } + ILOG("(Re)-fetching method ID to postCommand..."); jclass cls = env->GetObjectClass(obj); postCommand = env->GetMethodID(cls, "postCommand", "(Ljava/lang/String;Ljava/lang/String;)V"); ILOG("MethodID: %i", (int)postCommand); } extern "C" void Java_com_henrikrydgard_libnative_NativeRenderer_displayResize(JNIEnv *, jobject clazz, jint w, jint h) { - ILOG("displayResize (%i, %i)!", w, h); + ILOG("NativeApp.displayResize(%i, %i)", w, h); + // TODO: Move some of the logic from displayInit here? } extern "C" void Java_com_henrikrydgard_libnative_NativeRenderer_displayRender(JNIEnv *env, jobject obj) { + // Too spammy + // ILOG("NativeApp.displayRender()"); if (renderer_inited) { + // TODO: Look into if these locks are a perf loss { lock_guard guard(input_state.lock); input_state.pad_buttons |= pad_buttons_async_set; @@ -247,11 +261,12 @@ extern "C" void Java_com_henrikrydgard_libnative_NativeRenderer_displayRender(JN lock_guard guard(input_state.lock); EndInputState(&input_state); } + NativeRender(); time_update(); } else { - ELOG("Ended up in nativeRender even though app has quit.%s", ""); - // Shouldn't really get here. + ELOG("BAD: Ended up in nativeRender even though app has quit.%s", ""); + // Shouldn't really get here. Let's draw magenta. glstate.depthWrite.set(GL_TRUE); glstate.colorMask.set(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE); glClearColor(1.0, 0.0, 1.0f, 1.0f); @@ -271,6 +286,9 @@ extern "C" void Java_com_henrikrydgard_libnative_NativeRenderer_displayRender(JN } extern "C" void Java_com_henrikrydgard_libnative_NativeApp_audioRender(JNIEnv* env, jclass clazz, jshortArray array) { + // Too spammy + // ILOG("NativeApp.audioRender"); + // The audio thread can pretty safely enable Flush-to-Zero mode on the FPU. EnableFZ(); diff --git a/android/src/com/henrikrydgard/libnative/NativeApp.java b/android/src/com/henrikrydgard/libnative/NativeApp.java index 9c016f0ada..7c119ae96a 100644 --- a/android/src/com/henrikrydgard/libnative/NativeApp.java +++ b/android/src/com/henrikrydgard/libnative/NativeApp.java @@ -10,7 +10,7 @@ public class NativeApp { // These have Android semantics: Resume is always called on bootup, after init public static native void pause(); - public static native void resume(); + public static native void resume(); public static native void shutdown(); diff --git a/gfx/gl_lost_manager.cpp b/gfx/gl_lost_manager.cpp index 99a9776db9..1c2ed98262 100644 --- a/gfx/gl_lost_manager.cpp +++ b/gfx/gl_lost_manager.cpp @@ -1,13 +1,19 @@ -#include +#include #include "base/logging.h" #include "gfx/gl_lost_manager.h" -std::list *holders; +std::vector *holders; GfxResourceHolder::~GfxResourceHolder() {} +static bool inLost; + void register_gl_resource_holder(GfxResourceHolder *holder) { + if (inLost) { + FLOG("BAD: Should not call register_gl_resource_holder from lost path"); + return; + } if (holders) { holders->push_back(holder); } else { @@ -15,28 +21,43 @@ void register_gl_resource_holder(GfxResourceHolder *holder) { } } void unregister_gl_resource_holder(GfxResourceHolder *holder) { + if (inLost) { + FLOG("BAD: Should not call unregister_gl_resource_holder from lost path"); + return; + } if (holders) { - holders->remove(holder); + for (size_t i = 0; i < holders->size(); i++) { + if ((*holders)[i] == holder) { + holders->erase(holders->begin() + i); + return; + } + } + WLOG("unregister_gl_resource_holder: Resource not registered"); } else { WLOG("GL resource holder not initialized or already shutdown, cannot unregister resource"); } } void gl_lost() { + inLost = true; if (!holders) { WLOG("GL resource holder not initialized, cannot process lost request"); return; } - for (std::list::iterator iter = holders->begin(); iter != holders->end(); ++iter) { - (*iter)->GLLost(); + ILOG("gl_lost() restoring %i items:", (int)holders->size()); + for (size_t i = 0; i < holders->size(); i++) { + ILOG("GLLost(%i, %p)", i, holders->size(), (*holders)[i]); + (*holders)[i]->GLLost(); } + ILOG("gl_lost() completed restoring %i items:", (int)holders->size()); + inLost = false; } void gl_lost_manager_init() { if (holders) { FLOG("Double GL lost manager init"); } - holders = new std::list(); + holders = new std::vector(); } void gl_lost_manager_shutdown() { diff --git a/gfx/texture.cpp b/gfx/texture.cpp index 79b9736926..77e5c353f4 100644 --- a/gfx/texture.cpp +++ b/gfx/texture.cpp @@ -29,6 +29,7 @@ void Texture::Destroy() { void Texture::GLLost() { ILOG("Reloading lost texture %s", filename_.c_str()); Load(filename_.c_str()); + ILOG("Reloaded lost texture %s", filename_.c_str()); } Texture::~Texture() { @@ -87,8 +88,9 @@ bool Texture::Load(const char *filename) { // They shouldn't really hurt anything else very much though. int len = strlen(filename); - char fn[256]; - strcpy(fn, filename); + char fn[1024]; + strncpy(fn, filename, sizeof(fn)); + fn[1023] = 0; bool zim = false; if (!strcmp("dds", &filename[len-3])) { strcpy(&fn[len-3], "zim"); @@ -119,11 +121,11 @@ bool Texture::Load(const char *filename) { } else #endif if (!strcmp("zim", &name[len-3])) { - if (!LoadZIM(name)) { + if (LoadZIM(name)) { + return true; + } else { LoadXOR(); return false; - } else { - return true; } } LoadXOR(); @@ -197,8 +199,11 @@ bool Texture::LoadZIM(const char *filename) { int flags; int num_levels = ::LoadZIM(filename, &width[0], &height[0], &flags, &image_data[0]); + ILOG("ZIM loaded: %i levels", num_levels); if (!num_levels) return false; + if (num_levels >= ZIM_MAX_MIP_LEVELS) + return false; width_ = width[0]; height_ = height[0]; int data_type = GL_UNSIGNED_BYTE; @@ -223,6 +228,9 @@ bool Texture::LoadZIM(const char *filename) { } GL_CHECK(); + + + ILOG("Gen-binding texture"); glGenTextures(1, &id_); glBindTexture(GL_TEXTURE_2D, id_); SetTextureParameters(flags); diff --git a/gfx_es2/draw_buffer.cpp b/gfx_es2/draw_buffer.cpp index 6cef2e0bcd..6e2545e001 100644 --- a/gfx_es2/draw_buffer.cpp +++ b/gfx_es2/draw_buffer.cpp @@ -32,12 +32,13 @@ DrawBuffer::~DrawBuffer() { delete [] verts_; } -void DrawBuffer::Init() { +void DrawBuffer::Init(bool registerAsHolder) { if (inited_) return; inited_ = true; glGenBuffers(1, (GLuint *)&vbo_); - register_gl_resource_holder(this); + if (registerAsHolder) + register_gl_resource_holder(this); } void DrawBuffer::Shutdown() { @@ -49,7 +50,7 @@ void DrawBuffer::Shutdown() { void DrawBuffer::GLLost() { inited_ = false; - Init(); + Init(false); } void DrawBuffer::Begin(DrawBufferMode dbmode) { diff --git a/gfx_es2/draw_buffer.h b/gfx_es2/draw_buffer.h index 1ec42a847f..3ba0e7f001 100644 --- a/gfx_es2/draw_buffer.h +++ b/gfx_es2/draw_buffer.h @@ -51,7 +51,7 @@ public: void End(); // TODO: Enforce these. Now Init is autocalled and shutdown not called. - void Init(); + void Init(bool registerAsHolder = true); void Shutdown(); virtual void GLLost();