SDL: Android: don't need to set the SurfaceHolder format from java code

From 98a966d1c20afc0abd27d8cd449810893104a908 Mon Sep 17 00:00:00 2001
From: Sylvain <[EMAIL REDACTED]>
Date: Thu, 22 Apr 2021 18:06:17 +0200
Subject: [PATCH] Android: don't need to set the SurfaceHolder format from java
 code It's already set with ANativeWindow_setGeometry, and eventually
 set/changed also by eglCreateWindowSurface.  - avoid issues with older device
 where SurfaceView cycle create/changed/destroy appears broken:    calling
 create/changed/changed, and leading to "deuqueBuffer failed at server side,
 error: -19", with black screen.  - re-read the format after egl window
 surface is created, to report the correct one (sometimes, changed from
 RGBA8888 to RGB24)

---
 .../main/java/org/libsdl/app/SDLActivity.java | 62 +------------------
 src/core/android/SDL_android.c                | 31 ++--------
 src/core/android/SDL_android.h                |  1 -
 src/video/SDL_egl.c                           | 35 ++++++-----
 src/video/android/SDL_androidvideo.c          | 46 ++++++++++++--
 src/video/android/SDL_androidvideo.h          |  3 +-
 6 files changed, 70 insertions(+), 108 deletions(-)

diff --git a/android-project/app/src/main/java/org/libsdl/app/SDLActivity.java b/android-project/app/src/main/java/org/libsdl/app/SDLActivity.java
index d829fbfec..2d83b9f65 100644
--- a/android-project/app/src/main/java/org/libsdl/app/SDLActivity.java
+++ b/android-project/app/src/main/java/org/libsdl/app/SDLActivity.java
@@ -605,7 +605,6 @@ public static void handleNativeState() {
     static final int COMMAND_CHANGE_TITLE = 1;
     static final int COMMAND_CHANGE_WINDOW_STYLE = 2;
     static final int COMMAND_TEXTEDIT_HIDE = 3;
-    static final int COMMAND_CHANGE_SURFACEVIEW_FORMAT = 4;
     static final int COMMAND_SET_KEEP_SCREEN_ON = 5;
 
     protected static final int COMMAND_USER = 0x8000;
@@ -703,32 +702,6 @@ public void handleMessage(Message msg) {
                 }
                 break;
             }
