Unify and clean-up Godot Android surface allocation and render thread logic #105529
Conversation
m4gr3d
force-pushed
the
implement_unified_renderer
branch
6 times, most recently
from
eef0958 to
d803cda
Compare
|
I skimmed the code in the 2nd commit and it seems fine to me, but I didn't spend enough time to be comprehensive. I also skimmed over the 3rd and 4th commit with the Kotlin conversion, and that looked OK too. I tested this on the Meta Quest with both OpenGL and Vulkan, and it worked fine! |
m4gr3d
force-pushed
the
implement_unified_renderer
branch
2 times, most recently
from
2d882f8 to
699c179
Compare
m4gr3d
force-pushed
the
implement_unified_renderer
branch
2 times, most recently
from
2d31db6 to
5c814be
Compare
m4gr3d
force-pushed
the
implement_unified_renderer
branch
from
5c814be to
fb3af14
Compare
|
Note for tracking: there's a bug that causes the Android editor to crash when the embedded game window is minimized then brought back to front. |
m4gr3d
force-pushed
the
implement_unified_renderer
branch
2 times, most recently
from
bcc43d2 to
2fee88c
Compare
m4gr3d
force-pushed
the
implement_unified_renderer
branch
from
2fee88c to
f1f6303
Compare
| } | ||
| try { | ||
| view.mRenderer.onSurfaceCreated(gl, mEglHelper.mEglConfig); | ||
| mRenderer.onRenderSurfaceCreated(null); |
There was a problem hiding this comment.
Why does this always pass null as the surface?
There was a problem hiding this comment.
Historically, it's because the native Android OpenGL logic didn't make use of the Android surface given that most of the OpenGL setup was done in the Java layer.
| } | ||
| try { | ||
| view.mRenderer.onSurfaceChanged(gl, w, h); | ||
| mRenderer.onRenderSurfaceChanged(null, mRegisteredGLSurface.mFrameParams.w, mRegisteredGLSurface.mFrameParams.h); |
There was a problem hiding this comment.
Same question here: why always pass null as the surface?
m4gr3d
force-pushed
the
implement_unified_renderer
branch
from
f1f6303 to
1659b4b
Compare
|
To try and check if there were any performance regressions from this PR, I've run the benchmarks built into this fork of the GDQuest TPS demo on the Meta Quest 3, using both OpenGL and the Vulkan mobile renderer. OpenGL frame times (lower is better):
Vulkan mobile frame times (lower is better):
I re-ran both the before and after OpenGL tests a couple times, and the results are consistent: there does seem to be small slow down with this PR. The Vulkan mobile numbers don't seem to have been affected. I don't know how specific my results are to the project that I'm testing or the hardware that I'm testing it on |
m4gr3d
force-pushed
the
implement_unified_renderer
branch
from
1659b4b to
d9f447c
Compare
Intermediary (broken) step to migration the classes to Kotlin while conserving the git history
Final (compiling) step of the migration to Kotlin
…*` APIs and classes
m4gr3d
force-pushed
the
implement_unified_renderer
branch
from
d9f447c to
db82949
Compare
|
@m4gr3d is this PR ready? |
@syntaxerror247 Not yet. This PR is blocked on a performance instrumented tests PR I'm working on, so we can validate that no performance regressions is introduced. I'll switch it to draft for the meantime. |
3 participants
Unification and clean up of the Godot Android render logic.
We go from having two
SurfaceViewrenderers,GodotRendererfor OpenGL andVkRendererfor Vulkan, to a singleGodotRendererwhich covers both.The render thread management logic has been moved from the Godot
SurfaceViewimplementations to theGodotRendererrenderer, allowing to decouple its initialization from theSurfaceViewinitialization, and (for future uses) to use the same render thread to power multiple GodotSurfaceViewviews.Fixes #98381