Skip to content

Commit

Permalink
Fix visualisation FRP bugs. (#6831)
Browse files Browse the repository at this point in the history 
Fixes
* Empty Visualization when opening a full-screen visualization directly without opening the visualization before. #6770

https://github.com/enso-org/enso/assets/1428930/5812ed03-652c-4a27-8e33-b85512ca11b6

* Empty visualization when opening the full-screen visualization before the data for the visualization has arrived. #6561

https://github.com/enso-org/enso/assets/1428930/d8e58f2d-f1b6-4b70-84fa-e917f6c0af1f

* Visualization is reset to default when reconnecting nodes #6673

https://github.com/enso-org/enso/assets/1428930/ac6cf79a-7147-4f13-9045-52599fb39900


* Redundant internal open/lose events caused by logic loops around the show/hide button, as well as many redundant layer setting/unsetting issues internal to the visualization code.

Generally improves the logic around the visualization API by avoiding decentralized logic in different places and removing old code that is no longer needed.
  • Loading branch information
@MichaelMauderer
MichaelMauderer authored Jun 5, 2023
1 parent db96bd2 commit 72b202b
Show file tree
Hide file tree
Showing 12 changed files with 246 additions and 242 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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
1 change: 1 addition & 0 deletions app/gui/view/graph-editor/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ analytics = { path = "../../analytics" }
ast = { path = "../../language/ast/impl" }
base64 = "0.13"
bimap = { version = "0.4.0" }
derivative = "2.2.0"
engine-protocol = { path = "../../controller/engine-protocol" }
enso-config = { path = "../../config" }
enso-frp = { path = "../../../../lib/rust/frp" }
Expand Down
90 changes: 36 additions & 54 deletions app/gui/view/graph-editor/src/component/node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ ensogl::define_endpoints_2! {
select (),
deselect (),
enable_visualization (),
enable_fullscreen_visualization (),
disable_visualization (),
set_visualization (Option<visualization::Definition>),
set_disabled (bool),
Expand Down Expand Up @@ -318,12 +319,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 @@ -607,12 +602,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 @@ -753,7 +742,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 @@ -790,7 +778,10 @@ 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
enabled <- bool(&input.disable_visualization, &input.enable_visualization);

out.error <+ input.set_error;
is_error_set <- input.set_error.map(
Expand All @@ -806,11 +797,23 @@ 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)
);
viz_enabled <- enabled && no_error_set;
visualization.set_view_state <+ viz_enabled.on_true().constant(visualization::ViewState::Enabled);
visualization.set_view_state <+ viz_enabled.on_false().constant(visualization::ViewState::Disabled);

// Integration between visualization and action bar.
visualization.set_visualization <+ input.set_visualization;
is_enabled <- visualization.view_state.map(|state|{
matches!(state,visualization::ViewState::Enabled)
});
action_bar.set_action_visibility_state <+ is_enabled;
button_set_to_true <- action_bar.user_action_visibility.on_true();
button_set_to_true_without_error <- button_set_to_true.gate_not(&is_error_set);
button_set_to_true_with_error <- button_set_to_true.gate(&is_error_set);
visualization.set_view_state <+ button_set_to_true_without_error.constant(visualization::ViewState::Enabled);
action_bar.set_action_visibility_state <+ button_set_to_true_with_error.constant(false);

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,38 +843,10 @@ impl Node {
hide_preview <+ editing_finished;
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;
vis_preview_visible <- preview_visible && no_error_set;
vis_preview_visible <- vis_preview_visible.on_change();
visualization.set_view_state <+ vis_preview_visible.on_true().constant(visualization::ViewState::Preview);
visualization.set_view_state <+ vis_preview_visible.on_false().constant(visualization::ViewState::Disabled);

update_error <- all(input.set_error,preview_visible);
eval update_error([model]((error,visible)){
Expand All @@ -883,6 +858,10 @@ impl Node {
});

eval error_color_anim.value ((value) model.set_error_color(value));
visualization.set_view_state <+ input.set_error.is_some().constant(visualization::ViewState::Disabled);

enable_fullscreen <- frp.enable_fullscreen_visualization.gate(&no_error_set);
visualization.set_view_state <+ enable_fullscreen.constant(visualization::ViewState::Fullscreen);

}

Expand Down Expand Up @@ -949,16 +928,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 @@ -997,6 +974,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 @@ -412,6 +416,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 72b202b

Please sign in to comment.