Skip to content

Commit

Permalink
Fix visualizations sometimes not opening with space bar. (#5624)
Browse files Browse the repository at this point in the history
Visualizations closing right after opening was caused by the GUI being unresponsive during loading of some visualizations. This caused the timer for measuring the time between space bar press and space bar release to be inflated. The delayed events triggered the "visualization preview mode”, thus closing the visualization has it seemed that the space bar was held down, even though the events just arrived with some delay.

The problem is mitigated by considering the number of frames that have passed between the space and down and the space bar up event, instead of just the wall clock time. If the number of frames is too low, this indicates that frames were dropped to the time is inflated.

Fixes #5223
  • Loading branch information
MichaelMauderer authored Feb 17, 2023
1 parent 0fd390d commit e81d1e3
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 3 deletions.
31 changes: 28 additions & 3 deletions app/gui/view/graph-editor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,13 @@ pub mod prelude {
// =================

const SNAP_DISTANCE_THRESHOLD: f32 = 10.0;
/// Time between key down and key up event to consider it a press and hold action as opposed to a
/// simple key press.
const VIZ_PREVIEW_MODE_TOGGLE_TIME_MS: f32 = 300.0;
/// Number of frames we expect to pass during the `VIZ_PREVIEW_MODE_TOGGLE_TIME_MS` interval.
/// Assumes 60fps. We use this value to check against dropped frames during the interval.
const VIZ_PREVIEW_MODE_TOGGLE_FRAMES: i32 =
(VIZ_PREVIEW_MODE_TOGGLE_TIME_MS / 1000.0 * 60.0) as i32;
const MACOS_TRAFFIC_LIGHTS_CONTENT_WIDTH: f32 = 52.0;
const MACOS_TRAFFIC_LIGHTS_CONTENT_HEIGHT: f32 = 12.0;
/// Horizontal and vertical offset between traffic lights and window border
Expand Down Expand Up @@ -3446,10 +3452,29 @@ fn new_graph_editor(app: &Application) -> GraphEditor {
viz_was_pressed <- viz_pressed.previous();
viz_press <- viz_press_ev.gate_not(&viz_was_pressed);
viz_release <- viz_release_ev.gate(&viz_was_pressed);
viz_press_time <- viz_press . map(|_| web::window.performance_or_panic().now() as f32);
viz_press_time <- viz_press . map(|_| {
let time = web::window.performance_or_panic().now() as f32;
let frame_counter = Rc::new(web::FrameCounter::start_counting());
(time, Some(frame_counter))
});
viz_release_time <- viz_release . map(|_| web::window.performance_or_panic().now() as f32);
viz_press_time_diff <- viz_release_time.map2(&viz_press_time,|t1,t0| t1-t0);
viz_preview_mode <- viz_press_time_diff.map(|t| *t > VIZ_PREVIEW_MODE_TOGGLE_TIME_MS);
viz_preview_mode <- viz_release_time.map2(&viz_press_time,|t1,(t0,counter)| {
let diff = t1-t0;
// We check the time between key down and key up. If the time is less than the threshold
// then it was a key press and we do not want to enter preview mode. If it is longer then
// it was a key hold and we want to enter preview mode.
let long_enough = diff > VIZ_PREVIEW_MODE_TOGGLE_TIME_MS;
// We also check the number of passed frames, since the time measure can be misleading, if
// there were dropped frames. The visualisation might have just appeared while more than
// the threshold time has passed.
let enough_frames = if let Some(counter) = counter {
let frames = counter.frames_since_start();
frames > VIZ_PREVIEW_MODE_TOGGLE_FRAMES
} else {
false
};
long_enough && enough_frames
});
viz_preview_mode_end <- viz_release.gate(&viz_preview_mode).gate_not(&out.is_fs_visualization_displayed);
viz_tgt_nodes <- viz_press.gate_not(&out.is_fs_visualization_displayed).map(f_!(model.nodes.all_selected()));
viz_tgt_nodes_off <- viz_tgt_nodes.map(f!([model](node_ids) {
Expand Down
61 changes: 61 additions & 0 deletions lib/rust/web/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#![feature(specialization)]
#![feature(auto_traits)]
#![feature(unsize)]
#![feature(cell_update)]
// === Standard Linter Configuration ===
#![deny(non_ascii_idents)]
#![warn(unsafe_code)]
Expand All @@ -26,6 +27,7 @@

use crate::prelude::*;

use std::cell::Cell;
use wasm_bindgen::prelude::wasm_bindgen;


Expand Down Expand Up @@ -947,3 +949,62 @@ impl TimeProvider for Performance {
self.now()
}
}


// ====================
// === FrameCounter ===
// ====================

type Counter = Rc<Cell<i32>>;

#[derive(Debug)]
/// A counter that counts the number of frames that have passed since its initialization.
///
/// Uses `request_animation_frame` under the hood to count frames.
pub struct FrameCounter {
frames: Counter,
js_on_frame_handle_id: Rc<Cell<i32>>,
_closure_handle: Rc<RefCell<Option<Closure<(dyn FnMut(f64))>>>>,
}

impl FrameCounter {
/// Creates a new frame counter.
pub fn start_counting() -> Self {
let frames: Counter = default();
let frames_handle = Rc::downgrade(&frames);
let closure_handle = Rc::new(RefCell::new(None));
let closure_handle_internal = Rc::downgrade(&closure_handle);
let js_on_frame_handle_id = Rc::new(Cell::new(0));
let js_on_frame_handle_id_internal = Rc::downgrade(&js_on_frame_handle_id);
*closure_handle.as_ref().borrow_mut() = Some(Closure::new(move |_| {
frames_handle.upgrade().map(|fh| fh.as_ref().update(|value| value.saturating_add(1)));
if let Some(maybe_handle) = closure_handle_internal.upgrade() {
if let Some(handle) = maybe_handle.borrow_mut().as_ref() {
let new_handle_id =
window.request_animation_frame_with_closure_or_panic(handle);
if let Some(handle_id) = js_on_frame_handle_id_internal.upgrade() {
handle_id.as_ref().set(new_handle_id)
}
}
}
}));

js_on_frame_handle_id.as_ref().set(window.request_animation_frame_with_closure_or_panic(
closure_handle.borrow().as_ref().unwrap(),
));

debug_assert!(closure_handle.borrow().is_some());
Self { frames, js_on_frame_handle_id, _closure_handle: closure_handle }
}

/// Returns the number of frames that have passed since the counter was created.
pub fn frames_since_start(&self) -> i32 {
self.frames.as_ref().get()
}
}

impl Drop for FrameCounter {
fn drop(&mut self) {
window.cancel_animation_frame_or_warn(self.js_on_frame_handle_id.get());
}
}

0 comments on commit e81d1e3

Please sign in to comment.