Remove glyph FRP #5725

kazcw · 2023-02-21T23:45:23Z

Pull Request Description

Implements #5724. Cuts new_glyph time in half. I'm looking in to the remainder of the time...

Important Notes

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.

enso-bot · 2023-02-22T06:36:59Z

Keziah Wesley reports a new STANDUP for today (2023-02-21):

Progress: Checked out text rendering. Removing glyph FRP (#5725) reduced cost by 30%. Pre-computing MSDFs should eliminate an additional 40% (#5722). It should be finished by 2023-02-22.

Next Day: Next day I will be working on the #5722 task. See if there's anything else I can cut from new_glyph (#5725), then start on pre-computing MSDFs.

kazcw · 2023-02-22T18:09:25Z

Most of the remaining time in new_glyph is spent creating the display::Object and instantiating the shape system:

self_duration	total_duration	count	profiler
220.7	220.7	599	instantiate (lib/rust/ensogl/core/src/display/shape/primitive/system.rs:303)
107.2	107.2	293	new (lib/rust/ensogl/core/src/display/object/instance.rs:1122)
73.0	401.3	1107	new_glyph (lib/rust/ensogl/component/text/src/font/glyph.rs:552)
0.4	0.4	1	new (lib/rust/ensogl/core/src/display/shape/primitive/system.rs:272)

These are worth looking in to after more impactful planned improvements like #5722.

wdanilo · 2023-02-22T21:44:13Z

@kazcw , what about updating the text-area debug scene? Have you tried it? After removing these FRP bindings here, the scene does not allow for testing bigger glyphs, different colors, etc. It would be cool to allow for it by implementing the removed logic there. Have you tried doing it?

kazcw · 2023-02-23T21:35:56Z

@kazcw , what about updating the text-area debug scene? Have you tried it? After removing these FRP bindings here, the scene does not allow for testing bigger glyphs, different colors, etc. It would be cool to allow for it by implementing the removed logic there. Have you tried doing it?

Implemented. The bindings to change hinting are now Ctrl+(Shift)+E to increase (decrease) opacity_exponent, and Ctrl+(Shift)+O to increase (decrease) opacity_increment. Other bindings are unchanged. Internally, hinting parameters are now maintained per font, not per glyph.

enso-bot · 2023-02-24T06:09:51Z

Keziah Wesley reports a new 🔴 DELAY for yesterday (2023-02-22):

Summary: There is 1 day delay in implementation of the Remove glyph FRP (#5725) task.
It will cause 0 days delay for the delivery of this weekly plan.

Delay Cause: Designing a way to set glyph parameters for debug scene to work without FRP.

enso-bot · 2023-02-24T06:09:54Z

Keziah Wesley reports a new STANDUP for yesterday (2023-02-22):

Progress: Finished removing per-glyph FRP. Planning pre-computed MSDFs. It should be finished by 2023-02-23.

Next Day: Next day I will be working on the #5725 task. Port text debug scene to new API.

enso-bot · 2023-02-24T06:10:12Z

Keziah Wesley reports a new STANDUP for today (2023-02-23):

Progress: Implemented text area debug scene functionality without FRP. It should be finished by 2023-02-23.

Next Day: Next day I will be working on the #5722 task. Implementing pre-computed MSDF atlas.

farmaazon · 2023-03-01T08:56:15Z

lib/rust/ensogl/component/text/src/font.rs

+impl Hinting {
+    fn for_font(font_name: &str) -> Self {
+        let platform = platform::current();
+        match (platform, font_name) {
+            (Some(platform::Platform::MacOS), "mplus1p") =>
+                Self { opacity_increase: 0.4, opacity_exponent: 4.0 },
+            (Some(platform::Platform::Windows), "mplus1p") =>
+                Self { opacity_increase: 0.3, opacity_exponent: 3.0 },
+            (Some(platform::Platform::Linux), "mplus1p") =>
+                Self { opacity_increase: 0.3, opacity_exponent: 3.0 },
+            _ => Self::default(),
+        }
+    }
+}


I think we should add explanation here why do we need it and why it differs.

farmaazon · 2023-03-01T08:59:44Z

lib/rust/ensogl/component/text/src/font/glyph.rs

@@ -526,7 +461,8 @@ impl WeakGlyph {
 // ==============

 #[cfg(not(target_arch = "wasm32"))]
-#[derive(Clone, Copy, CloneRef, Debug, Default)]
+#[derive(Clone, CloneRef, Debug, Default)]
+#[allow(missing_copy_implementations)]


Why cannot we implement Copy?

This is a mock of a non-Copy type; if it's Copy, the linter will complain when we clone it.

Ok. You can add a comment there, so nobody will ask this question again.

lib/rust/profiler/data/src/aggregate.rs

farmaazon · 2023-03-01T09:11:13Z

lib/rust/profiler/data/src/bin/intervals.rs

+        if let Some(prefix) = INCLUDE_ONLY_SUBTREES_MATCHING_PREFIX
+                && label.starts_with(prefix) {
+            enable = true;
+        }


let enabled = enable || INCLUDE_ONLY_SUBTREES_MATCHING_PREFIX.map_or(false, |pfx| label.starts_with(pfx)); if enabled {...

I feel it shows the logic more clearly.

kazcw added 2 commits February 21, 2023 23:43

Profiling.

22ade87

Remove glyph FRP.

14fffa0

kazcw self-assigned this Feb 21, 2023

kazcw linked an issue Feb 22, 2023 that may be closed by this pull request

Eliminate Glyph FRP Network #5724

Closed

kazcw added the CI: No changelog needed Do not require a changelog entry for this PR. label Feb 22, 2023

kazcw added 4 commits February 22, 2023 16:59

Profiling: Filter intervals to missed frames.

4f676c0

Remove glyph FRP.

a9ce388

Profiling.

0014ac6

Profiling: Support filtering intervals by subtree

ded95bf

Clean up.

8a1400d

kazcw marked this pull request as ready for review February 22, 2023 18:12

kazcw requested review from MichaelMauderer, wdanilo, farmaazon and mwu-tow as code owners February 22, 2023 18:12

kazcw added 2 commits February 22, 2023 21:13

lint

1681893

Profiling

0e6469a

kazcw marked this pull request as draft February 22, 2023 22:03

kazcw added 3 commits February 23, 2023 16:54

Refactor.

ec41ccc

refactor

1c7aa3f

Support tweaking hinting in text area demo.

9f6df66

Merge branch 'develop' into wip/kw/remove-glyph-frp

eb547d9

kazcw marked this pull request as ready for review February 23, 2023 21:36

Merge branch 'develop' into wip/kw/remove-glyph-frp

7167e36

enso-org deleted a comment from enso-bot bot Feb 28, 2023

farmaazon approved these changes Mar 1, 2023

View reviewed changes

kazcw added 2 commits March 3, 2023 19:14

Review.

ade290d

Broaden some CODEOWNERS.

067b422

kazcw added the CI: Ready to merge This PR is eligible for automatic merge label Mar 3, 2023

mergify bot added 2 commits March 3, 2023 19:16

Merge branch 'develop' into wip/kw/remove-glyph-frp

cdfd6f3

Merge branch 'develop' into wip/kw/remove-glyph-frp

711babd

kazcw mentioned this pull request Mar 6, 2023

Precompute MSDFs #5811

Merged

4 tasks

wdanilo approved these changes Mar 6, 2023

View reviewed changes

Merge branch 'develop' into wip/kw/remove-glyph-frp

cef88a0

mergify bot merged commit b74debb into develop Mar 6, 2023

mergify bot deleted the wip/kw/remove-glyph-frp branch March 6, 2023 23:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove glyph FRP #5725

Remove glyph FRP #5725

kazcw commented Feb 21, 2023 •

edited

Loading

enso-bot bot commented Feb 22, 2023

kazcw commented Feb 22, 2023

wdanilo commented Feb 22, 2023

kazcw commented Feb 23, 2023

enso-bot bot commented Feb 24, 2023

enso-bot bot commented Feb 24, 2023

enso-bot bot commented Feb 24, 2023

farmaazon Mar 1, 2023

farmaazon Mar 1, 2023 •

edited

Loading

kazcw Mar 3, 2023

farmaazon Mar 6, 2023

farmaazon Mar 1, 2023

Remove glyph FRP #5725

Remove glyph FRP #5725

Conversation

kazcw commented Feb 21, 2023 • edited Loading

Pull Request Description

Important Notes

Checklist

enso-bot bot commented Feb 22, 2023

kazcw commented Feb 22, 2023

wdanilo commented Feb 22, 2023

kazcw commented Feb 23, 2023

enso-bot bot commented Feb 24, 2023

enso-bot bot commented Feb 24, 2023

enso-bot bot commented Feb 24, 2023

farmaazon Mar 1, 2023

Choose a reason for hiding this comment

farmaazon Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

kazcw Mar 3, 2023

Choose a reason for hiding this comment

farmaazon Mar 6, 2023

Choose a reason for hiding this comment

farmaazon Mar 1, 2023

Choose a reason for hiding this comment

kazcw commented Feb 21, 2023 •

edited

Loading

farmaazon Mar 1, 2023 •

edited

Loading