Skip to content

Commit

Permalink
Fix splash screen upside down on Android
Browse files Browse the repository at this point in the history
Fixes an issue introduced in godotengine#96439 (see
godotengine#96439 (comment))

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
godotengine#96439) and now saves the preTransform value in the swapchain.
  • Loading branch information
darksylinc authored and ChrisBase committed Nov 15, 2024
1 parent 7a98c37 commit fc57986
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 53 deletions.
25 changes: 25 additions & 0 deletions drivers/vulkan/rendering_device_driver_vulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 2 additions & 0 deletions drivers/vulkan/rendering_device_driver_vulkan.h
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ class RenderingDeviceDriverVulkan : public RenderingDeviceDriver {
LocalVector<CommandQueue *> command_queues_acquired;
LocalVector<uint32_t> 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;
Expand All @@ -373,6 +374,7 @@ class RenderingDeviceDriverVulkan : public RenderingDeviceDriver {
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;
Expand Down
8 changes: 0 additions & 8 deletions platform/android/display_server_android.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,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;
}
Expand Down
1 change: 0 additions & 1 deletion platform/android/display_server_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ class DisplayServerAndroid : public DisplayServer {

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;
Expand Down
22 changes: 0 additions & 22 deletions platform/android/java/lib/src/org/godotengine/godot/GodotIO.java
Original file line number Diff line number Diff line change
Expand Up @@ -296,28 +296,6 @@ public int getScreenOrientation() {
}
}

/**
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;
}
Expand Down
11 changes: 0 additions & 11 deletions platform/android/java_godot_io_wrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;");
}
}
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 0 additions & 2 deletions platform/android/java_godot_io_wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ class GodotIOJavaWrapper {
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:
Expand Down Expand Up @@ -89,7 +88,6 @@ class GodotIOJavaWrapper {
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);
};

Expand Down
7 changes: 0 additions & 7 deletions servers/display_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -361,13 +361,6 @@ class DisplayServer : public Object {

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;
Expand Down
4 changes: 2 additions & 2 deletions servers/rendering/renderer_rd/renderer_compositor_rd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -238,7 +238,7 @@ void RendererCompositorRD::set_boot_image(const Ref<Image> &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);
Expand Down
9 changes: 9 additions & 0 deletions servers/rendering/rendering_device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<DisplayServer::WindowID, RDD::SwapChainID>::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_

Expand Down
1 change: 1 addition & 0 deletions servers/rendering/rendering_device.h
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,7 @@ class RenderingDevice : public RenderingDeviceCommons {
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);

Expand Down
3 changes: 3 additions & 0 deletions servers/rendering/rendering_device_driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,9 @@ class RenderingDeviceDriver : public RenderingDeviceCommons {
// 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;

Expand Down

0 comments on commit fc57986

Please sign in to comment.