Android: Stabilize camera lifecycle handling#111871
Merged
Repiteo merged 1 commit intogodotengine:masterfrom Oct 23, 2025
Merged
Android: Stabilize camera lifecycle handling#111871Repiteo merged 1 commit intogodotengine:masterfrom
Repiteo merged 1 commit intogodotengine:masterfrom
Conversation
m4gr3d
reviewed
Oct 21, 2025
m4gr3d
reviewed
Oct 21, 2025
modules/camera/camera_android.h
Outdated
|
|
||
| struct RotationResult { | ||
| int rotationAngle; | ||
| bool shouldMirror; |
Contributor
Author
There was a problem hiding this comment.
I simplified it by using std::optional instead of a struct.
m4gr3d
reviewed
Oct 21, 2025
|
|
||
| val newOrientation = newConfig.orientation | ||
| if (currentOrientation != -1 && currentOrientation != newOrientation) { | ||
| GodotLib.onScreenRotationChange() |
Contributor
There was a problem hiding this comment.
This ensures the screen rotation change callback is invoked on the render thread, similar to the renderer paused / resumed callbacks.
Suggested change
| GodotLib.onScreenRotationChange() | |
| runOnRenderThread { | |
| GodotLib.onScreenRotationChange() | |
| } |
shiena
force-pushed
the
fix/camera_android.2
branch
from
cffd798 to
289ed9b
Compare
Contributor
Author
|
Fixes compilation of camera handlers from 060d312. |
m4gr3d
reviewed
Oct 21, 2025
| */ | ||
| private var renderViewInitialized = false | ||
| private var primaryHost: GodotHost? = null | ||
| private var currentOrientation = -1 |
Contributor
There was a problem hiding this comment.
Let's replace this with:
private var currentConfig = context.resources.configuration
Comment on lines
+762
to
+768
| val newOrientation = newConfig.orientation | ||
| if (currentOrientation != -1 && currentOrientation != newOrientation) { | ||
| runOnRenderThread { | ||
| GodotLib.onScreenRotationChange() | ||
| } | ||
| } | ||
| currentOrientation = newOrientation |
Contributor
There was a problem hiding this comment.
Suggested change
| val newOrientation = newConfig.orientation | |
| if (currentOrientation != -1 && currentOrientation != newOrientation) { | |
| runOnRenderThread { | |
| GodotLib.onScreenRotationChange() | |
| } | |
| } | |
| currentOrientation = newOrientation | |
| if (currentConfig != null && currentConfig.orientation != newConfig.orientation) { | |
| runOnRenderThread { | |
| GodotLib.onScreenRotationChange() | |
| } | |
| } | |
| currentConfig = newConfig |
shiena
force-pushed
the
fix/camera_android.2
branch
from
289ed9b to
a550b6f
Compare
m4gr3d
reviewed
Oct 22, 2025
| } | ||
|
|
||
| #ifdef MODULE_CAMERA_ENABLED | ||
| JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_onScreenRotationChange(JNIEnv *env, jclass clazz) { |
Contributor
There was a problem hiding this comment.
The method should be outside of the #ifdef, and be declared in java_godot_lib_jni.h like the other jni methods.
m4gr3d
reviewed
Oct 22, 2025
| /** | ||
| * Invoked when the screen orientation changes. | ||
| */ | ||
| public static native void onScreenRotationChange(); |
Contributor
There was a problem hiding this comment.
Suggested change
| public static native void onScreenRotationChange(); | |
| static native void onScreenRotationChange(); |
m4gr3d
approved these changes
Oct 22, 2025
Contributor
m4gr3d
left a comment
m4gr3d
left a comment
There was a problem hiding this comment.
Looks good!
Can you squash the commits, and take care of that last comment.
shiena
force-pushed
the
fix/camera_android.2
branch
from
ad5623b to
d3b5248
Compare
AThousandShips
approved these changes
Oct 22, 2025
shiena
force-pushed
the
fix/camera_android.2
branch
from
d3b5248 to
1ff3303
Compare
- Pause camera feeds during lifecycle transitions to avoid crashes - Refresh camera metadata after rotation to keep orientation accurate
shiena
force-pushed
the
fix/camera_android.2
branch
from
1ff3303 to
4483871
Compare
Contributor
Author
|
I have applied the reviews up to this point. |
Contributor
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split from #110720
GodotLib.onScreenRotationChange()and guard image callbacks with a mutex to keep metadata refresh and frame delivery synchronized.I wanted to split the rotation handling and lifecycle event handling, but both rely on refresh_camera_metadata(), so I couldn’t separate them.
This PR provides the following benefits: