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

Make physics framerate configurable #1479

Merged
2 commits merged into from Sep 27, 2017
Merged

Make physics framerate configurable #1479

2 commits merged into from Sep 27, 2017

Conversation

akortunov
Copy link
Collaborator

physicsDt = 1/60 is too much for low-clocked CPUs, so I tried to make it configurable.
At least this PR will allow developers with an old hardware to test an engine.

FYI: I played with 30 fps physics about dozen of hours on my old laptop, and I saw no difference excepts FPS in large cities.

:Range: 10.0 to 60.0
:Default: 60.0

Allow to set how frequently an engine will do physics calculations. A default falue is 60 times per second.
Copy link
Member

Choose a reason for hiding this comment

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

s/falue/value/


Allow to set how frequently an engine will do physics calculations. A default falue is 60 times per second.
This setting allow developers with low-clocked CPUs to test an engine.
Changing from default value can lead to physics bugs. Change this setting on your own risk, and reset a value to default before filling a physics-related bugreport!
Copy link
Member

Choose a reason for hiding this comment

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

Such as? You tested for awhile at 30, did you notice any?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did not notice any problem, but maybe I just missed something. Take a look at this topic.

@kcat
Copy link
Contributor

kcat commented Sep 26, 2017

I don't think this is a good idea. We want physics to be determinable, that is the same output for the same input. Allowing users to change it would mess that up, especially as many people wouldn't understand exactly what can change by changing it and just see it as a way to improve performance (I bet even most developers wouldn't fully understand all that can go different or wrong from tweaking it).

It'd be far better to find out how fix the slowness of physics, given that we're not using Bullet optimally (IIRC, we're doing far more ray tests than we should be). We should also try looking at doing frame interpolation/tweening, so we don't have to run AI and physics every frame or at a high fixed rate to maintain smooth motion.

@@ -1357,7 +1358,11 @@ namespace MWPhysics
mMovementResults.clear();

mTimeAccum += dt;
const float physicsDt = 1.f/60.0f;

static const float physFramerate = Settings::Manager::getFloat("physics framerate", "Physics");
Copy link
Contributor

Choose a reason for hiding this comment

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

You should avoid local static const variables that are initialized like this in time-sensitive code. They aren't actually initialized until the first time the code is reached and thus needs hidden atomic checks, branches, and locks to ensure it's initialized only once and are safely reentrant (the compiler can't know if multiple threads may hit that code at the same time).

Copy link

Choose a reason for hiding this comment

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

Is it really that time sensitive code though? This function is only called once per frame.

@ghost
Copy link

ghost commented Sep 26, 2017

What hardware are we talking about? Not seeing any problem with fairly old hardware I have. That said
I agree we should provide a way to run OpenMW if the hardware just isn't fast enough (or is slowed down by a debugger, valgrind, or something like that). After all, a slightly inaccurate game is better than no game at all.

I don't think this is a good idea. We want physics to be determinable, that is the same output for the same input. Allowing users to change it would mess that up, especially as many people wouldn't understand exactly what can change by changing it and just see it as a way to improve performance (I bet even most developers wouldn't fully understand all that can go different or wrong from tweaking it).

As far as I can tell, Bullet's convexCast should work the same regardless of timesteps, but apparently it didn't back then when we had to introduce the fixed framerate. Might be there were bugs in bullet, who knows if they're still there. Some of our own logic might still be framerate dependent, but it shouldn't lead to big issues. @akortunov Did you test what happens for extremely low framerate values? The 'stepping' logic might be a little slower, but things like clipping inside objects should (theoretically) not happen.

(IIRC, we're doing far more ray tests than we should be).

Not anymore I don't think. There was a PR by logzero that improved the movement code a lot.

Anyway I'd prefer a solution that doesn't involve settings. Can we autodetect when the physics loop is going into a death-spiral and change to non-deterministic (variable) framerate in that case? Preferably with a warning logged to the console saying something along the lines of 'your computer is too slow to run OpenMW accurately'.

@ghost
Copy link

ghost commented Sep 26, 2017

We should also try looking at doing frame interpolation/tweening, so we don't have to run AI and physics every frame or at a high fixed rate to maintain smooth motion.

Physics interpolation is already done, isn't it?

@ghost
Copy link

ghost commented Sep 26, 2017

On a related note, I'm considering to use environment variables for engine-related or more 'dangerous' settings like this one. That'll make it harder for users to mess things up. Only power users know what environments variables are or how to set them, and if they are set, they're usually set for the scope of one run (i.e. OPENMW_SOMESETTING=XYZ ./openmw) which makes it harder to unintentionally leave bad values behind in a file somewhere.

@akortunov
Copy link
Collaborator Author

akortunov commented Sep 26, 2017

What hardware are we talking about?

I had a problem with physics performance on my laptop with i7-3632QM 2.2GHz CPU. Actually, I had to use this patch to be able to run this game with the playable FPS.
IIRC, some low-clocked AMD CPUs are affected too.

Did you test what happens for extremely low framerate values?

Do you mean physics framerate values? With "physics framerate" < 10 actors moving a bit strange (e.g glide on the earth surface). With value > 60 actors do not move at all.

Also I like an idea to use environment variable instead of setting.

@kcat
Copy link
Contributor

kcat commented Sep 26, 2017

What hardware are we talking about? Not seeing any problem with fairly old hardware I have.

There are still issues with mods that have badly optimized (i.e. completely unoptimized, highly detailed) physics shapes that can be collided with causing the framerate to drop. There was a recent mention about a mod that added a bunch of sitting NPCs to the House of Earthly Delights at night, where those NPCs were sitting on and continually colliding with unoptimized meshes causing the frame rate to tank in OpenMW. Though I can't remember if it was on the issue tracker or forums somewhere.

Anyway I'd prefer a solution that doesn't involve settings. Can we autodetect when the physics loop is going into a death-spiral and change to non-deterministic (variable) framerate in that case?

That just makes the problem of variable physics rates worse, since it'll be non-deterministic when it goes into non-deterministic mode. Do we want to have to deal with people reporting bugs caused by physics becoming non-determinable, which they and we will only see when we check the log?

Perhaps a somewhat better option would be to increase the minimum framerate. We currently allow up to 200ms per frame, or 5 fps, before letting the game desync from real-time. We could increase that to 10 or 15, or maybe even 20, so the death-spiral won't drop the frame rate as badly.

Physics interpolation is already done, isn't it?

I don't believe it is for actor movement (which includes gravity).

@Aesylwinn
Copy link
Contributor

Just throwing it out there, but you could make the time step 1/30. Shouldn't be a problem unless you think penetration is an issue. Also, you may want to consider the android port in case someone picks that up again.

@ghost ghost merged commit e5d1fd0 into OpenMW:master Sep 27, 2017
@ghost
Copy link

ghost commented Sep 27, 2017

Also I like an idea to use environment variable instead of setting.

We already used some environment variables before. Anyway I'm wondering where to document them. Maybe the Testing wiki page?

@akortunov akortunov deleted the physics branch October 4, 2017 13:45
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants