Wip/wdanilo/text shape system single scene 183406745 #3776

wdanilo · 2022-10-06T21:38:03Z

Pull Request Description

Implementation of fast fonts rendering 🥳🎉

Changes to glyphs and Shape Systems

Glyphs are using shape system now. When a new font is created in a layer used by another font, the fonts definitions are re-used and no shader re-compilation happens.
The shape system definition was re-written from scratch. It's implementation is simpler, faster, and more extensible.
The ensogl::define_shape_system! macro was renamed to ensogl::shape!. Also, unlike the previous version, it requires now the first style: Style argument always.

Changes to scene and layer management.

Scene is a global entity right now. It simplifies many computations. In particular, when a new shape is created, it does not require Shape Proxy objects anymore, as it can get the scene reference at the creation time. This change does NOT mean that multi-scene projects are impossible. Quite the opposite. The world object instantiates the global scene variable and in case of the existence of multiple scenes, it can instantiate the object multiple times per frame, once per every scene. 2.
A display object can be added to only one layer now. This simplification allows us to make the code faster and simpler. We never used the possibility to add the same object to multiple layers, so this functionality was removed. The api layer.ad_exclusive was changed to simply layer.add. 3.

IntelliJ configuration inclusion.

This PR also includes a sharable IntelliJ run configuration and a script to re-generate it based on available crates. The script is located in build/intellij-run-config-gen. This is how int looks in IntelliJ:

Screen.Recording.2022-10-31.at.08.36.57.mov

Checklist

Please include the following checklist in your PR:

The documentation has been updated if necessary.
All code conforms to the
Scala,
Java,
and
Rust
style guides.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed: Enso GUI was tested when built using BOTH
  ./run ide build and ./run ide watch.

… work

…mpiles, does not work.

kazcw · 2022-10-10T17:14:03Z

lib/rust/prelude/src/range.rs

 where
    R: Clone + RangeOverlap + RangeOps,
-    <R as RangeOps>::Item: Clone + PartialOrd, {
-    let mut ranges = ranges.to_vec();
+    <R as RangeOps>::Item: Clone + Ord, {
    crate::gen_iter!({
        ranges.sort_unstable_by(|a, b| a.start().partial_cmp(b.start()).unwrap());


Now that the bound is Ord, partial_cmp+unwrap can be replaced by cmp.

Oh, right! thanks!

app/gui/view/graph-editor/src/component/breadcrumbs/breadcrumb.rs

MichaelMauderer · 2022-10-31T11:30:14Z

build/intellij-run-config-gen/src/main.rs

+    let md = fs::metadata(path);
+    let md = md.unwrap_or_else(|_| panic!("Could get metadata of {}", path.display()));
+    if md.is_dir() {
+        let file_name = path.file_name().map(|t| t.to_string_lossy().to_string());


folder_name might be clearer, as we just checked that it is a folder.

MichaelMauderer · 2022-10-31T11:33:43Z

build/intellij-run-config-gen/src/main.rs

@@ -0,0 +1,382 @@
+//! Generate IntelliJ run configurations and place them in the .idea/runConfigurations directory.


How are we ensuring this script does not go out of date? Should this be executed on CI to check the output has not changed?

Let's do it as part of fat script (I'll chat with Michal)

MichaelMauderer · 2022-10-31T11:51:57Z

lib/rust/callback/src/lib.rs

+    /// Callback list used to register new callbacks when this registry is running. During run,
+    /// the [`callback_list`] is borrowed and thus can't be mutated.
+    #[derivative(Debug = "ignore")]
+    callback_list_during_run: RefCell<Vec<(Guard, Box<F>)>>,
 }


Some additional information about how these two lists are managed and operated during a run might be good here.
After reading the code, I'd suggest the following change:

Suggested change

/// Callback list used to register new callbacks when this registry is running. During run,

/// the [`callback_list`] is borrowed and thus can't be mutated.

#[derivative(Debug = "ignore")]

callback_list_during_run: RefCell<Vec<(Guard, Box<F>)>>,

}

/// Temporary buffer to store new callbacks that are registered while the registry is running. During a run,

/// the [`callback_list`] is borrowed and thus new callbacks cannot be added because it cannot be mutated.

/// The buffer is processed and emptied after the registry has finished processing the existing callbacks.

#[derivative(Debug = "ignore")]

new_callback_buffer: RefCell<Vec<(Guard, Box<F>)>>,

}

I added the comment, but I did not change the var name. I feel that callback_list_during_run is more descriptive.

MichaelMauderer · 2022-10-31T11:54:43Z

lib/rust/callback/src/lib.rs

+///   [`T2`] will be passed as copies.
+/// - The [`Ref1`] registry accepts callbacks in a form of [`FnMut(&T1)`].
+/// - The [`NoArgs`] registry is a registry for [`FnMut()`] functions.
+/// - The [`NoArgs`] registry is a registry for [`FnMut()`] functions.


Duplicate line.

Suggested change

/// - The [`NoArgs`] registry is a registry for [`FnMut()`] functions.

MichaelMauderer · 2022-10-31T14:00:49Z

lib/rust/ensogl/example/list-view/src/lib.rs

@@ -99,7 +99,7 @@ fn init(app: &Application) {
    list_view.focus();
    app.display.add_child(&list_view);
    // FIXME[WD]: This should not be needed after text gets proper depth-handling.


Does this FIXME need re-evaluating?

MichaelMauderer · 2022-10-31T14:02:11Z

lib/rust/ensogl/example/text-area/src/lib.rs

@@ -161,6 +161,7 @@ pub fn main() {
    });
 }

