Skip to content
This repository has been archived by the owner on Dec 28, 2021. It is now read-only.

Fix freezing after inactivity #1776

Merged
merged 13 commits into from
Sep 29, 2021
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,14 @@ these updates be shipped in a stable release before the end of the year.

#### Visual Environment

- [Fixed a bug where edited node expression was sometimes altered.][1743]. When
- [Fixed a bug where edited node expression was sometimes altered.][1743] When
editing node expression, the changes were occasionally reverted, or the
grayed-out parameter names were added to the actual expression.
- [Fixed freezing after inactivity.][1776] When the IDE window was minimized or
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be in the "Next Release" section.

covered by other windows or invisible for any other reason for a duration
around one minute or longer then it would often be frozen for some seconds on
return. Now it is possible to interact with the IDE instantly, no matter how
long it had been inactive.

<br/>

Expand All @@ -59,6 +64,7 @@ these updates be shipped in a stable release before the end of the year.
[1726]: https://github.com/enso-org/ide/pull/1762
[1743]: https://github.com/enso-org/ide/pull/1743
[1744]: https://github.com/enso-org/ide/pull/1744
[1776]: https://github.com/enso-org/ide/pull/1776

# Enso 2.0.0-alpha.10 (2021-07-23)

Expand Down
95 changes: 69 additions & 26 deletions src/rust/ensogl/lib/core/src/animation/physics/inertia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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();
Copy link
Contributor

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 for Value? That might be clearer.

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 don't think we do. But I agree with you. Maybe the Zero trait should be included in the definition of Value. But the code used default() for zero before. It might have been a conscious decision.

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason this subtraction is written as -value, while other ones are value * -1.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 Value, because we do not have subtraction on that trait.

}

/// Compute air drag force. Please note that this is physically incorrect. Read the docs of
Expand All @@ -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 }
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl Status {
};
let duration_delta = max_global_duration - min_global_duration;
let hue_delta = theme.max_time_hue - theme.min_time_hue;
let relative_duration = if duration_delta != 0.0 {
let relative_duration = if duration_delta != 0.0 && !duration_delta.is_nan() {
(duration - min_global_duration) / duration_delta
} else {
0.0
Expand Down
5 changes: 4 additions & 1 deletion src/rust/ide/view/graph-editor/src/profiling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,16 @@ impl Statuses {
frp.source.max_duration <+ min_and_max._1().on_change();
}

frp.source.min_duration.emit(Self::min_and_max(durations.borrow().deref()).0);
frp.source.max_duration.emit(Self::min_and_max(durations.borrow().deref()).1);

Self {frp,durations}
}

fn min_and_max(durations:&BiBTreeMap<NodeId,OrderedFloat<f32>>) -> (f32,f32) {
let mut durations = durations.right_values().copied();

let min = durations.next().map(OrderedFloat::into_inner).unwrap_or(std::f32::INFINITY);
let min = durations.next().map(OrderedFloat::into_inner).unwrap_or(f32::INFINITY);
let max = durations.last().map(OrderedFloat::into_inner).unwrap_or(0.0);
(min, max)
}
Expand Down