mirror of
https://github.com/enso-org/enso.git
synced 2024-11-23 08:08:34 +03:00
Left align component browser to the edited node (#3636)
[ci no changelog needed] [Task link](https://www.pivotaltracker.com/story/show/181870555) This PR changes the relative position of the edited node in such a way that it is left-aligned to the component browser window. This change reflects the most recent version of the [design doc](https://github.com/enso-org/design/blob/main/epics/component-browser/design.md#overview) <img width="1157" alt="Screenshot 2022-08-08 at 19 15 47" src="https://user-images.githubusercontent.com/6566674/183454192-81960e0a-ab69-43a4-b7df-d13320a9d16d.png"> As an additional change, the FRP implementation of the `Camera2d` was extended with a new output (screen dimensions) and fixed. With the old implementation, there was a possibility of panic at runtime because of non-exclusive borrows of `RefCell`. The FRP event for camera position was emited inside the scope with a mutable `RefCell` borrow. Any attempt to borrow the camera one more time (e.g., by calling one of the getters, such as `zoom()`) caused panic at runtime.
This commit is contained in:
parent
60b1dce79e
commit
698a6a0674
@ -7,6 +7,7 @@ use crate::prelude::*;
|
|||||||
|
|
||||||
use crate::component_browser::component_group::prelude::StyleWatchFrp;
|
use crate::component_browser::component_group::prelude::StyleWatchFrp;
|
||||||
use crate::documentation;
|
use crate::documentation;
|
||||||
|
use crate::graph_editor::component::node::HEIGHT as NODE_HEIGHT;
|
||||||
|
|
||||||
use enso_frp as frp;
|
use enso_frp as frp;
|
||||||
use ensogl::application::frp::API;
|
use ensogl::application::frp::API;
|
||||||
@ -51,6 +52,22 @@ impl component::Model for Model {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl Model {
|
||||||
|
fn expression_input_position(
|
||||||
|
panel_size: &Vector2,
|
||||||
|
panel_x: &f32,
|
||||||
|
vertical_gap: &f32,
|
||||||
|
) -> Vector2 {
|
||||||
|
let half_width = panel_size.x / 2.0;
|
||||||
|
let half_node_height = NODE_HEIGHT / 2.0;
|
||||||
|
let panel_left = panel_x - half_width;
|
||||||
|
let panel_bottom = -panel_size.y / 2.0;
|
||||||
|
let x = panel_left;
|
||||||
|
let y = panel_bottom - vertical_gap - half_node_height;
|
||||||
|
Vector2(x, y)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
impl display::Object for Model {
|
impl display::Object for Model {
|
||||||
fn display_object(&self) -> &display::object::Instance {
|
fn display_object(&self) -> &display::object::Instance {
|
||||||
&self.display_object
|
&self.display_object
|
||||||
@ -76,7 +93,6 @@ impl component::Frp<Model> for Frp {
|
|||||||
model: &Model,
|
model: &Model,
|
||||||
style: &StyleWatchFrp,
|
style: &StyleWatchFrp,
|
||||||
) {
|
) {
|
||||||
use crate::graph_editor::component::node::HEIGHT as NODE_HEIGHT;
|
|
||||||
let network = &frp_api.network;
|
let network = &frp_api.network;
|
||||||
let input = &frp_api.input;
|
let input = &frp_api.input;
|
||||||
let out = &frp_api.output;
|
let out = &frp_api.output;
|
||||||
@ -106,14 +122,11 @@ impl component::Frp<Model> for Frp {
|
|||||||
model.list.input.shown <+ any(&input.show, &init);
|
model.list.input.shown <+ any(&input.show, &init);
|
||||||
out.is_visible <+ bool(&input.hide, &input.show);
|
out.is_visible <+ bool(&input.hide, &input.show);
|
||||||
out.size <+ size;
|
out.size <+ size;
|
||||||
out.expression_input_position <+ all_with4(
|
out.expression_input_position <+ all_with3(
|
||||||
&list_panel.size,
|
&list_panel.size,
|
||||||
&list_position_x,
|
&list_position_x,
|
||||||
&size,
|
|
||||||
&gap,
|
&gap,
|
||||||
|list_size, list_x, size, gap| {
|
Model::expression_input_position
|
||||||
Vector2(*list_x - list_size.x / 6.0, - size.y / 2.0 - gap - NODE_HEIGHT / 2.0)
|
|
||||||
}
|
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
init.emit(());
|
init.emit(());
|
||||||
|
@ -167,6 +167,8 @@ pub struct Frp {
|
|||||||
pub position: frp::Source<Vector3<f32>>,
|
pub position: frp::Source<Vector3<f32>>,
|
||||||
/// Camera zoom factor.
|
/// Camera zoom factor.
|
||||||
pub zoom: frp::Source<f32>,
|
pub zoom: frp::Source<f32>,
|
||||||
|
/// Camera's frustum screen dimensions.
|
||||||
|
pub screen: frp::Source<Screen>,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Function used to return the updated screen dimensions.
|
/// Function used to return the updated screen dimensions.
|
||||||
@ -213,8 +215,9 @@ impl Camera2dData {
|
|||||||
frp::extend! { network
|
frp::extend! { network
|
||||||
frp_position <- source();
|
frp_position <- source();
|
||||||
frp_zoom <- source();
|
frp_zoom <- source();
|
||||||
|
frp_screen <- source();
|
||||||
}
|
}
|
||||||
let frp = Frp { network, position: frp_position, zoom: frp_zoom };
|
let frp = Frp { network, position: frp_position, zoom: frp_zoom, screen: frp_screen };
|
||||||
Self {
|
Self {
|
||||||
frp,
|
frp,
|
||||||
display_object,
|
display_object,
|
||||||
@ -293,8 +296,6 @@ impl Camera2dData {
|
|||||||
self.matrix.view_projection = self.matrix.projection * self.matrix.view;
|
self.matrix.view_projection = self.matrix.projection * self.matrix.view;
|
||||||
let zoom = self.zoom;
|
let zoom = self.zoom;
|
||||||
self.zoom_update_registry.run_all(zoom);
|
self.zoom_update_registry.run_all(zoom);
|
||||||
self.frp.position.emit(self.display_object.position());
|
|
||||||
self.frp.zoom.emit(zoom);
|
|
||||||
}
|
}
|
||||||
changed
|
changed
|
||||||
}
|
}
|
||||||
@ -415,7 +416,12 @@ impl Camera2d {
|
|||||||
impl Camera2d {
|
impl Camera2d {
|
||||||
/// Sets screen dimensions.
|
/// Sets screen dimensions.
|
||||||
pub fn set_screen(&self, width: f32, height: f32) {
|
pub fn set_screen(&self, width: f32, height: f32) {
|
||||||
self.data.borrow_mut().set_screen(width, height)
|
let (endpoint, screen) = {
|
||||||
|
let mut borrowed = self.data.borrow_mut();
|
||||||
|
borrowed.set_screen(width, height);
|
||||||
|
(borrowed.frp.screen.clone_ref(), borrowed.screen)
|
||||||
|
};
|
||||||
|
endpoint.emit(screen);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Resets the zoom of the camera to the 1.0 value.
|
/// Resets the zoom of the camera to the 1.0 value.
|
||||||
@ -425,7 +431,18 @@ impl Camera2d {
|
|||||||
|
|
||||||
/// Update all dirty camera parameters and compute updated view-projection matrix.
|
/// Update all dirty camera parameters and compute updated view-projection matrix.
|
||||||
pub fn update(&self, scene: &Scene) -> bool {
|
pub fn update(&self, scene: &Scene) -> bool {
|
||||||
self.data.borrow_mut().update(scene)
|
let (is_updated, frp, zoom) = {
|
||||||
|
let mut borrowed = self.data.borrow_mut();
|
||||||
|
let is_updated = borrowed.update(scene);
|
||||||
|
let frp = borrowed.frp.clone_ref();
|
||||||
|
let zoom = borrowed.zoom;
|
||||||
|
(is_updated, frp, zoom)
|
||||||
|
};
|
||||||
|
if is_updated {
|
||||||
|
frp.position.emit(self.display_object.position());
|
||||||
|
frp.zoom.emit(zoom);
|
||||||
|
}
|
||||||
|
is_updated
|
||||||
}
|
}
|
||||||
|
|
||||||
// FIXME: This can fail, for example, when during calling the callback another callback is
|
// FIXME: This can fail, for example, when during calling the callback another callback is
|
||||||
@ -525,3 +542,40 @@ impl display::Object for Camera2d {
|
|||||||
&self.display_object
|
&self.display_object
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
// =============
|
||||||
|
// === Tests ===
|
||||||
|
// =============
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
/// A regression test checks whether handling the camera's FRP events does not cause panics
|
||||||
|
/// at runtime.
|
||||||
|
///
|
||||||
|
/// If the events are emitted with the camera's internal [`RefCell`] lock held, the usage of
|
||||||
|
/// camera API methods in event handlers can cause a panic. We need to check methods that use
|
||||||
|
/// both immutable and mutable borrows because, depending on the implementation of
|
||||||
|
/// the camera, it can hold either lock variant while emitting the event.
|
||||||
|
#[test]
|
||||||
|
fn test_frp_endpoints_are_not_causing_refcell_locks() {
|
||||||
|
let app = crate::application::Application::new("root");
|
||||||
|
let camera = app.display.default_scene.camera();
|
||||||
|
let frp = camera.frp();
|
||||||
|
let network = frp::Network::new("TestCamera2d");
|
||||||
|
frp::extend! { network
|
||||||
|
// `zoom()` method uses immutable borrow under the hood.
|
||||||
|
dummy <- frp.position.map(f_!(camera.zoom()));
|
||||||
|
// `set_position` method uses mutable borrow.
|
||||||
|
eval_ dummy(camera.set_position(default()));
|
||||||
|
// `screen` output is fired from the method that uses mutable borrow.
|
||||||
|
eval_ frp.screen(camera.zoom());
|
||||||
|
eval_ frp.screen(camera.set_position(default()));
|
||||||
|
}
|
||||||
|
camera.set_position(Vector3(1.0, 2.0, 3.0));
|
||||||
|
camera.update(&app.display.default_scene);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user