+// TODO: pub fn extend do define_endpoints_2


MichaelMauderer · 2022-10-31T14:02:23Z

lib/rust/ensogl/example/text-area/src/lib.rs

@@ -170,21 +171,18 @@ fn init(app: Application) {
    let _text = quote.to_string() + snowman + zalgo;
    let _text = "test".to_string();
    // area.set_content("aஓbc🧑🏾de\nfghij\nklmno\npqrst\n01234\n56789");


Commented code and what seems to be testing code.

MichaelMauderer · 2022-10-31T14:08:41Z

lib/rust/prelude/src/lib.rs

+pub use anyhow_macros::*;
+
+#[macro_export]
+macro_rules! ite {


Do we really need this? Seems a bit overkill to replace if cond then { a } else { b } with ite!(cond, a, b).

As agreed via phone, lets leave it for now and lets chat about it on discord.

Changed my mind, removing it :)

MichaelMauderer · 2022-10-31T14:09:03Z

lib/rust/prelude/src/lib.rs

-pub fn init_wasm() {
-    init_tracing(WARN);
+pub fn init_global() {
+    init_tracing(DEBUG);


Suggested change

init_tracing(DEBUG);

init_tracing(WARN);

…e-183406745

MichaelMauderer · 2022-11-02T12:24:25Z

build/cli/src/lib.rs

@@ -815,6 +815,7 @@ pub async fn main_internal(config: enso_build::config::Config) -> Result {
            try_join(git_clean, clean_cache).await?;
        }
        Target::Lint => {
+            println!("LINTING!!!");


Print left in code.

ouch, thanks!

MichaelMauderer · 2022-11-02T12:35:49Z

lib/rust/ensogl/example/complex-shape-system/src/lib.rs

                // DEBUG!("{scene.layers.node_searcher_mask:#?}");
                // DEBUG!("{scene.layers.viz:#?}");


Commented code.

These are left intentionally, there is a comment for it: // These comments are left for easy debugging in the future.. Its not a new code and I think these should be left there.

MichaelMauderer · 2022-11-02T12:36:01Z

lib/rust/ensogl/example/complex-shape-system/src/lib.rs

+    // FIXME: scissor box should not be computed from the left screen border. It should be affected
+    //     by the camera position.


FIXME without owner. (Maybe should just be fixed)

MichaelMauderer · 2022-11-02T12:37:33Z

lib/rust/ensogl/example/grid-view/src/lib.rs

+    // let grid_views_with_headers =
+    //     std::iter::repeat_with(|| setup_grid_view_with_headers(app)).take(3).collect_vec();
+    // let with_hover_mask = [&grid_views_with_headers[2]];
+    // let with_selection_mask = [&grid_views_with_headers[1], &grid_views_with_headers[2]];
    let mut positions = itertools::iproduct!([-450.0, 50.0], [350.0, -50.0]).map(pair_to_vec2);

-    grids_layer.add_exclusive(&plain_grid_view);
+    grids_layer.add(&plain_grid_view);
    plain_grid_view.set_position_xy(positions.next().unwrap());
-    for (view, position) in grid_views_with_headers.iter().zip(positions) {
-        grids_layer.add_exclusive(view);
-        view.set_position_xy(position);
-    }
-
-    let view = &grid_views_with_headers[0];
-    for i in (0..1000).step_by(2) {
-        view.set_column_width((i, 60.0));
-    }
-
-    for view in with_hover_mask {
-        view.hover_highlight_frp().setup_masked_layer(Some(hover_layer.downgrade()));
-        let params = grid_view::simple::EntryParams {
-            bg_color: color::Lcha::from(color::Rgba(0.7, 0.7, 0.9, 1.0)),
-            bg_margin: 0.0,
-            text_offset: 8.0,
-            text_color: color::Lcha::from(color::Rgba(0.9, 0.9, 0.9, 1.0)),
-            ..default()
-        };
-        view.hover_highlight_frp().set_entries_params(params);
-    }
-
-    for view in with_selection_mask {
-        view.selection_highlight_frp().setup_masked_layer(Some(selection_layer.downgrade()));
-        let params = grid_view::simple::EntryParams {
-            bg_color: color::Lcha::from(color::Rgba(0.5, 0.5, 0.5, 1.0)),
-            bg_margin: 0.0,
-            text_color: color::Lcha::from(color::Rgba(1.0, 1.0, 1.0, 1.0)),
-            text_offset: 8.0,
-            ..default()
-        };
-        view.selection_highlight_frp().set_entries_params(params);
-    }
+    // for (view, position) in grid_views_with_headers.iter().zip(positions) {
+    //     grids_layer.add(view);
+    //     view.set_position_xy(position);
+    // }
+
+    // let view = &grid_views_with_headers[0];
+    // for i in (0..3).step_by(2) {
+    //     view.set_column_width((i, 60.0));
+    // }
+
+    // for view in with_hover_mask {
+    //     view.hover_highlight_frp().setup_masked_layer(Some(hover_layer.downgrade()));
+    //     let params = grid_view::simple::EntryParams {
+    //         bg_color: color::Lcha::from(color::Rgba(0.7, 0.7, 0.9, 1.0)),
+    //         bg_margin: 0.0,
+    //         text_offset: 8.0,
+    //         text_color: color::Lcha::from(color::Rgba(0.9, 0.9, 0.9, 1.0)),
+    //         ..default()
+    //     };
+    //     view.hover_highlight_frp().set_entries_params(params);
+    // }
+    //
+    // for view in with_selection_mask {
+    //     view.selection_highlight_frp().setup_masked_layer(Some(selection_layer.downgrade()));
+    //     let params = grid_view::simple::EntryParams {
+    //         bg_color: color::Lcha::from(color::Rgba(0.5, 0.5, 0.5, 1.0)),
+    //         bg_margin: 0.0,
+    //         text_color: color::Lcha::from(color::Rgba(1.0, 1.0, 1.0, 1.0)),
+    //         text_offset: 8.0,
+    //         ..default()
+    //     };
+    //     view.selection_highlight_frp().set_entries_params(params);
+    // }



Lots of commented code.

This is accidental, sorry for that

MichaelMauderer · 2022-11-02T12:38:15Z

lib/rust/ensogl/example/grid-view/src/lib.rs

@@ -231,7 +231,7 @@ fn init(app: &Application) {
    navigator.disable_wheel_panning();

    std::mem::forget(plain_grid_view);
-    std::mem::forget(grid_views_with_headers);
+    // std::mem::forget(grid_views_with_headers);


More commented code.

MichaelMauderer · 2022-11-02T12:48:43Z

lib/rust/prelude/src/range.rs

@@ -177,5 +178,8 @@ mod test {
        test_merged!([0..=1, 1..=2, 2..=3], [0..=3]);
        test_merged!([0..=2, 3..=5, 1..=4], [0..=5]);
        test_merged!([0..=10, 5..=15], [0..=15]);
+
+        test_merged!([0..2, 0..1], [0..2]);
+        test_merged!([0..10, 4..5, 8..11], [0..11]);


This is not really documented and tested, but what happens for non-overlapping ranges or multiple partial but not overall overlapping ranges?
There should be test cases checking the correct output for those. For example,

test_merged!([0..10, 20..30, 40..50], [0..10, 20..30, 40..50]);

test_merged!([0..10, 5..20, 21..30, 25..40], [0..20, 21..30]);
And I would also add this edge case:
test_merged!([0..10, 10..20], [0..20]); (I would assume that to be correct, even if they are only “adjacent” and not overlapping)

We are never using it for such cases, but your right, tests should cover it. Adding them there! thanks for catching it!

…e-183406745

wdanilo added 30 commits September 9, 2022 12:13

wip

90f50c1

Reusing existing cursor

ebb823d

Refactoring

d955dcd

Refactoring

2783cd4

Refactoring

5e392f1

Refactoring

2895e65

Moving locations from code points to byte offsets. Compiles, does not…

1f3c857

… work

Refactoring, not working

5a3fde9

Fixing left/right cursor movement

c053b4e

Fixing cursor movement bugs

9f7fb13

Parametrizing location

af26c0b

Fixing cursor left/right movement for unicode glyphs

020153a

Fixing up/down cursor movement for unicode glyphs

e37f733

Moving selection to Column-based types instead of Byte-based ones. Co…

12ae5ae

…mpiles, does not work.

Fixing cursor movement after moving selection to the column type

28ac0ac

Refactoring

4f453cd

Refactoring

b207f54

Refactoring. Line shape caching disabled.

eece7e4

Working lazy line shape cache update

670c5c1

refactoring

6009003

Update

1ddecc1

Managing default color setting

1cdbf56

Cleaning

71ac3b1

Refactoring

23edb3e

Refactoring

a4fa3b3

Bugs fixing

8cc3839

Adding color animation

d74cfb4

Fixing an error

e767aa4

Fixing bugs

0ba0766

Fixing bugs

72cf887

Applying review from previous PR.

98e9737

kazcw reviewed Oct 10, 2022

View reviewed changes

wdanilo added 7 commits October 12, 2022 17:00

Refactoring

ba75d72

Fixing truncation color update

1cfe273

Fixing bug with improper text styling sometimes

8c9d634

Refactoring

e02bb27

Linting

3c4f8ee

Formatting

588e3e8

Refactoring

99797f5

wdanilo marked this pull request as ready for review October 31, 2022 06:38

wdanilo requested review from MichaelMauderer, farmaazon, JaroslavTulach and mwu-tow as code owners October 31, 2022 06:38

Refactoring and adding intellij run configurations

4cf4798

MichaelMauderer suggested changes Oct 31, 2022

View reviewed changes

wdanilo added 5 commits November 1, 2022 14:00

Merge branch 'develop' into wip/wdanilo/text-shape-system-single-scen…

ad6be6b

…e-183406745

Fixing the text panic

eeb0e76

Refactoring

c5f0a21

Applying review

21caf2d

Refactoring

208349e

MichaelMauderer approved these changes Nov 2, 2022

View reviewed changes

wdanilo added 6 commits November 3, 2022 01:54

Applying review

aea96e0

Fixing tests, linting

5b827c2

Fixing tests

33194ff

Merge branch 'develop' into wip/wdanilo/text-shape-system-single-scen…

1619303

…e-183406745

Linting

e924494

Linting

a6374ff

wdanilo merged commit 48ce68c into develop Nov 3, 2022

wdanilo deleted the wip/wdanilo/text-shape-system-single-scene-183406745 branch November 3, 2022 07:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wip/wdanilo/text shape system single scene 183406745 #3776

Wip/wdanilo/text shape system single scene 183406745 #3776

wdanilo commented Oct 6, 2022 •

edited

Loading

kazcw Oct 10, 2022

wdanilo Nov 1, 2022

MichaelMauderer Oct 31, 2022

MichaelMauderer Oct 31, 2022

wdanilo Oct 31, 2022

MichaelMauderer Oct 31, 2022

wdanilo Nov 1, 2022

MichaelMauderer Oct 31, 2022

wdanilo Nov 1, 2022

MichaelMauderer Oct 31, 2022

MichaelMauderer Oct 31, 2022

MichaelMauderer Oct 31, 2022

MichaelMauderer Oct 31, 2022

wdanilo Nov 1, 2022 •

edited

Loading

wdanilo Nov 1, 2022

MichaelMauderer Oct 31, 2022

MichaelMauderer Nov 2, 2022

wdanilo Nov 2, 2022

MichaelMauderer Nov 2, 2022

wdanilo Nov 2, 2022

MichaelMauderer Nov 2, 2022

wdanilo Nov 2, 2022

MichaelMauderer Nov 2, 2022

wdanilo Nov 2, 2022

MichaelMauderer Nov 2, 2022

MichaelMauderer Nov 2, 2022

wdanilo Nov 2, 2022

		@@ -0,0 +1,382 @@
		//! Generate IntelliJ run configurations and place them in the .idea/runConfigurations directory.

		// DEBUG!("{scene.layers.node_searcher_mask:#?}");
		// DEBUG!("{scene.layers.viz:#?}");

		// FIXME: scissor box should not be computed from the left screen border. It should be affected
		// by the camera position.

Wip/wdanilo/text shape system single scene 183406745 #3776

Wip/wdanilo/text shape system single scene 183406745 #3776

Conversation

wdanilo commented Oct 6, 2022 • edited Loading

Pull Request Description

Changes to glyphs and Shape Systems

Changes to scene and layer management.

IntelliJ configuration inclusion.

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wdanilo Nov 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wdanilo commented Oct 6, 2022 •

edited

Loading

wdanilo Nov 1, 2022 •

edited

Loading