Skip to content

Commit

Permalink
Refactor FRP for visualisation to fix bugs described in #6673, #6561 …
Browse files Browse the repository at this point in the history 
…as well as some other undiscovered issues around the order of events that arrive at the visualisation container.
  • Loading branch information
@MichaelMauderer
MichaelMauderer committed May 25, 2023
1 parent 0318880 commit ea162d3
Show file tree
Hide file tree
Showing 9 changed files with 212 additions and 233 deletions.
10 changes: 5 additions & 5 deletions app/gui/src/presenter/graph/visualization.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,20 +186,20 @@ impl Visualization {
eval view.visualization_preprocessor_changed (((node, preprocessor)) model.visualization_preprocessor_changed(*node, preprocessor.clone_ref()));
eval view.set_node_error_status (((node, error)) model.error_on_node_changed(*node, error));

update <- source::<(ViewNodeId, visualization_view::Data)>();
set_data <- source::<(ViewNodeId, visualization_view::Data)>();
error_update <- source::<(ViewNodeId, visualization_view::Data)>();
visualization_failure <- source::<ViewNodeId>();
error_vis_failure <- source::<ViewNodeId>();

view.set_visualization_data <+ update;
view.set_visualization_data <+ set_data;
view.set_error_visualization_data <+ error_update;
view.disable_visualization <+ visualization_failure;

eval_ view.visualization_registry_reload_requested (model.load_visualizations());
}

