This repository has been archived by the owner on Dec 28, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 34
Fix freezing after inactivity #1776
Merged
Merged
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
3a93efe
Fixed build script on Windows.
mwu-tow d09ff7a
not needed after all
mwu-tow b893a50
Fix freezing after inactivity
s9ferech 174baaf
Merge branch 'develop' into wip/fr/freezing
s9ferech f4980b1
Fix whitespace
s9ferech c00e8f8
Update CHANGELOG.md
s9ferech 6c57507
Requested changes
s9ferech 2a17e6a
Merge branch 'develop' into wip/fr/freezing
s9ferech 47459ee
Merge branch 'develop' into wip/fr/freezing
MichaelMauderer 2f27878
Requested changes
s9ferech 44cbffb
Fix duplication in CHANGELOG
s9ferech 72aef5e
Merge branch 'develop' into wip/fr/freezing
MichaelMauderer 8443a17
Merge branch 'develop' into wip/fr/freezing
MichaelMauderer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -220,14 +220,30 @@ impl Thresholds { | |
/// ``` | ||
#[derive(Clone,Copy,Debug,Default)] | ||
pub struct SimulationData<T> { | ||
value : T, | ||
target_value : T, | ||
velocity : T, | ||
mass : Mass, | ||
spring : Spring, | ||
drag : Drag, | ||
thresholds : Thresholds, | ||
active : bool, | ||
// We store the current value as an offset from the target rather than an absolute value. This | ||
// reduces numerical errors when animating floating point numbers: The offset will become very | ||
// small towards the end of the animation. Small floating point numbers offer higher precision | ||
// than large ones, because more digits can be used behind the point, for the fractional part of | ||
// the number. This higher precision helps us to avoid non-termination that could otherwise | ||
// happen due to rounding errors in our `step` function. | ||
// | ||
// For example: The precision of `f32` values is so low that we can only represent every second | ||
// integer above 16 777 216. If we simulate values that large and represented the simulation's | ||
// state by its current total value then this internal state would have to jump over those gaps. | ||
// The animation would either become to fast (if we rounded the steps up) or slow down too early | ||
// (if we rounded the steps down). Generally, it would be difficult to handle the rounding | ||
// errors gracefully. By representing the state as an offset, we achieve the highest possible | ||
// precision as the animation approaches its target. Large rounding errors might only happen | ||
// when the simulation is still far away from the target. But in those situations, high | ||
// precision is not as important. | ||
offset_from_target : T, | ||
target_value : T, | ||
velocity : T, | ||
mass : Mass, | ||
spring : Spring, | ||
drag : Drag, | ||
thresholds : Thresholds, | ||
active : bool, | ||
} | ||
|
||
impl<T:Value> SimulationData<T> { | ||
|
@@ -240,29 +256,26 @@ impl<T:Value> SimulationData<T> { | |
fn step(&mut self, delta_seconds:f32) { | ||
if self.active { | ||
let velocity = self.velocity.magnitude(); | ||
let distance = (self.value + self.target_value * -1.0).magnitude(); | ||
let distance = self.offset_from_target.magnitude(); | ||
let snap_velocity = velocity < self.thresholds.speed; | ||
let snap_distance = distance < self.thresholds.distance; | ||
let should_snap = snap_velocity && snap_distance; | ||
if should_snap { | ||
self.value = self.target_value; | ||
self.velocity = default(); | ||
self.active = false; | ||
if should_snap || distance.is_nan() { | ||
self.offset_from_target = default(); | ||
self.velocity = default(); | ||
self.active = false; | ||
} else { | ||
let force = self.spring_force() + self.drag_force(); | ||
let acceleration = force * (1.0 / self.mass.value); | ||
self.velocity = self.velocity + acceleration * delta_seconds; | ||
self.value = self.value + self.velocity * delta_seconds; | ||
let force = self.spring_force() + self.drag_force(); | ||
let acceleration = force * (1.0 / self.mass.value); | ||
self.velocity = self.velocity + acceleration * delta_seconds; | ||
self.offset_from_target = self.offset_from_target + self.velocity * delta_seconds; | ||
} | ||
} | ||
} | ||
|
||
/// Compute spring force. | ||
fn spring_force(&self) -> T { | ||
let value_delta = self.target_value + self.value * -1.0; | ||
let spring_stretch = value_delta.magnitude(); | ||
let coefficient = spring_stretch * self.spring.value; | ||
value_delta.normalize() * coefficient | ||
self.offset_from_target * -self.spring.value | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason this subtraction is written as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The first one is more readable. But the second one is necessary for values satisfying only the trait |
||
} | ||
|
||
/// Compute air drag force. Please note that this is physically incorrect. Read the docs of | ||
|
@@ -287,7 +300,7 @@ impl<T:Value> SimulationData<T> { | |
|
||
#[allow(missing_docs)] | ||
impl<T:Value> SimulationData<T> { | ||
pub fn value (&self) -> T { self.value } | ||
pub fn value (&self) -> T { self.target_value + self.offset_from_target } | ||
pub fn target_value (&self) -> T { self.target_value } | ||
pub fn velocity (&self) -> T { self.velocity } | ||
pub fn mass (&self) -> Mass { self.mass } | ||
|
@@ -310,12 +323,14 @@ impl<T:Value> SimulationData<T> { | |
|
||
pub fn set_value(&mut self, value:T) { | ||
self.active = true; | ||
self.value = value; | ||
self.offset_from_target = value + self.target_value * -1.0; | ||
} | ||
|
||
pub fn set_target_value(&mut self, target_value:T) { | ||
self.active = true; | ||
let old_target_value = self.target_value; | ||
self.target_value = target_value; | ||
self.offset_from_target = old_target_value + self.offset_from_target + target_value * -1.0; | ||
} | ||
|
||
pub fn update_value<F:FnOnce(T)->T>(&mut self, f:F) { | ||
|
@@ -350,9 +365,9 @@ impl<T:Value> SimulationData<T> { | |
|
||
/// Stop the animator and set it to the target value. | ||
pub fn skip(&mut self) { | ||
self.active = false; | ||
self.value = self.target_value; | ||
self.velocity = default(); | ||
self.active = false; | ||
self.offset_from_target = default(); | ||
self.velocity = default(); | ||
} | ||
} | ||
|
||
|
@@ -723,3 +738,31 @@ impl Default for EndStatus { | |
Self::Normal | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
||
/// We test that simulations with target value `f32::NaN` terminate and that their final value | ||
/// is in fact NaN. | ||
#[test] | ||
fn animation_to_nan() { | ||
let mut data = SimulationData::<f32>::new(); | ||
data.set_value(0.0); | ||
data.set_target_value(f32::NAN); | ||
data.step(1.0); | ||
assert!(data.value().is_nan()); | ||
assert!(!data.active); | ||
} | ||
|
||
/// We test that simulations with start value `f32::NaN` terminate and reach their target. | ||
#[test] | ||
fn animation_from_nan() { | ||
let mut data = SimulationData::<f32>::new(); | ||
data.set_value(f32::NAN); | ||
data.set_target_value(0.0); | ||
data.step(1.0); | ||
assert_eq!(data.value(),0.0); | ||
assert!(!data.active); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a
::zero()
method available forValue
? That might be clearer.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we do. But I agree with you. Maybe the
Zero
trait should be included in the definition ofValue
. But the code useddefault()
for zero before. It might have been a conscious decision.