Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android low FPS and GPU timer not updating after 73eff10 #88355

Closed
roalyr opened this issue Feb 15, 2024 · 10 comments · Fixed by #88361
Closed

Android low FPS and GPU timer not updating after 73eff10 #88355

roalyr opened this issue Feb 15, 2024 · 10 comments · Fixed by #88361

Comments

@roalyr
Copy link

roalyr commented Feb 15, 2024

Tested versions

Bug appears in 4.x 73eff10

Previous commit f317cc7 doesn't have the bug

(builds are here: https://github.com/roalyr/godot-for-3d-open-worlds/releases/tag/Test2 )

System information

Lenovo TB350FU, ARM Mali-G57 MC2 and Motorola moto g(60), Adreno 618, all in Vulkan mobile mode.

Issue description

  1. GPU timer freezes and rarely updates.
  2. FPS is lower than expected in 3D scenes (bug-free editor has 2 - 2.5 more FPS, both in editor itself and when launching the scene).
  3. Occasionally prints out error:
drivers/vulkan/rendering_device_driver_vulkan.cpp:2538 - Condition "err != VK_SUCCESS" is true. Returning: ERR_CANT_CREATE

Steps to reproduce

Compile Android editor of both version (see commit numbers above) and test 3D scenes that are expected to render at less FPS than screen refresh rate (moderately demanding renders). This way on the same scenes the difference in FPS will be clearly visible.

Minimal reproduction project (MRP)

Any (I noticed on this scene (just launch it after cloning the repo): https://github.com/roalyr/GD4Lancer/blob/main/Scenes/Stellar/Nebulas/Nebula.tscn the FPS difference was 17-18 vs 38-42 less or more).

@roalyr
Copy link
Author

roalyr commented Feb 15, 2024

@akien-mga akien-mga added this to the 4.3 milestone Feb 15, 2024
@akien-mga akien-mga moved this from Unassessed to Release Blocker in 4.x Release Blockers Feb 15, 2024
@akien-mga akien-mga moved this from Release Blocker to Immediate Blocker in 4.x Release Blockers Feb 15, 2024
@DarioSamo
Copy link
Contributor

DarioSamo commented Feb 15, 2024

A few things to confirm that come to mind:

  • If this behavior actually happens in upstream Godot or only in the fork you linked.
  • Testing with and without V-Sync to see if behavior is any different.
  • Adding some printing to swap chain resizing to see if for some reason it's not constantly being triggered.

There's not many much behavior changes I can think of yet that could trigger such an issue. I'll try to test myself on Android but I don't recall other people noticing any such behavior when testing the PR on the platform, so I'm a bit lost on what it could possibly be at the moment.

@DarioSamo
Copy link
Contributor

If this behavior actually happens in upstream Godot or only in the fork you linked.

I can confirm this one and the frametime updates break as well in upstream, so it's not the fork.

@DarioSamo
Copy link
Contributor

Adding some printing to swap chain resizing to see if for some reason it's not constantly being triggered.

This is what's constantly happening for some reason apparently!

@DarioSamo
Copy link
Contributor

It seems we're stuck in a loop where VK_SUBOPTIMAL_KHR is returned during present no matter what we do in Android.

I'm not sure how we can retrieve why it does this, but whatever we're doing to recreate the swap chain. It could be easily explained why the previous version didn't trigger this bug: it just ignored suboptimal.

I'll try to confirm whether the previous version was running into this as well and simply ignored it or if the swap chain recreation we're doing right now is missing something from doing it as optimal.

@DarioSamo
Copy link
Contributor

It seems indeed the previous version has always triggered VK_SUBOPTIMAL_KHR in android, it just chose to ignore it during presentation and it never led to resizing. I'll investigate as to why it never achieves the optimal presentation but I suspect it might be due to something...

@DarioSamo
Copy link
Contributor

DarioSamo commented Feb 15, 2024

So my suspicion was right.

Godot does not handle screen rotation natively, as it always creates the swap chain with VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR despite the current transform being different (for example VK_SURFACE_TRANSFORM_ROTATE_90_BIT_KHR on my landscape mode during testing).

The error goes away if I use the transform that is marked as the current transform, but the screen looks distorted and incorrect.

image

But the constant resizing error goes away.

Since it's unlikely Godot will get modified to support screen rotations natively, I guess our only choice is to handle Suboptimal as a valid result. I'll do a PR and we can verify if it fixes the error. It's not the best solution but at least it'll bring it back to the behavior previous to the commit: it just basically exposed an underlying error that was being suppressed before. Therefore it should be clearly documented in code as well that the behavior is intentional.

@roalyr
Copy link
Author

roalyr commented Feb 15, 2024

A reminder to self to to open (or link to existing) an issue related to mouse handling on Android when dragging sliders and rotation gizmo in 3D view (left-right mouse motion is perceived as up-down and vice versa). Might also be related to poor orientation handling.

@akien-mga
Copy link
Member

CC @m4gr3d for context.

@m4gr3d
Copy link
Contributor

m4gr3d commented Feb 15, 2024

So my suspicion was right.

Godot does not handle screen rotation natively, as it always creates the swap chain with VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR despite the current transform being different (for example VK_SURFACE_TRANSFORM_ROTATE_90_BIT_KHR on my landscape mode during testing).

The error goes away if I use the transform that is marked as the current transform, but the screen looks distorted and incorrect.

image

But the constant resizing error goes away.

Since it's unlikely Godot will get modified to support screen rotations natively, I guess our only choice is to handle Suboptimal as a valid result. I'll do a PR and we can verify if it fixes the error. It's not the best solution but at least it'll bring it back to the behavior previous to the commit: it just basically exposed an underlying error that was being suppressed before. Therefore it should be clearly documented in code as well that the behavior is intentional.

@DarioSamo Are we applying this vulkan Android optimization tip, I wonder if that's the cause of the issue you're seeing when applying the current transform.

As mentioned in the tip, you may need to set the VkSwapchainCreateInfoKHR::preTransform field to report that the transform was applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Immediate Blocker
Development

Successfully merging a pull request may close this issue.

4 participants