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

[WIP] Adding sensor delay setting for android #22644

Closed

Conversation

BastiaanOlij
Copy link
Contributor

@BastiaanOlij BastiaanOlij commented Oct 2, 2018

Adding a new project setting to be used on Android with which you can set the sensor update frequency. This defaults to the setting "Game" which is good for games (whats in a name), it is a faster update frequency then normal but still retains some accuracy taking out jitter from the accelerometer, gyrometer and magnetometer readings.

For VR however this update frequency is not high enough resulting in a lot of jitter and you need to use the "Fastest" option.

The new setting allows you to set all 4 presets available in Android. The higher/faster the update frequency, the more noise there will be in the readings but the more up to date your reading is.

Fixes #19645

@BastiaanOlij
Copy link
Contributor Author

I lack a good phone to test this on, if anyone could verify my work that be cool


GodotLib.initialize(this, io.needsReloadHooks(), getAssets(), use_apk_expansion);

String sensor_delay_string = GodotLib.getGlobal("input_devices/sensors/sensor_delay");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This call seems to be crashing android for me, I don't know if getGlobal is unavailable at this point of time, or whether our project file hasn't been loaded yet, or if there is some other reason... Anyone have any ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

no idea but I got several reports related to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty much in the dark how this whole thing ties together, it all feels pretty messy to me. So yeah I still don't know if I'm doing this too early, if there is a better place to call this from, or if there is something wrong with getting the setting.

it's a shame because this is the final bit to get simple VR going well on Android :)

Copy link
Contributor

@m4gr3d m4gr3d Apr 2, 2019

Choose a reason for hiding this comment

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

The call is crashing because GodotLib.getGlobal relies on ProjectSettings which is not initialized until GodotLib.setup has been invoked here.

I'm adding another comment below about how to possibly resolve the issue.

@m4gr3d m4gr3d mentioned this pull request Apr 2, 2019
mSensorManager.registerListener(this, mAccelerometer, sensor_delay);
mSensorManager.registerListener(this, mGravity, sensor_delay);
mSensorManager.registerListener(this, mMagnetometer, sensor_delay);
mSensorManager.registerListener(this, mGyroscope, sensor_delay);
Copy link
Contributor

Choose a reason for hiding this comment

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

This code doesn't look correct as the sensor manager is registering listeners here in 'onCreate', and it's also doing the same thing below in 'onResume'.
Based on the Android activity lifecycle, 'onResume' always follows 'onCreate' so the operation is just being repeated twice for no reasons.

I'd recommend deleting the section here in favor of registering the listener in onResume. This would also allow to resolve the crashing issue commented above, as by the time 'onResume' is invoked, GodotLib.setup would have already been invoked, preventing GodotLib.getGlobal from crashing.

@akien-mga akien-mga removed the request for review from reduz April 30, 2019 15:03
@akien-mga
Copy link
Member

Needs a rebase, and I guess some changes after your recent refactor?

@BastiaanOlij
Copy link
Contributor Author

Needs a rebase, and I guess some changes after your recent refactor?

Yeah I just need to find time to actually do it...

@akien-mga akien-mga modified the milestones: 3.2, 4.0 Oct 4, 2019
@akien-mga
Copy link
Member

Moving to 4.0 milestone as it's not mergeable for now. If it can be fixed in coming days though, I'm happy to include it in 3.2.

@aaronfranke aaronfranke marked this pull request as draft April 9, 2020 00:35
@aaronfranke
Copy link
Member

@BastiaanOlij Another reminder that this needs to be rebased on the latest master branch.

Otherwise, abandoned pull requests will be closed in the future as announced here.

@BastiaanOlij
Copy link
Contributor Author

I need to get back into doing stuff with Android, too many other things going on at the moment

@aaronfranke
Copy link
Member

This PR has not received any new commits for over a year, closing. It can be re-opened or re-created if the feature is rebased on the latest master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android sensor lag
5 participants