Self { model, _network: network }
.spawn_visualization_handler(notifications, manager, update, visualization_failure)
.spawn_visualization_handler(notifications, manager, set_data, visualization_failure)
.spawn_visualization_handler(
error_notifications,
error_manager,
Expand All @@ -213,15 +213,15 @@ impl Visualization {
self,
notifier: impl Stream<Item = manager::Notification> + Unpin + 'static,
manager: Rc<Manager>,
update_endpoint: frp::Source<(ViewNodeId, visualization_view::Data)>,
set_data_endpoint: frp::Source<(ViewNodeId, visualization_view::Data)>,
failure_endpoint: frp::Source<ViewNodeId>,
) -> Self {
let weak = Rc::downgrade(&self.model);
spawn_stream_handler(weak, notifier, move |notification, model| {
info!("Received update for visualization: {notification:?}");
match notification {
manager::Notification::ValueUpdate { target, data, .. } => {
model.handle_value_update(&update_endpoint, target, data);
model.handle_value_update(&set_data_endpoint, target, data);
}
manager::Notification::FailedToAttach { visualization, error } => {
error!("Visualization {} failed to attach: {error}.", visualization.id);
Expand Down
71 changes: 18 additions & 53 deletions app/gui/view/graph-editor/src/component/node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,12 +317,6 @@ ensogl::define_endpoints_2! {
freeze (bool),
hover (bool),
error (Option<Error>),
/// Whether visualization was permanently enabled (e.g. by pressing the button).
visualization_enabled (bool),
/// Visualization can be visible even when it is not enabled, e.g. when showing preview.
/// Visualization can be invisible even when enabled, e.g. when the node has an error.
visualization_visible (bool),
visualization_path (Option<visualization::Path>),
expression_label_visible (bool),
/// The [`display::object::Model::position`] of the Node. Emitted when the Display Object
/// hierarchy is updated (see: [`ensogl_core::display::object::Instance::update`]).
Expand Down Expand Up @@ -606,12 +600,6 @@ impl NodeModel {
size
}

#[profile(Debug)]
#[allow(missing_docs)] // FIXME[everyone] All pub functions should have docs.
pub fn visualization(&self) -> &visualization::Container {
&self.visualization
}

#[profile(Debug)]
fn set_error(&self, error: Option<&Error>) {
if let Some(error) = error {
Expand Down Expand Up @@ -752,7 +740,6 @@ impl Node {

// === Action Bar ===

let visualization_button_state = action_bar.action_visibility.clone_ref();
out.context_switch <+ action_bar.action_context_switch;
out.skip <+ action_bar.action_skip;
out.freeze <+ action_bar.action_freeze;
Expand Down Expand Up @@ -789,7 +776,11 @@ impl Node {
hover_onset_delay.set_delay(VIS_PREVIEW_ONSET_MS);
hover_onset_delay.set_duration(0.0);

let visualization = &model.visualization.frp;

frp::extend! { network
visualization.set_view_state <+ input.enable_visualization.constant(visualization::ViewState::Enabled);
visualization.set_view_state <+ input.disable_visualization.constant(visualization::ViewState::Disabled);

out.error <+ input.set_error;
is_error_set <- input.set_error.map(
Expand All @@ -805,11 +796,11 @@ impl Node {
}
));

eval input.set_visualization ((t) model.visualization.frp.set_visualization.emit(t));
visualization_enabled_frp <- bool(&input.disable_visualization,&input.enable_visualization);
eval visualization_enabled_frp ((enabled)
model.action_bar.set_action_visibility_state(enabled)
);
// Integration between visualization and action bar.
visualization.set_visualization <+ input.set_visualization;
action_bar.set_action_visibility_state <+ visualization.visible.on_change();
visualization.set_view_state <+ action_bar.user_action_visibility.on_true().constant(visualization::ViewState::Enabled);
visualization.set_view_state <+ action_bar.user_action_visibility.on_false().constant(visualization::ViewState::Disabled);

// Show preview visualisation after some delay, depending on whether we show an error
// or are in quick preview mode. Also, omit the preview if we don't have an
Expand Down Expand Up @@ -840,37 +831,8 @@ impl Node {
preview_enabled <- bool(&hide_preview, &input.show_preview);
preview_visible <- hover_preview_visible || preview_enabled;
preview_visible <- preview_visible.on_change();

// If the preview is visible while the visualization button is disabled, clicking the
// visualization button hides the preview and keeps the visualization button disabled.
vis_button_on <- visualization_button_state.filter(|e| *e).constant(());
vis_button_off <- visualization_button_state.filter(|e| !*e).constant(());
visualization_on <- vis_button_on.gate_not(&preview_visible);
vis_button_on_while_preview_visible <- vis_button_on.gate(&preview_visible);
hide_preview <+ vis_button_on_while_preview_visible;
hide_preview <+ vis_button_off;
action_bar.set_action_visibility_state <+
vis_button_on_while_preview_visible.constant(false);
visualization_enabled <- bool(&vis_button_off, &visualization_on);

visualization_visible <- visualization_enabled || preview_visible;
visualization_visible <- visualization_visible && no_error_set;
visualization_visible_on_change <- visualization_visible.on_change();
out.visualization_visible <+ visualization_visible_on_change;
out.visualization_enabled <+ visualization_enabled;
eval visualization_visible_on_change ((is_visible)
model.visualization.frp.set_visibility(is_visible)
);
out.visualization_path <+ model.visualization.frp.visualisation.all_with(&init,|def_opt,_| {
def_opt.as_ref().map(|def| def.signature.path.clone_ref())
});

// Ensure the preview is visible above all other elements, but the normal visualisation
// is below nodes.
layer_on_hover <- hover_preview_visible.on_false().map(|_| visualization::Layer::Default);
layer_on_not_hover <- hover_preview_visible.on_true().map(|_| visualization::Layer::Front);
layer <- any(layer_on_hover,layer_on_not_hover);
model.visualization.frp.set_layer <+ layer;
visualization.set_view_state <+ preview_visible.on_true().constant(visualization::ViewState::Preview);
visualization.set_view_state <+ preview_visible.on_false().constant(visualization::ViewState::Disabled);

update_error <- all(input.set_error,preview_visible);
eval update_error([model]((error,visible)){
Expand Down Expand Up @@ -948,16 +910,14 @@ impl Node {
// === Type Labels ===

model.output.set_type_label_visibility
<+ visualization_visible.not().and(&no_error_set);
<+ visualization.visible.not().and(&no_error_set);


// === Bounding Box ===

let visualization_size = &model.visualization.frp.size;
// Visualization can be enabled and not visible when the node has an error.
visualization_enabled_and_visible <- visualization_enabled && visualization_visible;
bbox_input <- all4(
&out.position,&new_size,&visualization_enabled_and_visible,visualization_size);
&out.position,&new_size,&visualization.visible,visualization_size);
out.bounding_box <+ bbox_input.map(|(a,b,c,d)| bounding_box(*a,*b,c.then(|| *d)));

inner_bbox_input <- all2(&out.position,&new_size);
Expand Down Expand Up @@ -996,6 +956,11 @@ impl Node {
color::Lcha::transparent()
}
}

/// FRP API of the visualization container attached to this node.
pub fn visualization(&self) -> &visualization::container::Frp {
&self.model().visualization.frp
}
}

impl display::Object for Node {
Expand Down
5 changes: 5 additions & 0 deletions app/gui/view/graph-editor/src/component/node/action_bar.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ ensogl::define_endpoints! {
Input {
set_size (Vector2),
set_visibility (bool),
/// Set whether the `visibility` icon should be toggled on or off.
set_action_visibility_state (bool),
set_action_skip_state (bool),
set_action_freeze_state (bool),
Expand All @@ -86,6 +87,9 @@ ensogl::define_endpoints! {
mouse_over (),
mouse_out (),
action_visibility (bool),
/// The last visibility selection by the user. Ignores changes to the
/// visibility chooser icon made through the input API.
user_action_visibility (bool),
action_context_switch (bool),
action_freeze (bool),
action_skip (bool),
Expand Down Expand Up @@ -419,6 +423,7 @@ impl ActionBar {
// === Icon Actions ===

frp.source.action_visibility <+ model.icons.visibility.state;
frp.source.user_action_visibility <+ model.icons.visibility.last_user_state;
frp.source.action_skip <+ model.icons.skip.state;
frp.source.action_freeze <+ model.icons.freeze.state;
disable_context_button_clicked <- model.icons.context_switch.disable_button.is_pressed.on_true();
Expand Down
Loading

0 comments on commit ea162d3

Please sign in to comment.