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

[Post-1.0] Add rain collision and muffle weather sounds when under cover #1711

Closed
wants to merge 2 commits into from
Closed

[Post-1.0] Add rain collision and muffle weather sounds when under cover #1711

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented May 17, 2018

Early PR for code review and feedback, these commits add collision for rain particles via PhysicsSystem::castRay.

Option for this is currently placed under [Water] in config files and in the Settings UI, currently defaulting to disabled.

The source for the PhysicsSystem::castRay call is an extrapolated particle origin somewhere high in the sky as castRay will not report a hit if both the source and destination vectors are inside an object (frequent when in a Vivec canton).

Rain sounds are now also pitched-down to sound more muffled depending on how many of the surrounding weather particles are being culled in the collision check, resulting in muffled sounding rain when standing under cover.

@psi29a
Copy link
Member

psi29a commented May 17, 2018

Cool! :)

What's the performance hit for having this turned on? I assume there none when turned off?

@tcotys
Copy link
Contributor

tcotys commented May 17, 2018

Nice,
just to say that, in order to respect real physics (I'm not a real programmer, just a physicist), you cannot change the pitch of the sound (exept when the player is moving fast).
You have to apply a low pass filter around 200-300 Hz (maybe add some reverb).
The result might be better (or at least similar).

There is an example with
edfba68

@akortunov
Copy link
Collaborator

akortunov commented May 17, 2018

Nice feature!

Note: unfortunately, rain water ripples are part of water shader, so an engine still renders them during rain (under bridges over rivers, for example).

What's the performance hit for having this turned on?

Looks like this PR cast ray for every rain particle in scene in every frame.
Also this feature does not work for snow and blight/ash storms yet.

osg::Vec3f pos = cameraPosition + p->getPosition();
osg::Vec3f extrapolatedOrigin = pos - (p->getVelocity() * 100);

MWPhysics::PhysicsSystem::RayResult result = mPhysicsSystem->castRay(extrapolatedOrigin, pos, MWWorld::ConstPtr(), std::vector<MWWorld::Ptr>(), MWPhysics::CollisionType_World);
Copy link
Collaborator

@akortunov akortunov May 17, 2018

Choose a reason for hiding this comment

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

Keep in mind that not all game objects have precise collision mesh. Probably rendering raycasting will give better results.

Or even use the LineSegmentIntersector directly: pass parentNode to WorldCollisionOperator instead of mPhysicsSystem.

@kcat
Copy link
Contributor

kcat commented May 17, 2018

I don't think we want to add collision to the rain particles. Given the number of particles moving around the player, it won't be particularly efficient since our physics aren't very optimized, and the way each particle handles multiple raindrops or snowflakes, with the inexact collision meshes, you'll end up with precipitation still going through some cover or being blocked by seemingly nothing.

I'd still prefer something like this idea to prevent rain and snow from falling through roofs.

I'd probably also hold off on muffling the rain sound until we have a more generic solution for sound occlusion/obstruction, which can be applied to most sounds blocked by geometry (and as mentioned, you don't want to do it by simply lowering the pitch; it's better to use a low-pass filter).

@ghost
Copy link
Author

ghost commented May 18, 2018

Thanks for the comments guys.

I've been running this on my local branch for a little while, just while casually playing the game and I've sunk about 20 hours into playing with this turned on, as well as some more directed performance testing. In my testing I've not been able to bring about any noticable performance drops by enabling this feature, though my PC is probably toward the higher-end of systems OpenMW targets.

Theoretically there should be no performance drops at all when the option is turned off, apart from the comparison in WorldCollisionOperator::operateParticles against mRainCollision, which I assume would be totally negligible on any targeted system.

I was initially using rendering raycasts as opposed to collision raycasts, but that definitely did have a huge impact on performance, unplayably so, therefore I went with collision raycasts instead. Throughout my own playtesting the collision meshes seem to be more than adequate for this, though this is pretty anecdotal and others' experience may vary.

There's certainly some room for optimization here too. The first idea in my head is to not update particles that aren't in the camera's view, though that would invalidate the rain coverage calculations (at least in its current implementation).

These calculations and rain sound processing were put in after the collision, it's definitely less of my focus here. I had initially thought about low-pass filtering but changed my mind when I saw just how easy a pitch-down could be implemented. I'm certainly no physicist, and I thought that, given that the rain sound's main element is similar enough to white noise, a pitch-down was a fairly nice approximation of low-pass filtering.

Also, in terms of the implementation itself, can anyone see any obvious issues? I'm not very experienced with C++ so it's quite possible I've gone about something the wrong way here, and I'd appreciate any feedback regarding the actual code changes in these commits.

Again, thanks for the comments everyone. Even if this idea gets shelved it's really cool to be having this discussion and getting feedback.

@psi29a
Copy link
Member

psi29a commented May 18, 2018

@akortunov I don't think this PR should address the water shader, especially under a bridge because that problem has always been there. That isn't a blocker and will take a bit more work to fix that as well.

@akortunov
Copy link
Collaborator

That isn't a blocker and will take a bit more work to fix that as well.

I did not say that it is a blocker.

About possible issues: in which thread does WorldCollisionOperator work? If it works in own OSG thread, there could be problems with multithreading.

@zinnschlag
Copy link
Contributor

This is an advanced feature that is not required for 1.0. Therefore I nominate it for putting it on hold until 1.0.

@psi29a
Copy link
Member

psi29a commented Jun 13, 2018

It still needs a bit of work I believe, like ray-casting changes and the use of a low-pass filter before this can be considered for inclusion.

@zinnschlag zinnschlag changed the title [Feedback needed] Add rain collision and muffle weather sounds when under cover [Post-1.0] Add rain collision and muffle weather sounds when under cover Jun 13, 2018
@i30817
Copy link

i30817 commented Nov 10, 2018

And a way to turn it of in settings. Suffering from 25 fps here, have a little mercy for terrible gpus will ya?

@davidcernat
Copy link
Member

On a technical level, this should be redone in an entirely different and much more optimized way that is also usable for dust and snow and potentially other types of weather particles that can get added in post-1.0. If you like, I'm fine with doing it myself once 1.0 is out, perhaps based on Chris' ideas.

On an ethical level, OpenMW shouldn't turn a blind eye to the fact that the creator of this pull request is the only coder who teamed up with far-right extremist Malseph to do a GPL-violating hostile takeover of the TES3MP name by releasing something that they had no right to call TES3MP 0.6.3, unless OpenMW wants to create a precedent where anyone taking over the OpenMW name is also tolerated, while also harming relations with TES3MP at a time when it is seriously being considered for inclusion in its parent project, all for the sake of having a framerate-halving take on rain collision.

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

Successfully merging this pull request may close these issues.

8 participants