From 6927c44fba6e4fa9d892563b8131a515ea50c71d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 22 Dec 2022 11:38:36 +0100 Subject: [PATCH 1/5] Remove unused functions, log and comment fixes --- android/jni/app-android.cpp | 24 +++++-------------- .../org/ppsspp/ppsspp/InputDeviceState.java | 2 -- android/src/org/ppsspp/ppsspp/NativeApp.java | 2 -- .../src/org/ppsspp/ppsspp/NativeGLView.java | 2 +- .../src/org/ppsspp/ppsspp/NativeRenderer.java | 3 ++- .../src/org/ppsspp/ppsspp/PpssppActivity.java | 2 -- 6 files changed, 9 insertions(+), 26 deletions(-) diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index c0ddad6ff9..93ddb99cde 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -846,7 +846,6 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_shutdown(JNIEnv *, jclass) { INFO_LOG(SYSTEM, "NativeApp.shutdown() -- begin"); if (renderer_inited) { INFO_LOG(G3D, "Shutting down renderer"); - // This will be from the wrong thread? :/ graphicsContext->Shutdown(); delete graphicsContext; graphicsContext = nullptr; @@ -947,10 +946,8 @@ static void recalculateDpi() { pixel_in_dps_x = (float)pixel_xres / dp_xres; pixel_in_dps_y = (float)pixel_yres / dp_yres; - INFO_LOG(G3D, "RecalcDPI: display_xres=%d display_yres=%d", display_xres, display_yres); - INFO_LOG(G3D, "RecalcDPI: g_dpi=%f g_dpi_scale_x=%f g_dpi_scale_y=%f", g_dpi, g_dpi_scale_x, g_dpi_scale_y); - INFO_LOG(G3D, "RecalcDPI: dp_xres=%d dp_yres=%d", dp_xres, dp_yres); - INFO_LOG(G3D, "RecalcDPI: pixel_xres=%d pixel_yres=%d", pixel_xres, pixel_yres); + INFO_LOG(G3D, "RecalcDPI: display_xres=%d display_yres=%d pixel_xres=%d pixel_yres=%d", display_xres, display_yres, pixel_xres, pixel_yres); + INFO_LOG(G3D, "RecalcDPI: g_dpi=%f g_dpi_scale_x=%f g_dpi_scale_y=%f dp_xres=%d dp_yres=%d", g_dpi, g_dpi_scale_x, g_dpi_scale_y, dp_xres, dp_yres); } extern "C" void JNICALL Java_org_ppsspp_ppsspp_NativeApp_backbufferResize(JNIEnv *, jclass, jint bufw, jint bufh, jint format) { @@ -1109,11 +1106,6 @@ extern "C" jboolean Java_org_ppsspp_ppsspp_NativeApp_keyUp(JNIEnv *, jclass, jin return NativeKey(keyInput); } -extern "C" void Java_org_ppsspp_ppsspp_NativeApp_beginJoystickEvent( - JNIEnv *env, jclass) { - // mutex lock? -} - extern "C" jboolean Java_org_ppsspp_ppsspp_NativeApp_joystickAxis( JNIEnv *env, jclass, jint deviceId, jint axisId, jfloat value) { if (!renderer_inited) @@ -1127,14 +1119,11 @@ extern "C" jboolean Java_org_ppsspp_ppsspp_NativeApp_joystickAxis( return NativeAxis(axis); } -extern "C" void Java_org_ppsspp_ppsspp_NativeApp_endJoystickEvent( - JNIEnv *env, jclass) { - // mutex unlock? -} - - extern "C" jboolean Java_org_ppsspp_ppsspp_NativeApp_mouseWheelEvent( JNIEnv *env, jclass, jint stick, jfloat x, jfloat y) { + if (!renderer_inited) + return false; + // TODO: Support mousewheel for android return true; } @@ -1376,6 +1365,7 @@ extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runEGLRenderLoop(J display_xres, display_yres, desiredBackbufferSizeX, desiredBackbufferSizeY); if (!wnd) { + // This shouldn't ever happen. ERROR_LOG(G3D, "Error: Surface is null."); renderLoopRunning = false; return false; @@ -1401,9 +1391,7 @@ extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runEGLRenderLoop(J } graphicsContext->ThreadStart(); renderer_inited = true; - } - if (!exitRenderLoop) { static bool hasSetThreadName = false; if (!hasSetThreadName) { hasSetThreadName = true; diff --git a/android/src/org/ppsspp/ppsspp/InputDeviceState.java b/android/src/org/ppsspp/ppsspp/InputDeviceState.java index 9c7317fa6c..b221032cd6 100644 --- a/android/src/org/ppsspp/ppsspp/InputDeviceState.java +++ b/android/src/org/ppsspp/ppsspp/InputDeviceState.java @@ -147,13 +147,11 @@ public class InputDeviceState { if ((event.getSource() & InputDevice.SOURCE_CLASS_JOYSTICK) == 0) { return false; } - NativeApp.beginJoystickEvent(); for (int i = 0; i < mAxes.length; i++) { int axisId = mAxes[i]; float value = event.getAxisValue(axisId); NativeApp.joystickAxis(deviceId, axisId, value); } - NativeApp.endJoystickEvent(); return true; } } diff --git a/android/src/org/ppsspp/ppsspp/NativeApp.java b/android/src/org/ppsspp/ppsspp/NativeApp.java index 8b3f709130..2fb7fa45a4 100644 --- a/android/src/org/ppsspp/ppsspp/NativeApp.java +++ b/android/src/org/ppsspp/ppsspp/NativeApp.java @@ -41,9 +41,7 @@ public class NativeApp { public static native boolean keyDown(int deviceId, int key, boolean isRepeat); public static native boolean keyUp(int deviceId, int key); - public static native void beginJoystickEvent(); public static native void joystickAxis(int deviceId, int axis, float value); - public static native void endJoystickEvent(); public static native boolean mouseWheelEvent(float x, float y); diff --git a/android/src/org/ppsspp/ppsspp/NativeGLView.java b/android/src/org/ppsspp/ppsspp/NativeGLView.java index b56380358d..d899fa5ce4 100644 --- a/android/src/org/ppsspp/ppsspp/NativeGLView.java +++ b/android/src/org/ppsspp/ppsspp/NativeGLView.java @@ -48,7 +48,7 @@ public class NativeGLView extends GLSurfaceView implements SensorEventListener, Log.i(TAG, "MOGA initialized"); mController.setListener(this, new Handler()); } catch (Exception e) { - Log.i(TAG, "Moga failed to initialize"); + // Log.d(TAG, "MOGA failed to initialize"); } } diff --git a/android/src/org/ppsspp/ppsspp/NativeRenderer.java b/android/src/org/ppsspp/ppsspp/NativeRenderer.java index 58d4de48d8..87904645dd 100644 --- a/android/src/org/ppsspp/ppsspp/NativeRenderer.java +++ b/android/src/org/ppsspp/ppsspp/NativeRenderer.java @@ -10,6 +10,7 @@ import javax.microedition.khronos.egl.EGLContext; import javax.microedition.khronos.egl.EGLDisplay; import javax.microedition.khronos.opengles.GL10; +// Only used for the OpenGL backend. public class NativeRenderer implements GLSurfaceView.Renderer { private static String TAG = "NativeRenderer"; private NativeActivity mActivity; @@ -35,7 +36,7 @@ public class NativeRenderer implements GLSurfaceView.Renderer { public void onSurfaceCreated(GL10 gl, EGLConfig config) { failed = false; - Log.i(TAG, "NativeRenderer: onSurfaceCreated"); + Log.i(TAG, "NativeRenderer (OpenGL): onSurfaceCreated"); EGL10 egl = (EGL10)EGLContext.getEGL(); if (egl != null) { diff --git a/android/src/org/ppsspp/ppsspp/PpssppActivity.java b/android/src/org/ppsspp/ppsspp/PpssppActivity.java index aa383a1568..a025926d67 100644 --- a/android/src/org/ppsspp/ppsspp/PpssppActivity.java +++ b/android/src/org/ppsspp/ppsspp/PpssppActivity.java @@ -107,7 +107,6 @@ public class PpssppActivity extends NativeActivity { } else { String param = getIntent().getStringExtra(SHORTCUT_EXTRA_KEY); String args = getIntent().getStringExtra(ARGS_EXTRA_KEY); - Log.e(TAG, "Got ACTION_VIEW without a valid uri, trying param"); if (param != null) { Log.i(TAG, "Found Shortcut Parameter in extra-data: " + param); super.setShortcutParam("\"" + param.replace("\\", "\\\\").replace("\"", "\\\"") + "\""); @@ -115,7 +114,6 @@ public class PpssppActivity extends NativeActivity { Log.i(TAG, "Found args parameter in extra-data: " + args); super.setShortcutParam(args); } else { - Log.e(TAG, "Shortcut missing parameter!"); super.setShortcutParam(""); } } From 463d703feb8fd6a8326c0433ea445cacd164eb3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 22 Dec 2022 23:07:30 +0100 Subject: [PATCH 2/5] More assorted cleanup --- Common/GPU/OpenGL/GLFeatures.cpp | 22 ++++++--- Common/GPU/OpenGL/GLFeatures.h | 4 +- android/jni/AndroidGraphicsContext.h | 12 ++++- android/jni/AndroidJavaGLContext.cpp | 14 ++++-- android/jni/AndroidJavaGLContext.h | 11 +---- android/jni/AndroidVulkanContext.cpp | 5 ++ android/jni/AndroidVulkanContext.h | 8 +-- android/jni/app-android.cpp | 49 ++++++++++--------- .../src/org/ppsspp/ppsspp/NativeActivity.java | 19 ++++--- 9 files changed, 82 insertions(+), 62 deletions(-) diff --git a/Common/GPU/OpenGL/GLFeatures.cpp b/Common/GPU/OpenGL/GLFeatures.cpp index c7bd284feb..c474ad7b8c 100644 --- a/Common/GPU/OpenGL/GLFeatures.cpp +++ b/Common/GPU/OpenGL/GLFeatures.cpp @@ -127,14 +127,14 @@ void ProcessGPUFeatures() { // http://stackoverflow.com/questions/16147700/opengl-es-using-tegra-specific-extensions-gl-ext-texture-array -void CheckGLExtensions() { - +bool CheckGLExtensions() { #if PPSSPP_API(ANY_GL) + // Make sure to only do this once. It's okay to call CheckGLExtensions from wherever, + // as long as you're on the rendering thread (the one with the GL context). + if (extensionsDone) { + return true; + } - // Make sure to only do this once. It's okay to call CheckGLExtensions from wherever. - if (extensionsDone) - return; - extensionsDone = true; memset(&gl_extensions, 0, sizeof(gl_extensions)); gl_extensions.IsCoreContext = useCoreContext; @@ -142,6 +142,12 @@ void CheckGLExtensions() { const char *versionStr = (const char *)glGetString(GL_VERSION); const char *glslVersionStr = (const char *)glGetString(GL_SHADING_LANGUAGE_VERSION); + if (!renderer || !versionStr || !glslVersionStr) { + // Something is very wrong! Bail. + return false; + } + + extensionsDone = true; #ifdef USING_GLES2 gl_extensions.IsGLES = !useCoreContext; @@ -269,7 +275,7 @@ void CheckGLExtensions() { // If the above didn't give us a version, or gave us a crazy version, fallback. #ifdef USING_GLES2 - if (gl_extensions.ver[0] < 3 || gl_extensions.ver[0] > 5) { + if (versionStr && (gl_extensions.ver[0] < 3 || gl_extensions.ver[0] > 5)) { // Try to load GLES 3.0 only if "3.0" found in version // This simple heuristic avoids issues on older devices where you can only call eglGetProcAddress a limited // number of times. Make sure to check for 3.0 in the shader version too to avoid false positives, see #5584. @@ -569,7 +575,7 @@ void CheckGLExtensions() { ERROR_LOG(G3D, "GL error in init: %i", error); #endif - + return true; } void SetGLCoreContext(bool flag) { diff --git a/Common/GPU/OpenGL/GLFeatures.h b/Common/GPU/OpenGL/GLFeatures.h index a2d20b178f..c85c8b3e38 100644 --- a/Common/GPU/OpenGL/GLFeatures.h +++ b/Common/GPU/OpenGL/GLFeatures.h @@ -129,7 +129,9 @@ void ProcessGPUFeatures(); extern std::string g_all_gl_extensions; extern std::string g_all_egl_extensions; -void CheckGLExtensions(); +// If this returns false, we're not gonna be able to use a GL context. +bool CheckGLExtensions(); + void SetGLCoreContext(bool flag); void ResetGLExtensions(); diff --git a/android/jni/AndroidGraphicsContext.h b/android/jni/AndroidGraphicsContext.h index 434017e077..b35f0de2a5 100644 --- a/android/jni/AndroidGraphicsContext.h +++ b/android/jni/AndroidGraphicsContext.h @@ -16,13 +16,23 @@ enum { ANDROID_VERSION_NOUGAT_1 = 25, }; +enum class GraphicsContextState { + PENDING, + INITIALIZED, + FAILED_INIT, + SHUTDOWN, +}; + class AndroidGraphicsContext : public GraphicsContext { public: // This is different than the base class function since on // Android (EGL, Vulkan) we do have all this info on the render thread. virtual bool InitFromRenderThread(ANativeWindow *wnd, int desiredBackbufferSizeX, int desiredBackbufferSizeY, int backbufferFormat, int androidVersion) = 0; - virtual bool Initialized() = 0; virtual void BeginAndroidShutdown() {} + virtual GraphicsContextState GetState() const { return state_; } + +protected: + GraphicsContextState state_ = GraphicsContextState::PENDING; private: using GraphicsContext::InitFromRenderThread; diff --git a/android/jni/AndroidJavaGLContext.cpp b/android/jni/AndroidJavaGLContext.cpp index f7f7b70444..08e52034a5 100644 --- a/android/jni/AndroidJavaGLContext.cpp +++ b/android/jni/AndroidJavaGLContext.cpp @@ -12,19 +12,27 @@ AndroidJavaEGLGraphicsContext::AndroidJavaEGLGraphicsContext() { bool AndroidJavaEGLGraphicsContext::InitFromRenderThread(ANativeWindow *wnd, int desiredBackbufferSizeX, int desiredBackbufferSizeY, int backbufferFormat, int androidVersion) { INFO_LOG(G3D, "AndroidJavaEGLGraphicsContext::InitFromRenderThread"); - CheckGLExtensions(); + if (!CheckGLExtensions()) { + ERROR_LOG(G3D, "CheckGLExtensions failed - not gonna attempt starting up."); + state_ = GraphicsContextState::FAILED_INIT; + return false; + } // OpenGL handles rotated rendering in the driver. g_display_rotation = DisplayRotation::ROTATE_0; g_display_rot_matrix.setIdentity(); + draw_ = Draw::T3DCreateGLContext(); // Can't fail renderManager_ = (GLRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER); renderManager_->SetInflightFrames(g_Config.iInflightFrames); if (!draw_->CreatePresets()) { + // This can't really happen now that compilation is async - they're only really queued for compile here. _assert_msg_(false, "Failed to compile preset shaders"); + state_ = GraphicsContextState::FAILED_INIT; return false; } + state_ = GraphicsContextState::INITIALIZED; return true; } @@ -34,7 +42,5 @@ void AndroidJavaEGLGraphicsContext::ShutdownFromRenderThread() { renderManager_ = nullptr; // owned by draw_. delete draw_; draw_ = nullptr; -} - -void AndroidJavaEGLGraphicsContext::Shutdown() { + state_ = GraphicsContextState::SHUTDOWN; } diff --git a/android/jni/AndroidJavaGLContext.h b/android/jni/AndroidJavaGLContext.h index e952de8acc..919bd8b80f 100644 --- a/android/jni/AndroidJavaGLContext.h +++ b/android/jni/AndroidJavaGLContext.h @@ -4,24 +4,17 @@ #include "Common/GPU/OpenGL/GLRenderManager.h" #include "Common/GPU/thin3d_create.h" -// Doesn't do much. Just to fit in. class AndroidJavaEGLGraphicsContext : public AndroidGraphicsContext { public: AndroidJavaEGLGraphicsContext(); - ~AndroidJavaEGLGraphicsContext() { - delete draw_; - } - - bool Initialized() override { - return draw_ != nullptr; - } + ~AndroidJavaEGLGraphicsContext() { delete draw_; } // This performs the actual initialization, bool InitFromRenderThread(ANativeWindow *wnd, int desiredBackbufferSizeX, int desiredBackbufferSizeY, int backbufferFormat, int androidVersion) override; void ShutdownFromRenderThread() override; - void Shutdown() override; + void Shutdown() override {} void SwapBuffers() override {} void SwapInterval(int interval) override {} void Resize() override {} diff --git a/android/jni/AndroidVulkanContext.cpp b/android/jni/AndroidVulkanContext.cpp index 2c6d63f8b5..6d5693273e 100644 --- a/android/jni/AndroidVulkanContext.cpp +++ b/android/jni/AndroidVulkanContext.cpp @@ -47,6 +47,7 @@ bool AndroidVulkanContext::InitAPI() { if (!VulkanLoad()) { ERROR_LOG(G3D, "Failed to load Vulkan driver library"); + state_ = GraphicsContextState::FAILED_INIT; return false; } @@ -65,6 +66,7 @@ bool AndroidVulkanContext::InitAPI() { VulkanSetAvailable(false); delete g_Vulkan; g_Vulkan = nullptr; + state_ = GraphicsContextState::FAILED_INIT; return false; } @@ -74,6 +76,7 @@ bool AndroidVulkanContext::InitAPI() { g_Vulkan->DestroyInstance(); delete g_Vulkan; g_Vulkan = nullptr; + state_ = GraphicsContextState::FAILED_INIT; return false; } @@ -86,10 +89,12 @@ bool AndroidVulkanContext::InitAPI() { g_Vulkan->DestroyInstance(); delete g_Vulkan; g_Vulkan = nullptr; + state_ = GraphicsContextState::FAILED_INIT; return false; } INFO_LOG(G3D, "Vulkan device created!"); + state_ = GraphicsContextState::INITIALIZED; return true; } diff --git a/android/jni/AndroidVulkanContext.h b/android/jni/AndroidVulkanContext.h index 2e5cfb7328..31d88c1a20 100644 --- a/android/jni/AndroidVulkanContext.h +++ b/android/jni/AndroidVulkanContext.h @@ -20,13 +20,7 @@ public: void Resize() override; void *GetAPIContext() override { return g_Vulkan; } - - Draw::DrawContext *GetDrawContext() override { - return draw_; - } - bool Initialized() override { - return draw_ != nullptr; - } + Draw::DrawContext *GetDrawContext() override { return draw_; } private: VulkanContext *g_Vulkan = nullptr; diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index 93ddb99cde..8f08a4233e 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -122,15 +122,15 @@ struct FrameCommand { static std::mutex frameCommandLock; static std::queue frameCommands; -std::string systemName; -std::string langRegion; -std::string mogaVersion; -std::string boardName; +static std::string systemName; +static std::string langRegion; +static std::string mogaVersion; +static std::string boardName; std::string g_externalDir; // Original external dir (root of Android storage). std::string g_extFilesDir; // App private external dir. -std::vector g_additionalStorageDirs; +static std::vector g_additionalStorageDirs; static int optimalFramesPerBuffer = 0; static int optimalSampleRate = 0; @@ -152,7 +152,7 @@ static int desiredBackbufferSizeX; static int desiredBackbufferSizeY; // Cache the class loader so we can use it from native threads. Required for TextAndroid. -JavaVM* gJvm = nullptr; +static JavaVM* gJvm = nullptr; static jobject gClassLoader; static jmethodID gFindClassMethod; @@ -167,18 +167,18 @@ static jmethodID getDebugString; static jobject nativeActivity; static std::atomic exitRenderLoop; -static bool renderLoopRunning; +static std::atomic renderLoopRunning; static bool renderer_inited = false; static std::mutex renderLock; static int inputBoxSequence = 1; -std::map> inputBoxCallbacks; +static std::map> inputBoxCallbacks; static bool sustainedPerfSupported = false; static std::map permissions; -AndroidGraphicsContext *graphicsContext; +static AndroidGraphicsContext *graphicsContext; #ifndef LOG_APP_NAME #define LOG_APP_NAME "PPSSPP" @@ -274,18 +274,23 @@ static void EmuThreadFunc() { INFO_LOG(SYSTEM, "Entering emu thread"); // Wait for render loop to get started. - if (!graphicsContext || !graphicsContext->Initialized()) { - INFO_LOG(SYSTEM, "Runloop: Waiting for displayInit..."); - while (!graphicsContext || !graphicsContext->Initialized()) { - sleep_ms(20); - } - } else { - INFO_LOG(SYSTEM, "Runloop: Graphics context available!"); + INFO_LOG(SYSTEM, "Runloop: Waiting for displayInit..."); + while (!graphicsContext || graphicsContext->GetState() == GraphicsContextState::PENDING) { + sleep_ms(20); + } + + // Check the state of the graphics context before we try to feed it into NativeInitGraphics. + if (graphicsContext->GetState() != GraphicsContextState::INITIALIZED) { + ERROR_LOG(G3D, "Failed to initialize the graphics context! %d", (int)graphicsContext->GetState()); + emuThreadState = (int)EmuThreadState::QUIT_REQUESTED; + gJvm->DetachCurrentThread(); + return; } if (!NativeInitGraphics(graphicsContext)) { - _assert_msg_(false, "Failed to initialize graphics, might as well bail"); + _assert_msg_(false, "NativeInitGraphics failed, might as well bail"); emuThreadState = (int)EmuThreadState::QUIT_REQUESTED; + gJvm->DetachCurrentThread(); return; } @@ -1186,7 +1191,7 @@ extern "C" void JNICALL Java_org_ppsspp_ppsspp_NativeApp_sendMessage(JNIEnv *env NativeMessageReceived(msg.c_str(), prm.c_str()); } -extern "C" void JNICALL Java_org_ppsspp_ppsspp_NativeActivity_exitEGLRenderLoop(JNIEnv *env, jobject obj) { +extern "C" void JNICALL Java_org_ppsspp_ppsspp_NativeActivity_requestExitVulkanRenderLoop(JNIEnv *env, jobject obj) { if (!renderLoopRunning) { ERROR_LOG(SYSTEM, "Render loop already exited"); return; @@ -1347,11 +1352,11 @@ static void ProcessFrameCommands(JNIEnv *env) { } // This runs in Vulkan mode only. -extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runEGLRenderLoop(JNIEnv *env, jobject obj, jobject _surf) { +extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runVulkanRenderLoop(JNIEnv *env, jobject obj, jobject _surf) { _assert_(!useCPUThread); if (!graphicsContext) { - ERROR_LOG(G3D, "runEGLRenderLoop: Tried to enter without a created graphics context."); + ERROR_LOG(G3D, "runVulkanRenderLoop: Tried to enter without a created graphics context."); return false; } @@ -1361,7 +1366,7 @@ extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runEGLRenderLoop(J ANativeWindow *wnd = _surf ? ANativeWindow_fromSurface(env, _surf) : nullptr; - WARN_LOG(G3D, "runEGLRenderLoop. display_xres=%d display_yres=%d desiredBackbufferSizeX=%d desiredBackbufferSizeY=%d", + WARN_LOG(G3D, "runVulkanRenderLoop. display_xres=%d display_yres=%d desiredBackbufferSizeX=%d desiredBackbufferSizeY=%d", display_xres, display_yres, desiredBackbufferSizeX, desiredBackbufferSizeY); if (!wnd) { @@ -1374,7 +1379,7 @@ extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runEGLRenderLoop(J if (!graphicsContext->InitFromRenderThread(wnd, desiredBackbufferSizeX, desiredBackbufferSizeY, backbuffer_format, androidVersion)) { // On Android, if we get here, really no point in continuing. // The UI is supposed to render on any device both on OpenGL and Vulkan. If either of those don't work - // on a device, we blacklist it. + // on a device, we blacklist it. Hopefully we should have already failed in InitAPI anyway and reverted to GL back then. ERROR_LOG(G3D, "Failed to initialize graphics context."); System_Toast("Failed to initialize graphics context."); diff --git a/android/src/org/ppsspp/ppsspp/NativeActivity.java b/android/src/org/ppsspp/ppsspp/NativeActivity.java index 02dc8c32a7..869403abde 100644 --- a/android/src/org/ppsspp/ppsspp/NativeActivity.java +++ b/android/src/org/ppsspp/ppsspp/NativeActivity.java @@ -25,7 +25,6 @@ import android.os.Bundle; import android.os.Environment; import android.os.PowerManager; import android.os.Vibrator; -import android.provider.DocumentsContract; import android.provider.MediaStore; import androidx.documentfile.provider.DocumentFile; import android.text.InputType; @@ -573,17 +572,17 @@ public abstract class NativeActivity extends Activity { public void run() { Log.i(TAG, "Starting the render loop: " + mSurface); // Start emulation using the provided Surface. - if (!runEGLRenderLoop(mSurface)) { + if (!runVulkanRenderLoop(mSurface)) { // Shouldn't happen. - Log.e(TAG, "Failed to start up OpenGL/Vulkan"); + Log.e(TAG, "Failed to start up OpenGL/Vulkan - runVulkanRenderLoop returned false"); } Log.i(TAG, "Left the render loop: " + mSurface); } }; - public native boolean runEGLRenderLoop(Surface surface); + public native boolean runVulkanRenderLoop(Surface surface); // Tells the render loop thread to exit, so we can restart it. - public native void exitEGLRenderLoop(); + public native void requestExitVulkanRenderLoop(); @Override public void onCreate(Bundle savedInstanceState) { @@ -688,17 +687,17 @@ public abstract class NativeActivity extends Activity { updateSustainedPerformanceMode(); } - // Invariants: After this, mRenderLoopThread will be set, and the thread will be running. + // Invariants: After this, mRenderLoopThread will be set, and the thread will be running, + // if in Vulkan mode. protected synchronized void ensureRenderLoop() { if (javaGL) { - Log.e(TAG, "JavaGL - should not get into ensureRenderLoop."); + Log.e(TAG, "JavaGL mode - should not get into ensureRenderLoop."); return; } if (mSurface == null) { Log.w(TAG, "ensureRenderLoop - not starting thread, needs surface"); return; } - if (mRenderLoopThread == null) { Log.w(TAG, "ensureRenderLoop: Starting thread"); mRenderLoopThread = new Thread(mEmulationRunner); @@ -715,8 +714,8 @@ public abstract class NativeActivity extends Activity { if (mRenderLoopThread != null) { // This will wait until the thread has exited. - Log.i(TAG, "exitEGLRenderLoop"); - exitEGLRenderLoop(); + Log.i(TAG, "requestExitVulkanRenderLoop"); + requestExitVulkanRenderLoop(); try { Log.i(TAG, "joining render loop thread..."); mRenderLoopThread.join(); From 708162a2b0e1b8d1f007b37ee12f9ebb7fb5deb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 22 Dec 2022 23:07:42 +0100 Subject: [PATCH 3/5] Vulkan validation layers: Cap outputting the same message at 10 times. --- Common/GPU/Vulkan/VulkanDebug.cpp | 30 ++++++++++++++++++++++++++++-- Common/GPU/Vulkan/VulkanDebug.h | 2 ++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanDebug.cpp b/Common/GPU/Vulkan/VulkanDebug.cpp index 7a3d3a079f..4daf41bdf7 100644 --- a/Common/GPU/Vulkan/VulkanDebug.cpp +++ b/Common/GPU/Vulkan/VulkanDebug.cpp @@ -17,16 +17,30 @@ #include #include +#include +#include #include "Common/Log.h" #include "Common/GPU/Vulkan/VulkanContext.h" #include "Common/GPU/Vulkan/VulkanDebug.h" +const int MAX_SAME_ERROR_COUNT = 10; + +// Used to stop outputting the same message over and over. +static std::map g_errorCount; +std::mutex g_errorCountMutex; + +// TODO: Call this when launching games in some clean way. +void VulkanClearValidationErrorCounts() { + std::lock_guard lock(g_errorCountMutex); + g_errorCount.clear(); +} + VKAPI_ATTR VkBool32 VKAPI_CALL VulkanDebugUtilsCallback( VkDebugUtilsMessageSeverityFlagBitsEXT messageSeverity, VkDebugUtilsMessageTypeFlagsEXT messageType, - const VkDebugUtilsMessengerCallbackDataEXT* pCallbackData, - void* pUserData) { + const VkDebugUtilsMessengerCallbackDataEXT *pCallbackData, + void *pUserData) { const VulkanLogOptions *options = (const VulkanLogOptions *)pUserData; std::ostringstream message; @@ -49,6 +63,18 @@ VKAPI_ATTR VkBool32 VKAPI_CALL VulkanDebugUtilsCallback( return false; } + int count; + { + std::lock_guard lock(g_errorCountMutex); + count = g_errorCount[messageCode]++; + } + if (count == MAX_SAME_ERROR_COUNT) { + WARN_LOG(G3D, "Too many validation messages with message %d, stopping", messageCode); + } + if (count >= MAX_SAME_ERROR_COUNT) { + return false; + } + if (messageSeverity & VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT) { message << "ERROR("; } else if (messageSeverity & VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT) { diff --git a/Common/GPU/Vulkan/VulkanDebug.h b/Common/GPU/Vulkan/VulkanDebug.h index 9287c1e8c5..c718a0303d 100644 --- a/Common/GPU/Vulkan/VulkanDebug.h +++ b/Common/GPU/Vulkan/VulkanDebug.h @@ -26,3 +26,5 @@ struct VulkanLogOptions { }; VKAPI_ATTR VkBool32 VKAPI_CALL VulkanDebugUtilsCallback(VkDebugUtilsMessageSeverityFlagBitsEXT messageSeverity, VkDebugUtilsMessageTypeFlagsEXT messageType, const VkDebugUtilsMessengerCallbackDataEXT *pCallbackData, void *pUserData); + +void VulkanClearValidationErrorCounts(); From 67cba831dd6b6c3811fbc261ad93dfa001727763 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 29 Dec 2022 00:01:36 +0100 Subject: [PATCH 4/5] Slightly more useful assert message in Hashmaps.h --- Common/Data/Collections/Hashmaps.h | 2 +- GPU/D3D11/DrawEngineD3D11.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Common/Data/Collections/Hashmaps.h b/Common/Data/Collections/Hashmaps.h index 8368c83db5..ac6b06c9f1 100644 --- a/Common/Data/Collections/Hashmaps.h +++ b/Common/Data/Collections/Hashmaps.h @@ -69,7 +69,7 @@ public: if (state[p] == BucketState::TAKEN) { if (KeyEquals(key, map[p].key)) { // Bad! We already got this one. Let's avoid this case. - _assert_msg_(false, "DenseHashMap: Duplicate key inserted"); + _assert_msg_(false, "DenseHashMap: Duplicate key of size %d inserted", (int)sizeof(Key)); return false; } // continue looking.... diff --git a/GPU/D3D11/DrawEngineD3D11.cpp b/GPU/D3D11/DrawEngineD3D11.cpp index 078c175169..5389775e98 100644 --- a/GPU/D3D11/DrawEngineD3D11.cpp +++ b/GPU/D3D11/DrawEngineD3D11.cpp @@ -201,7 +201,7 @@ static void VertexAttribSetup(D3D11_INPUT_ELEMENT_DESC * VertexElement, u8 fmt, ID3D11InputLayout *DrawEngineD3D11::SetupDecFmtForDraw(D3D11VertexShader *vshader, const DecVtxFormat &decFmt, u32 pspFmt) { // TODO: Instead of one for each vshader, we can reduce it to one for each type of shader // that reads TEXCOORD or not, etc. Not sure if worth it. - InputLayoutKey key{ vshader, decFmt.id }; + const InputLayoutKey key{ vshader, decFmt.id }; ID3D11InputLayout *inputLayout = inputLayoutMap_.Get(key); if (inputLayout) { return inputLayout; From 10c0b3f2aea282399145855fa19526e94e479fa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 29 Dec 2022 00:37:51 +0100 Subject: [PATCH 5/5] Comment improvements --- Common/GPU/OpenGL/GLFeatures.cpp | 2 +- Common/GPU/OpenGL/GLFeatures.h | 2 +- android/jni/app-android.cpp | 8 ++++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Common/GPU/OpenGL/GLFeatures.cpp b/Common/GPU/OpenGL/GLFeatures.cpp index c474ad7b8c..2a7158291a 100644 --- a/Common/GPU/OpenGL/GLFeatures.cpp +++ b/Common/GPU/OpenGL/GLFeatures.cpp @@ -135,7 +135,7 @@ bool CheckGLExtensions() { return true; } - memset(&gl_extensions, 0, sizeof(gl_extensions)); + gl_extensions = {}; gl_extensions.IsCoreContext = useCoreContext; const char *renderer = (const char *)glGetString(GL_RENDERER); diff --git a/Common/GPU/OpenGL/GLFeatures.h b/Common/GPU/OpenGL/GLFeatures.h index c85c8b3e38..fe72d037b2 100644 --- a/Common/GPU/OpenGL/GLFeatures.h +++ b/Common/GPU/OpenGL/GLFeatures.h @@ -30,7 +30,7 @@ enum { // Extensions to look at using: // GL_NV_map_buffer_range (same as GL_ARB_map_buffer_range ?) -// WARNING: This gets memset-d - so no strings please +// WARNING: This gets memset-d - so no strings or other non-POD types please // TODO: Rename this GLFeatures or something. struct GLExtensions { int ver[3]; diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index 8f08a4233e..3f0c1dca17 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -655,7 +655,7 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_init EARLY_LOG("NativeApp.init() -- begin"); PROFILE_INIT(); - std::lock_guard guard(renderLock); + std::lock_guard guard(renderLock); // Note: This is held for the rest of this function - intended? renderer_inited = false; androidVersion = jAndroidVersion; deviceType = jdeviceType; @@ -875,6 +875,7 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_shutdown(JNIEnv *, jclass) { } // JavaEGL. This doesn't get called on the Vulkan path. +// This gets called from onSurfaceCreated. extern "C" bool Java_org_ppsspp_ppsspp_NativeRenderer_displayInit(JNIEnv * env, jobject obj) { _assert_(useCPUThread); @@ -884,7 +885,10 @@ extern "C" bool Java_org_ppsspp_ppsspp_NativeRenderer_displayInit(JNIEnv * env, // We should be running on the render thread here. std::string errorMessage; if (renderer_inited) { - // Would be really nice if we could get something on the GL thread immediately when shutting down. + // Would be really nice if we could get something on the GL thread immediately when shutting down, + // but the only mechanism for handling lost devices seems to be that onSurfaceCreated is called again, + // which ends up calling displayInit. + INFO_LOG(G3D, "NativeApp.displayInit() restoring"); EmuThreadStop("displayInit"); graphicsContext->BeginAndroidShutdown();