-            case COMMAND_CHANGE_SURFACEVIEW_FORMAT:
-            {
-                int format = (Integer) msg.obj;
-                int pf;
-
-                if (SDLActivity.mSurface == null) {
-                    return;
-                }
-
-                SurfaceHolder holder = SDLActivity.mSurface.getHolder();
-                if (holder == null) {
-                    return;
-                }
-
-                if (format == 1) {
-                    pf = PixelFormat.RGBA_8888;
-                } else if (format == 2) {
-                    pf = PixelFormat.RGBX_8888;
-                } else {
-                    pf = PixelFormat.RGB_565;
-                }
-
-                holder.setFormat(pf);
-
-                break;
-            }
             default:
                 if ((context instanceof SDLActivity) && !((SDLActivity) context).onUnhandledMessage(msg.arg1, msg.obj)) {
                     Log.e(TAG, "error handling message, command is " + msg.arg1);
@@ -812,7 +785,7 @@ boolean sendCommand(int command, Object data) {
     public static native void nativeResume();
     public static native void nativeFocusChanged(boolean hasFocus);
     public static native void onNativeDropFile(String filename);
-    public static native void nativeSetScreenResolution(int surfaceWidth, int surfaceHeight, int deviceWidth, int deviceHeight, int format, float rate);
+    public static native void nativeSetScreenResolution(int surfaceWidth, int surfaceHeight, int deviceWidth, int deviceHeight, float rate);
     public static native void onNativeResize();
     public static native void onNativeKeyDown(int keycode);
     public static native void onNativeKeyUp(int keycode);
@@ -1218,13 +1191,6 @@ public static Surface getNativeSurface() {
         return SDLActivity.mSurface.getNativeSurface();
     }
 
-    /**
-     * This method is called by SDL using JNI.
-     */
-    public static void setSurfaceViewFormat(int format) {
-        mSingleton.sendCommand(COMMAND_CHANGE_SURFACEVIEW_FORMAT, format);
-    }
-
     // Input
 
     /**
@@ -1796,30 +1762,6 @@ public void surfaceChanged(SurfaceHolder holder,
             return;
         }
 
-        int sdlFormat = 0x15151002; // SDL_PIXELFORMAT_RGB565 by default
-        switch (format) {
-        case PixelFormat.RGBA_8888:
-            Log.v("SDL", "pixel format RGBA_8888");
-            sdlFormat = 0x16462004; // SDL_PIXELFORMAT_RGBA8888
-            break;
-        case PixelFormat.RGBX_8888:
-            Log.v("SDL", "pixel format RGBX_8888");
-            sdlFormat = 0x16261804; // SDL_PIXELFORMAT_RGBX8888
-            break;
-        case PixelFormat.RGB_565:
-            Log.v("SDL", "pixel format RGB_565");
-            sdlFormat = 0x15151002; // SDL_PIXELFORMAT_RGB565
-            break;
-        case PixelFormat.RGB_888:
-            Log.v("SDL", "pixel format RGB_888");
-            // Not sure this is right, maybe SDL_PIXELFORMAT_RGB24 instead?
-            sdlFormat = 0x16161804; // SDL_PIXELFORMAT_RGB888
-            break;
-        default:
-            Log.v("SDL", "pixel format unknown " + format);
-            break;
-        }
-
         mWidth = width;
         mHeight = height;
         int nDeviceWidth = width;
@@ -1842,7 +1784,7 @@ public void surfaceChanged(SurfaceHolder holder,
 
         Log.v("SDL", "Window size: " + width + "x" + height);
         Log.v("SDL", "Device size: " + nDeviceWidth + "x" + nDeviceHeight);
-        SDLActivity.nativeSetScreenResolution(width, height, nDeviceWidth, nDeviceHeight, sdlFormat, mDisplay.getRefreshRate());
+        SDLActivity.nativeSetScreenResolution(width, height, nDeviceWidth, nDeviceHeight, mDisplay.getRefreshRate());
         SDLActivity.onNativeResize();
 
         // Prevent a screen distortion glitch,
diff --git a/src/core/android/SDL_android.c b/src/core/android/SDL_android.c
index a01787ea3..47e1ed657 100644
--- a/src/core/android/SDL_android.c
+++ b/src/core/android/SDL_android.c
@@ -79,7 +79,7 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(onNativeDropFile)(
 JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativeSetScreenResolution)(
         JNIEnv *env, jclass jcls,
         jint surfaceWidth, jint surfaceHeight,
-        jint deviceWidth, jint deviceHeight, jint format, jfloat rate);
+        jint deviceWidth, jint deviceHeight, jfloat rate);
 
 JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(onNativeResize)(
         JNIEnv *env, jclass cls);
@@ -168,7 +168,7 @@ static JNINativeMethod SDLActivity_tab[] = {
     { "nativeSetupJNI",             "()I", SDL_JAVA_INTERFACE(nativeSetupJNI) },
     { "nativeRunMain",              "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;)I", SDL_JAVA_INTERFACE(nativeRunMain) },
     { "onNativeDropFile",           "(Ljava/lang/String;)V", SDL_JAVA_INTERFACE(onNativeDropFile) },
-    { "nativeSetScreenResolution",  "(IIIIIF)V", SDL_JAVA_INTERFACE(nativeSetScreenResolution) },
+    { "nativeSetScreenResolution",  "(IIIIF)V", SDL_JAVA_INTERFACE(nativeSetScreenResolution) },
     { "onNativeResize",             "()V", SDL_JAVA_INTERFACE(onNativeResize) },
     { "onNativeSurfaceCreated",     "()V", SDL_JAVA_INTERFACE(onNativeSurfaceCreated) },
     { "onNativeSurfaceChanged",     "()V", SDL_JAVA_INTERFACE(onNativeSurfaceChanged) },
@@ -318,7 +318,6 @@ static jmethodID midSetActivityTitle;
 static jmethodID midSetCustomCursor;
 static jmethodID midSetOrientation;
 static jmethodID midSetRelativeMouseEnabled;
-static jmethodID midSetSurfaceViewFormat;
 static jmethodID midSetSystemCursor;
 static jmethodID midSetWindowStyle;
 static jmethodID midShouldMinimizeOnFocusLoss;
@@ -598,7 +597,6 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativeSetupJNI)(JNIEnv *env, jclass cl
     midSetCustomCursor = (*env)->GetStaticMethodID(env, mActivityClass, "setCustomCursor", "(I)Z");
     midSetOrientation = (*env)->GetStaticMethodID(env, mActivityClass, "setOrientation","(IIZLjava/lang/String;)V");
     midSetRelativeMouseEnabled = (*env)->GetStaticMethodID(env, mActivityClass, "setRelativeMouseEnabled", "(Z)Z");
-    midSetSurfaceViewFormat = (*env)->GetStaticMethodID(env, mActivityClass, "setSurfaceViewFormat","(I)V");
     midSetSystemCursor = (*env)->GetStaticMethodID(env, mActivityClass, "setSystemCursor", "(I)Z");
     midSetWindowStyle = (*env)->GetStaticMethodID(env, mActivityClass, "setWindowStyle","(Z)V");
     midShouldMinimizeOnFocusLoss = (*env)->GetStaticMethodID(env, mActivityClass, "shouldMinimizeOnFocusLoss","()Z");
@@ -629,7 +627,6 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativeSetupJNI)(JNIEnv *env, jclass cl
         !midSetCustomCursor ||
         !midSetOrientation ||
         !midSetRelativeMouseEnabled ||
-        !midSetSurfaceViewFormat ||
         !midSetSystemCursor ||
         !midSetWindowStyle ||
         !midShouldMinimizeOnFocusLoss ||
@@ -845,11 +842,11 @@ void Android_ActivityMutex_Lock_Running() {
 JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativeSetScreenResolution)(
                                     JNIEnv *env, jclass jcls,
                                     jint surfaceWidth, jint surfaceHeight,
-                                    jint deviceWidth, jint deviceHeight, jint format, jfloat rate)
+                                    jint deviceWidth, jint deviceHeight, jfloat rate)
 {
     SDL_LockMutex(Android_ActivityMutex);
 
-    Android_SetScreenResolution(surfaceWidth, surfaceHeight, deviceWidth, deviceHeight, format, rate);
+    Android_SetScreenResolution(surfaceWidth, surfaceHeight, deviceWidth, deviceHeight, rate);
 
     SDL_UnlockMutex(Android_ActivityMutex);
 }
@@ -1384,26 +1381,6 @@ ANativeWindow* Android_JNI_GetNativeWindow(void)
     return anw;
 }
 
-void Android_JNI_SetSurfaceViewFormat(int format)
-{
-    JNIEnv *env = Android_JNI_GetEnv();
-    int new_format = 0;
-
-    /* Format from android/native_window.h,
-     * convert to temporary arbitrary values,
-     * then to java PixelFormat */
-    if (format == WINDOW_FORMAT_RGBA_8888) {
-        new_format = 1;
-    } else if (format == WINDOW_FORMAT_RGBX_8888) {
-        new_format = 2;
-    } else if (format == WINDOW_FORMAT_RGB_565) {
-        /* Default */
-        new_format = 0;
-    }
-
-    (*env)->CallStaticVoidMethod(env, mActivityClass, midSetSurfaceViewFormat, new_format);
-}
-
 void Android_JNI_SetActivityTitle(const char *title)
 {
     JNIEnv *env = Android_JNI_GetEnv();
diff --git a/src/core/android/SDL_android.h b/src/core/android/SDL_android.h
index 7c4e3dbad..5db745828 100644
--- a/src/core/android/SDL_android.h
+++ b/src/core/android/SDL_android.h
@@ -47,7 +47,6 @@ extern void Android_JNI_ShowTextInput(SDL_Rect *inputRect);
 extern void Android_JNI_HideTextInput(void);
 extern SDL_bool Android_JNI_IsScreenKeyboardShown(void);
 extern ANativeWindow* Android_JNI_GetNativeWindow(void);
-extern void Android_JNI_SetSurfaceViewFormat(int format);
 
 extern SDL_DisplayOrientation Android_JNI_GetDisplayOrientation(void);
 extern int Android_JNI_GetDisplayDPI(float *ddpi, float *xdpi, float *ydpi);
diff --git a/src/video/SDL_egl.c b/src/video/SDL_egl.c
index ca40cb8ba..e3b50d97f 100644
--- a/src/video/SDL_egl.c
+++ b/src/video/SDL_egl.c
@@ -28,6 +28,7 @@
 #if SDL_VIDEO_DRIVER_ANDROID
 #include <android/native_window.h>
 #include "../core/android/SDL_android.h"
+#include "../video/android/SDL_androidvideo.h"
 #endif
 
 #include "SDL_sysvideo.h"
@@ -1132,6 +1133,10 @@ SDL_EGL_DeleteContext(_THIS, SDL_GLContext context)
 EGLSurface *
 SDL_EGL_CreateSurface(_THIS, NativeWindowType nw) 
 {
+#if SDL_VIDEO_DRIVER_ANDROID
+    EGLint format_wanted;
+    EGLint format_got;
+#endif
     /* max 2 values plus terminator. */
     EGLint attribs[3];
     int attr = 0;
@@ -1141,24 +1146,18 @@ SDL_EGL_CreateSurface(_THIS, NativeWindowType nw)
     if (SDL_EGL_ChooseConfig(_this) != 0) {
         return EGL_NO_SURFACE;
     }
-    
+
 #if SDL_VIDEO_DRIVER_ANDROID
-    {
-        /* Android docs recommend doing this!
-         * Ref: http://developer.android.com/reference/android/app/NativeActivity.html 
-         */
-        EGLint format;
-        _this->egl_data->eglGetConfigAttrib(_this->egl_data->egl_display,
-                                            _this->egl_data->egl_config, 
-                                            EGL_NATIVE_VISUAL_ID, &format);
+    /* On Android, EGL_NATIVE_VISUAL_ID is an attribute of the EGLConfig that is
+     * guaranteed to be accepted by ANativeWindow_setBuffersGeometry(). */
+    _this->egl_data->eglGetConfigAttrib(_this->egl_data->egl_display,
+            _this->egl_data->egl_config,
+            EGL_NATIVE_VISUAL_ID, &format_wanted);
 
-        ANativeWindow_setBuffersGeometry(nw, 0, 0, format);
+    /* Format based on selected egl config. */
+    ANativeWindow_setBuffersGeometry(nw, 0, 0, format_wanted);
+#endif
 
-        /* Update SurfaceView holder format.
-         * May triggers a sequence surfaceDestroyed(), surfaceCreated(), surfaceChanged(). */
-        Android_JNI_SetSurfaceViewFormat(format);
-    }
-#endif    
     if (_this->gl_config.framebuffer_srgb_capable) {
 #ifdef EGL_KHR_gl_colorspace
         if (SDL_EGL_HasExtension(_this, SDL_EGL_DISPLAY_EXTENSION, "EGL_KHR_gl_colorspace")) {
@@ -1181,6 +1180,12 @@ SDL_EGL_CreateSurface(_THIS, NativeWindowType nw)
     if (surface == EGL_NO_SURFACE) {
         SDL_EGL_SetError("unable to create an EGL window surface", "eglCreateWindowSurface");
     }
+
+#if SDL_VIDEO_DRIVER_ANDROID
+    format_got = ANativeWindow_getFormat(nw);
+    Android_SetFormat(format_wanted, format_got);
+#endif
+
     return surface;
 }
 
diff --git a/src/video/android/SDL_androidvideo.c b/src/video/android/SDL_androidvideo.c
index 75ad82cc9..e5154f02e 100644
--- a/src/video/android/SDL_androidvideo.c
+++ b/src/video/android/SDL_androidvideo.c
@@ -64,7 +64,7 @@ int Android_SurfaceWidth           = 0;
 int Android_SurfaceHeight          = 0;
 static int Android_DeviceWidth     = 0;
 static int Android_DeviceHeight    = 0;
-static Uint32 Android_ScreenFormat = SDL_PIXELFORMAT_UNKNOWN;
+static Uint32 Android_ScreenFormat = SDL_PIXELFORMAT_RGB565; /* Default SurfaceView format, in case this is queried before being filled */
 static int Android_ScreenRate      = 0;
 SDL_sem *Android_PauseSem          = NULL;
 SDL_sem *Android_ResumeSem         = NULL;
@@ -194,7 +194,7 @@ Android_VideoInit(_THIS)
         return -1;
     }
     display = SDL_GetDisplay(display_index);
-    display->orientation = Android_JNI_GetDisplayOrientation();    
+    display->orientation = Android_JNI_GetDisplayOrientation();
 
     SDL_AddDisplayMode(&_this->displays[0], &mode);
 
@@ -222,16 +222,54 @@ Android_GetDisplayDPI(_THIS, SDL_VideoDisplay *display, float *ddpi, float *hdpi
 }
 
 void
-Android_SetScreenResolution(int surfaceWidth, int surfaceHeight, int deviceWidth, int deviceHeight, Uint32 format, float rate)
+Android_SetScreenResolution(int surfaceWidth, int surfaceHeight, int deviceWidth, int deviceHeight, float rate)
 {
     Android_SurfaceWidth  = surfaceWidth;
     Android_SurfaceHeight = surfaceHeight;
     Android_DeviceWidth   = deviceWidth;
     Android_DeviceHeight  = deviceHeight;
-    Android_ScreenFormat  = format;
     Android_ScreenRate    = (int)rate;
 }
 
+static
+Uint32 format_to_pixelFormat(int format) {
+    Uint32 pf;
+    if (format == AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM) {  /* 1 */
+        pf = SDL_PIXELFORMAT_RGBA8888;
+    } else if (format == AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM) { /* 2 */
+        pf = SDL_PIXELFORMAT_RGBX8888;
+    } else if (format == AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM) { /* 3 */
+        pf = SDL_PIXELFORMAT_RGB24;
+    } else if (format == AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM) { /* 4*/
+        pf = SDL_PIXELFORMAT_RGB565;
+    } else if (format == 5) {
+        pf = SDL_PIXELFORMAT_BGRA8888;
+    } else if (format == 6) {
+        pf = SDL_PIXELFORMAT_RGBA5551;
+    } else if (format == 7) {
+        pf = SDL_PIXELFORMAT_RGBA4444;
+    } else {
+        pf = SDL_PIXELFORMAT_UNKNOWN;
+    }
+    return pf;
+}
+
+void
+Android_SetFormat(int format_wanted, int format_got)
+{
+    Uint32 pf_wanted;
+    Uint32 pf_got;
+
+    pf_wanted = format_to_pixelFormat(format_wanted);
+    pf_got = format_to_pixelFormat(format_got);
+
+    Android_ScreenFormat = pf_got;
+
+    SDL_Log("pixel format wanted %s (%d), got %s (%d)",
+            SDL_GetPixelFormatName(pf_wanted), format_wanted,
+            SDL_GetPixelFormatName(pf_got), format_got);
+}
+
 void Android_SendResize(SDL_Window *window)
 {
     /*
diff --git a/src/video/android/SDL_androidvideo.h b/src/video/android/SDL_androidvideo.h
index d93054410..e36a3daf9 100644
--- a/src/video/android/SDL_androidvideo.h
+++ b/src/video/android/SDL_androidvideo.h
@@ -28,7 +28,8 @@
 #include "../SDL_sysvideo.h"
 
 /* Called by the JNI layer when the screen changes size or format */
-extern void Android_SetScreenResolution(int surfaceWidth, int surfaceHeight, int deviceWidth, int deviceHeight, Uint32 format, float rate);
+extern void Android_SetScreenResolution(int surfaceWidth, int surfaceHeight, int deviceWidth, int deviceHeight, float rate);
+extern void Android_SetFormat(int format_wanted, int format_got);
 extern void Android_SendResize(SDL_Window *window);
 
 /* Private display data */