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

Fixed screen twitches. #71

Closed
wants to merge 1 commit into from
Closed

Conversation

bibendovsky
Copy link
Contributor

@DanielGibson
Copy link
Member

I haven't tested that, but removing sound code is supposed to fix "screen
twitches"?

On Fri, May 17, 2013 at 7:35 AM, bibendovsky notifications@github.comwrote:

Video sample (not mine):

http://www.youtube.com/watch?v=ZUohifAbPW0

You can merge this Pull Request by running

git pull https://github.com/bibendovsky/dhewm3 patch-1

Or view, comment on, or merge it at:

#71
Commit Summary

  • Fixed screen twitches.

File Changes

Patch Links:

@bibendovsky
Copy link
Contributor Author

By removing this code I enforces to use correct calculation of amplitude, like it was done by original (non-OpenAL) sound backend (see idSoundWorldLocal::FindAmplitude in snd_world.cpp).

P.S. (For clarity) By "twitches" I mean very short but noticeable shake of screen (see provided video at 00:04).

@DanielGibson
Copy link
Member

And how does that amplitude influence the rendering/camera (resulting in
"twitches")?

(To be clear: I'm not saying this makes no sense, but I'd like more
explanation on what causes the bug in which way and how the patch prevents
that, because on first sight changing sound code to fix rendering/camera
issues sounds quite strange)

On Fri, May 17, 2013 at 1:52 PM, bibendovsky notifications@github.comwrote:

By removing this code I enforces to use correct calculation of amplitude
on the fly, like it was done by original (non-OpenAL) sound backend (see
idSoundWorldLocal::FindAmplitude in snd_world.cpp).

P.S. (For clarity) By "twitches" I mean very short but noticeable shakes
of screen (see provided video at 00:04).


Reply to this email directly or view it on GitHubhttps://github.com//pull/71#issuecomment-18057235
.

@motorsep
Copy link

Isn't it a feature that allows camera shake based on a sound shader played?

http://www.iddevnet.com/doom3/sounds.php see "shakes"

@bibendovsky
Copy link
Contributor Author

  1. "Changing sound code to fix rendering/camera issues sounds quite strange".
    The engine has a feature to shake the screen when this sound plays (http://www.iddevnet.com/doom3/sounds.php; see "shakes" keyword).

  2. "what causes the bug" - data inaccuracy between OpenAL and system backends.

The result amplitude calculating from a "data window" (DW) at specified sound offset. The size of DW is 512 samples.

a) For system backend samples for DW fetched by decoding sound data.

b) For OpenAL backend samples for DW fetched from precalculated pairs of min/max values at loading time (idSoundSample::Load). A number of theses pairs equals to "sound_size_in_samples / 512" (For example, we have a 1024 samples, so there are 1024 / 512 = 2 pairs).

So, for system backend we will have unique values in DW, but for OpenAL all values will be same.

@bibendovsky
Copy link
Contributor Author

"Isn't it a feature that allows camera shake based on a sound shader played?"

Correct. But I'm talking about abnormal/jerky camera shakes among a normal/smooth one which you can see in video.

@andre-d
Copy link
Contributor

andre-d commented May 17, 2013

I am a bit confused, is the code defective? Why has it been gutted rather than fixed?

@bibendovsky
Copy link
Contributor Author

I am a bit confused, is the code defective? Why has it been gutted rather than fixed?

Because the fixed code already exists. By gutting this code I reroute execution to a correct one.

@andre-d
Copy link
Contributor

andre-d commented May 18, 2013

Why was it there in the first place!

@bibendovsky
Copy link
Contributor Author

Why was it there in the first place!

I don't know why id Software developers left this code. Maybe they would like to gain some speed by precalculating data.

@bibendovsky
Copy link
Contributor Author

An amplitude for a sequence of sound samples calculating by a following formulas (look at bottom of idSoundWorldLocal::FindAmplitude in snd_world.cpp):
A = arctan (( Amax - Amin ) / 32768 / ( Pi / 4 ))
Amax = max ( s[0], s[1], ..., s[i], ..., s[N-2], s[N-1] )
Amin = min ( s[0], s[1], ..., s[i], ..., s[N-2], s[N-1] )
where s[i] - i-th sample value, N - number of samples.

Example
Let a sound data S consist of eight samples ( N = 8 ): {1, 2, 4, 8, 16, 32, 64, 128}.
Assume that at one time we can calculate an amplitude only for four samples (so-called "window"): Nw = 4
(in engine Nw = 512). Now let calculate differences d = Amax - Amin for four offsets: {0, 1, 2, 4}.
(I don't calculate full amplitude A for simplicity).

At-first, system backend.

Offset 0. Set of samples: {1, 2, 4, 8}. Amin = 1. Amax = 8. d = 7.
Offset 1. Set of samples: {2, 4, 8, 16}. Amin = 2. Amax = 16. d = 14.
Offset 2. Set of samples: {4, 8, 16, 32}. Amin = 32. Amax = 4. d = 28.
Offset 4. Set of samples: {16, 32, 64, 128}. Amin = 16. Amax = 128. d = 112.

At-second, OpenAL backend.

At sound load time a cache of pairs { Amin, Amax } created. Number of pairs is equal a number of "windows" in sound data rounded to a greater. For our example these windows are: {1, 2, 4, 8} and {16, 32, 64, 128}. And the cache contains only two pairs: {1, 8} and {16, 128}.

But how an index for cache entry ( cache[i] ) is calculated? It's just division of offset by a window size: c[i] = round ( offset / Nw ). So for offsets {0, 1, 2} we got indices {0, 0, 0} and for offset 5 - {1}. Now let calculate a differences.

Offset 0. Amin = 1. Amax = 8. d = 7;
Offset 1. Amin = 1. Amax = 8. d = 7;
Offset 2. Amin = 1. Amax= 8. d = 7;
Offset 4. Amin = 16. Amax = 128. d = 112;

As you can see, d values for system backend are more "smoothed" then in OpenAL one.

If we want to fix caching we need for each sound's sample calculate Amin / Amax pair.
This means for sample s[0] we need find pair from 512 samples, for sample s[1] another pair from another 512 samples, etc. But it will require twice more memory to store a cache. If memory not problem then I can fix this caching code.

@DanielGibson
Copy link
Member

Hi,

Sorry for not answering, I was busy and then on vacation..
Can you write a quick way to reproduce the bug, i.e. in where in which
level does it happen?

Thanks!

@bibendovsky
Copy link
Contributor Author

Here is a savegame for game/mars_city2 level (noclip is ON). Screen shackes will appear after several seconds after loading the savegame. Also you may fly out of the level to get enormous screen shackes.

@DanielGibson
Copy link
Member

Ok, I just tested it; actually, right after loading mars_city2, both the (intentional) shaking and the twitches can be observed.

I pushed your fix (with slightly changed commit message), thanks for the patch and the elaborate explanation!

@bibendovsky bibendovsky deleted the patch-1 branch September 13, 2013 06:12
kortemik pushed a commit to kortemik/dhewm_hack that referenced this pull request Aug 22, 2014
Caused by inaccuracies in the precached amplitude data in the OpenAL
backend, see dhewm/dhewm3#71 for more details.

Video sample (not mine):
http://www.youtube.com/watch?v=ZUohifAbPW0
The "twitches" can also be observed right at the beginning of the
mars_city2 map.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants