Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Android: Make ALooper_pollAll call always non-blocking #255

Merged
merged 1 commit into from Nov 30, 2023

Conversation

srifqi
Copy link
Member

@srifqi srifqi commented Nov 24, 2023

This PR tries to fix minetest/minetest#10842. See there for further discussion.

Implementation details

Original:

((Focused && !Paused) || !Initialized) ? 0 : -1

Since we want Minetest to still run when the text box shows up (Focused is false) and when Minetest is minimised (Paused is true), we remove both variables from the formula.

!Initialized ? 0 : -1

Because of this:

if (!Initialized)
return false;

we need to remove Initialized from the formula to prevent it from making Minetest freeze/stuck waiting a (blocking) ALooper_pollAll call.

0

EDIT: The Visual Studio builds failed due to vcpkg errors.

Copy link
Member

@grorp grorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pausing Minetest completely when the app is paused doesn't make sense in multiplayer. It's also problematic in the mainmenu if the user is currently downloading a package from CDB. So +1 for the concept.

What about line 67 in the same file?

while ((ALooper_pollAll(((Focused && !Paused) || !Initialized) ? 0 : -1, 0, &Events, (void**)&Source)) >= 0)


To avoid an unnecessarily big increase in battery usage from this PR, maybe we could add code to Minetest to stop rendering while the app is paused? Something like this:

diff --git a/include/IrrlichtDevice.h b/include/IrrlichtDevice.h
index 6dcdb49..98c21f7 100644
--- a/include/IrrlichtDevice.h
+++ b/include/IrrlichtDevice.h
@@ -173,6 +173,8 @@ namespace irr
 		/** \return True if window is maximized. */
 		virtual bool isWindowMaximized() const = 0;
 
+		virtual bool isAppPaused() const { return false; };
+
 		//! Checks if the Irrlicht window is running in fullscreen mode
 		/** \return True if window is fullscreen. */
 		virtual bool isFullscreen() const = 0;
diff --git a/source/Irrlicht/Android/CIrrDeviceAndroid.cpp b/source/Irrlicht/Android/CIrrDeviceAndroid.cpp
index b1df885..745ed4f 100644
--- a/source/Irrlicht/Android/CIrrDeviceAndroid.cpp
+++ b/source/Irrlicht/Android/CIrrDeviceAndroid.cpp
@@ -193,6 +193,10 @@ bool CIrrDeviceAndroid::isWindowMinimized() const
 	return !Focused;
 }
 
+bool CIrrDeviceAndroid::isAppPaused() const {
+	return Paused;
+}
+
 void CIrrDeviceAndroid::closeDevice()
 {
 	ANativeActivity_finish(Android->activity);
diff --git a/source/Irrlicht/Android/CIrrDeviceAndroid.h b/source/Irrlicht/Android/CIrrDeviceAndroid.h
index 656c456..f5a88a5 100644
--- a/source/Irrlicht/Android/CIrrDeviceAndroid.h
+++ b/source/Irrlicht/Android/CIrrDeviceAndroid.h
@@ -36,6 +36,8 @@ namespace irr
 
 		virtual bool isWindowMinimized() const;
 
+		virtual bool isAppPaused() const;
+
 		virtual void closeDevice();
 
 		virtual void setResizable(bool resize = false);
diff --git a/src/client/game.cpp b/src/client/game.cpp
index cc88915474..4c9a8ae0bc 100644
--- a/src/client/game.cpp
+++ b/src/client/game.cpp
@@ -1251,7 +1251,8 @@ void Game::run()
 		updateCamera(dtime);
 		updateSound(dtime);
 		processPlayerInteraction(dtime, m_game_ui->m_flags.show_hud);
-		updateFrame(&graph, &stats, dtime, cam_view);
+		if (!device->isAppPaused())
+			updateFrame(&graph, &stats, dtime, cam_view);
 		updateProfilerGraphs(&graph);
 
 		// Update if minimap has been disabled by the server

@srifqi
Copy link
Member Author

srifqi commented Nov 26, 2023

What about line 67 in the same file?

I am not sure about that line. I have not tested it. Since that is run in initialisation step, it is probably fine to not change it.


To avoid an unnecessarily big increase in battery usage from this PR, maybe we could add code to Minetest to stop rendering while the app is paused? Something like this:

That looks good. The frame/window would not be shown anyway, but we still need the engine to run for other background tasks. It is probably better to also use conditional inclusion (#ifdef directive) since it is only used for Android devices.

@sfan5
Copy link
Member

sfan5 commented Nov 26, 2023

To avoid an unnecessarily big increase in battery usage from this PR, maybe we could add code to Minetest to stop rendering while the app is paused?

take a look at isWindowActive, it already enables exactly this

Copy link
Member

@grorp grorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I think it would be good to merge this together with minetest/minetest#14058.

@sfan5
Copy link
Member

sfan5 commented Nov 30, 2023

I think that change in MT is a bit risky to pull in last minute.

@sfan5 sfan5 merged commit a2884e4 into minetest:master Nov 30, 2023
10 of 14 checks passed
@srifqi srifqi deleted the fix_android_freeze branch December 1, 2023 00:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Players remain stuck in mid air when they open the chat
3 participants