From b9a2f108fc055de6a093fcec89624de0583da9cd Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Thu, 31 Oct 2024 16:52:26 -0300 Subject: [PATCH] Fix splash screen upside down on Android Fixes an issue introduced in #96439 (see https://github.com/godotengine/godot/pull/96439#issuecomment-2447288702) Godot was relying on Java's activity.getWindowManager().getDefaultDisplay().getRotation(); to apply pre-rotation but this is wrong. First, getRotation() may temporarily return a different value from the correct one; which is what was causing the splash screen to be upside down. It would return -90 instead of 90 for the first rendered frame. But unfortunately, the splash screen is just one frame rendered for a very long time, so the error lingered for a long time for everyone to see. Second, to determine what rotation to use, we should be looking at what Vulkan told us, which is the value we pass to VkSurfaceTransformFlagBitsKHR::preTransform. This commit removes the now-unnecessary screen_get_internal_current_rotation() function (which was introduced by #96439) and now saves the preTransform value in the swapchain. --- .../vulkan/rendering_device_driver_vulkan.cpp | 25 +++++++++++++++++++ .../vulkan/rendering_device_driver_vulkan.h | 2 ++ platform/android/display_server_android.cpp | 8 ------ platform/android/display_server_android.h | 1 - .../src/org/godotengine/godot/GodotIO.java | 22 ---------------- platform/android/java_godot_io_wrapper.cpp | 11 -------- platform/android/java_godot_io_wrapper.h | 2 -- servers/display_server.h | 7 ------ .../renderer_rd/renderer_compositor_rd.cpp | 4 +-- servers/rendering/rendering_device.cpp | 9 +++++++ servers/rendering/rendering_device.h | 1 + servers/rendering/rendering_device_driver.h | 3 +++ 12 files changed, 42 insertions(+), 53 deletions(-) diff --git a/drivers/vulkan/rendering_device_driver_vulkan.cpp b/drivers/vulkan/rendering_device_driver_vulkan.cpp index f9f1168a97a..4be64590dc4 100644 --- a/drivers/vulkan/rendering_device_driver_vulkan.cpp +++ b/drivers/vulkan/rendering_device_driver_vulkan.cpp @@ -2996,6 +2996,24 @@ Error RenderingDeviceDriverVulkan::swap_chain_resize(CommandQueueID p_cmd_queue, swap_create_info.imageUsage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT; swap_create_info.imageSharingMode = VK_SHARING_MODE_EXCLUSIVE; swap_create_info.preTransform = surface_transform_bits; + switch (swap_create_info.preTransform) { + case VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR: + swap_chain->pre_transform_rotation_degrees = 0; + break; + case VK_SURFACE_TRANSFORM_ROTATE_90_BIT_KHR: + swap_chain->pre_transform_rotation_degrees = 90; + break; + case VK_SURFACE_TRANSFORM_ROTATE_180_BIT_KHR: + swap_chain->pre_transform_rotation_degrees = 180; + break; + case VK_SURFACE_TRANSFORM_ROTATE_270_BIT_KHR: + swap_chain->pre_transform_rotation_degrees = 270; + break; + default: + WARN_PRINT("Unexpected swap_create_info.preTransform = " + itos(swap_create_info.preTransform) + "."); + swap_chain->pre_transform_rotation_degrees = 0; + break; + } swap_create_info.compositeAlpha = composite_alpha; swap_create_info.presentMode = present_mode; swap_create_info.clipped = true; @@ -3167,6 +3185,13 @@ RDD::RenderPassID RenderingDeviceDriverVulkan::swap_chain_get_render_pass(SwapCh return swap_chain->render_pass; } +int RenderingDeviceDriverVulkan::swap_chain_get_pre_rotation_degrees(SwapChainID p_swap_chain) { + DEV_ASSERT(p_swap_chain.id != 0); + + SwapChain *swap_chain = (SwapChain *)(p_swap_chain.id); + return swap_chain->pre_transform_rotation_degrees; +} + RDD::DataFormat RenderingDeviceDriverVulkan::swap_chain_get_format(SwapChainID p_swap_chain) { DEV_ASSERT(p_swap_chain.id != 0); diff --git a/drivers/vulkan/rendering_device_driver_vulkan.h b/drivers/vulkan/rendering_device_driver_vulkan.h index 33cce30b349..3ae959a89fc 100644 --- a/drivers/vulkan/rendering_device_driver_vulkan.h +++ b/drivers/vulkan/rendering_device_driver_vulkan.h @@ -359,6 +359,7 @@ private: LocalVector command_queues_acquired; LocalVector command_queues_acquired_semaphores; RenderPassID render_pass; + int pre_transform_rotation_degrees = 0; uint32_t image_index = 0; #ifdef ANDROID_ENABLED uint64_t refresh_duration = 0; @@ -373,6 +374,7 @@ public: virtual Error swap_chain_resize(CommandQueueID p_cmd_queue, SwapChainID p_swap_chain, uint32_t p_desired_framebuffer_count) override final; virtual FramebufferID swap_chain_acquire_framebuffer(CommandQueueID p_cmd_queue, SwapChainID p_swap_chain, bool &r_resize_required) override final; virtual RenderPassID swap_chain_get_render_pass(SwapChainID p_swap_chain) override final; + virtual int swap_chain_get_pre_rotation_degrees(SwapChainID p_swap_chain) override final; virtual DataFormat swap_chain_get_format(SwapChainID p_swap_chain) override final; virtual void swap_chain_set_max_fps(SwapChainID p_swap_chain, int p_max_fps) override final; virtual void swap_chain_free(SwapChainID p_swap_chain) override final; diff --git a/platform/android/display_server_android.cpp b/platform/android/display_server_android.cpp index f5032eaa408..a725665a148 100644 --- a/platform/android/display_server_android.cpp +++ b/platform/android/display_server_android.cpp @@ -229,14 +229,6 @@ DisplayServer::ScreenOrientation DisplayServerAndroid::screen_get_orientation(in return (ScreenOrientation)orientation; } -int DisplayServerAndroid::screen_get_internal_current_rotation(int p_screen) const { - GodotIOJavaWrapper *godot_io_java = OS_Android::get_singleton()->get_godot_io_java(); - ERR_FAIL_NULL_V(godot_io_java, 0); - - const int rotation = godot_io_java->get_internal_current_screen_rotation(); - return rotation; -} - int DisplayServerAndroid::get_screen_count() const { return 1; } diff --git a/platform/android/display_server_android.h b/platform/android/display_server_android.h index 0b8b4dd6e82..c44a265750f 100644 --- a/platform/android/display_server_android.h +++ b/platform/android/display_server_android.h @@ -129,7 +129,6 @@ public: virtual void screen_set_orientation(ScreenOrientation p_orientation, int p_screen = SCREEN_OF_MAIN_WINDOW) override; virtual ScreenOrientation screen_get_orientation(int p_screen = SCREEN_OF_MAIN_WINDOW) const override; - virtual int screen_get_internal_current_rotation(int p_screen) const override; virtual int get_screen_count() const override; virtual int get_primary_screen() const override; diff --git a/platform/android/java/lib/src/org/godotengine/godot/GodotIO.java b/platform/android/java/lib/src/org/godotengine/godot/GodotIO.java index 55437454449..79751dd58f3 100644 --- a/platform/android/java/lib/src/org/godotengine/godot/GodotIO.java +++ b/platform/android/java/lib/src/org/godotengine/godot/GodotIO.java @@ -296,28 +296,6 @@ public class GodotIO { } } - /** - This function is used by DisplayServer::screen_get_internal_current_rotation (C++) - and is used to implement a performance optimization in devices that do not offer - a HW rotator. - @return - Rotation in degrees, in multiples of 90° - */ - public int getInternalCurrentScreenRotation() { - int rotation = activity.getWindowManager().getDefaultDisplay().getRotation(); - - switch (rotation) { - case Surface.ROTATION_90: - return 90; - case Surface.ROTATION_180: - return 180; - case Surface.ROTATION_270: - return 270; - default: - return 0; - } - } - public void setEdit(GodotEditText _edit) { edit = _edit; } diff --git a/platform/android/java_godot_io_wrapper.cpp b/platform/android/java_godot_io_wrapper.cpp index e58ef50a735..623db39985f 100644 --- a/platform/android/java_godot_io_wrapper.cpp +++ b/platform/android/java_godot_io_wrapper.cpp @@ -66,7 +66,6 @@ GodotIOJavaWrapper::GodotIOJavaWrapper(JNIEnv *p_env, jobject p_godot_io_instanc _has_hardware_keyboard = p_env->GetMethodID(cls, "hasHardwareKeyboard", "()Z"); _set_screen_orientation = p_env->GetMethodID(cls, "setScreenOrientation", "(I)V"); _get_screen_orientation = p_env->GetMethodID(cls, "getScreenOrientation", "()I"); - _get_internal_current_screen_rotation = p_env->GetMethodID(cls, "getInternalCurrentScreenRotation", "()I"); _get_system_dir = p_env->GetMethodID(cls, "getSystemDir", "(IZ)Ljava/lang/String;"); } } @@ -268,16 +267,6 @@ int GodotIOJavaWrapper::get_screen_orientation() { } } -int GodotIOJavaWrapper::get_internal_current_screen_rotation() { - if (_get_internal_current_screen_rotation) { - JNIEnv *env = get_jni_env(); - ERR_FAIL_NULL_V(env, 0); - return env->CallIntMethod(godot_io_instance, _get_internal_current_screen_rotation); - } else { - return 0; - } -} - String GodotIOJavaWrapper::get_system_dir(int p_dir, bool p_shared_storage) { if (_get_system_dir) { JNIEnv *env = get_jni_env(); diff --git a/platform/android/java_godot_io_wrapper.h b/platform/android/java_godot_io_wrapper.h index 903bdce4be5..0a372641cbb 100644 --- a/platform/android/java_godot_io_wrapper.h +++ b/platform/android/java_godot_io_wrapper.h @@ -61,7 +61,6 @@ private: jmethodID _has_hardware_keyboard = 0; jmethodID _set_screen_orientation = 0; jmethodID _get_screen_orientation = 0; - jmethodID _get_internal_current_screen_rotation = 0; jmethodID _get_system_dir = 0; public: @@ -89,7 +88,6 @@ public: void set_vk_height(int p_height); void set_screen_orientation(int p_orient); int get_screen_orientation(); - int get_internal_current_screen_rotation(); String get_system_dir(int p_dir, bool p_shared_storage); }; diff --git a/servers/display_server.h b/servers/display_server.h index 670afb36469..bca4fbb56ab 100644 --- a/servers/display_server.h +++ b/servers/display_server.h @@ -360,13 +360,6 @@ public: virtual void screen_set_orientation(ScreenOrientation p_orientation, int p_screen = SCREEN_OF_MAIN_WINDOW); virtual ScreenOrientation screen_get_orientation(int p_screen = SCREEN_OF_MAIN_WINDOW) const; - // Note: The "internal" current orientation is not necessarily the current orientation and will often be 0 for most platforms. - // - // Some Android GPUs come with a HW-based rotator which means the screen gets rotated for free to - // whatever orientation the device is currently facing. But many Android GPUs emulate it via SW instead, - // which costs performance and power. This value is an optimization that tells Godot's compositor how to - // rotate the render texture before presenting to screen so that Android's compositor doesn't have to. - virtual int screen_get_internal_current_rotation(int p_screen = SCREEN_OF_MAIN_WINDOW) const { return 0; } virtual void screen_set_keep_on(bool p_enable); //disable screensaver virtual bool screen_is_kept_on() const; diff --git a/servers/rendering/renderer_rd/renderer_compositor_rd.cpp b/servers/rendering/renderer_rd/renderer_compositor_rd.cpp index d04285fbb4f..ba47508700f 100644 --- a/servers/rendering/renderer_rd/renderer_compositor_rd.cpp +++ b/servers/rendering/renderer_rd/renderer_compositor_rd.cpp @@ -67,7 +67,7 @@ void RendererCompositorRD::blit_render_targets_to_screen(DisplayServer::WindowID RD::get_singleton()->draw_list_bind_uniform_set(draw_list, render_target_descriptors[rd_texture], 0); // We need to invert the phone rotation. - int screen_rotation_degrees = -DisplayServer::get_singleton()->screen_get_internal_current_rotation(); + const int screen_rotation_degrees = -RD::get_singleton()->screen_get_pre_rotation_degrees(p_screen); float screen_rotation = Math::deg_to_rad((float)screen_rotation_degrees); blit.push_constant.rotation_cos = Math::cos(screen_rotation); @@ -238,7 +238,7 @@ void RendererCompositorRD::set_boot_image(const Ref &p_image, const Color RD::get_singleton()->draw_list_bind_index_array(draw_list, blit.array); RD::get_singleton()->draw_list_bind_uniform_set(draw_list, uset, 0); - int screen_rotation_degrees = DisplayServer::get_singleton()->screen_get_internal_current_rotation(); + const int screen_rotation_degrees = -RD::get_singleton()->screen_get_pre_rotation_degrees(DisplayServer::MAIN_WINDOW_ID); float screen_rotation = Math::deg_to_rad((float)screen_rotation_degrees); blit.push_constant.rotation_cos = Math::cos(screen_rotation); blit.push_constant.rotation_sin = Math::sin(screen_rotation); diff --git a/servers/rendering/rendering_device.cpp b/servers/rendering/rendering_device.cpp index dc5a178aaa1..cc67873b242 100644 --- a/servers/rendering/rendering_device.cpp +++ b/servers/rendering/rendering_device.cpp @@ -3757,6 +3757,15 @@ int RenderingDevice::screen_get_height(DisplayServer::WindowID p_screen) const { return context->surface_get_height(surface); } +int RenderingDevice::screen_get_pre_rotation_degrees(DisplayServer::WindowID p_screen) const { + _THREAD_SAFE_METHOD_ + + HashMap::ConstIterator it = screen_swap_chains.find(p_screen); + ERR_FAIL_COND_V_MSG(it == screen_swap_chains.end(), ERR_CANT_CREATE, "A swap chain was not created for the screen."); + + return driver->swap_chain_get_pre_rotation_degrees(it->value); +} + RenderingDevice::FramebufferFormatID RenderingDevice::screen_get_framebuffer_format(DisplayServer::WindowID p_screen) const { _THREAD_SAFE_METHOD_ diff --git a/servers/rendering/rendering_device.h b/servers/rendering/rendering_device.h index c7b93f2fc7c..9939df976f6 100644 --- a/servers/rendering/rendering_device.h +++ b/servers/rendering/rendering_device.h @@ -1083,6 +1083,7 @@ public: Error screen_prepare_for_drawing(DisplayServer::WindowID p_screen = DisplayServer::MAIN_WINDOW_ID); int screen_get_width(DisplayServer::WindowID p_screen = DisplayServer::MAIN_WINDOW_ID) const; int screen_get_height(DisplayServer::WindowID p_screen = DisplayServer::MAIN_WINDOW_ID) const; + int screen_get_pre_rotation_degrees(DisplayServer::WindowID p_screen = DisplayServer::MAIN_WINDOW_ID) const; FramebufferFormatID screen_get_framebuffer_format(DisplayServer::WindowID p_screen = DisplayServer::MAIN_WINDOW_ID) const; Error screen_free(DisplayServer::WindowID p_screen = DisplayServer::MAIN_WINDOW_ID); diff --git a/servers/rendering/rendering_device_driver.h b/servers/rendering/rendering_device_driver.h index 953401e9bdf..110f8e3f1ee 100644 --- a/servers/rendering/rendering_device_driver.h +++ b/servers/rendering/rendering_device_driver.h @@ -454,6 +454,9 @@ public: // Retrieve the render pass that can be used to draw on the swap chain's framebuffers. virtual RenderPassID swap_chain_get_render_pass(SwapChainID p_swap_chain) = 0; + // Retrieve the rotation in degrees to apply as a pre-transform. Usually 0 on PC. May be 0, 90, 180 & 270 on Android. + virtual int swap_chain_get_pre_rotation_degrees(SwapChainID p_swap_chain) { return 0; } + // Retrieve the format used by the swap chain's framebuffers. virtual DataFormat swap_chain_get_format(SwapChainID p_swap_chain) = 0;