New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOM-based measurements #8811

Merged
merged 90 commits into from Oct 7, 2015

Conversation

Projects
None yet
@as-cii
Member

as-cii commented Sep 16, 2015

Supersedes and closes #6083.
Supersedes and closes #7945.

A Brief Flashback

Over the past month we have evaluated several strategies to attack the problem, among which:

iframe

Build the lines inside an <iframe> element and measure from them.

Pros: measurements happen in a separate context, completely decoupled from the current DOM. Some optimization can, therefore, be applied: skinnier DOM, less stylesheet, and so on.
Cons: building the lines twice has a cost which is higher than the optimizations we tried out. Moreover, we cannot alter the original DOM structure as, otherwise, measurements could be wrong.

Canvas

Have a canvas element and use the measureText function to provide all the metrics we need to position visual elements.

Pros: measurements happen in a separate context and the overhead is minimal.
Cons: CSS typography is not fully supported (e.g. ligatures/contextual alternates get screwed up) and the risk of reporting corrupted measurements is very high.

No Measurements

Instead of positioning decorations, cursors, etc. with absolute coordinates, embed them inside each line and let the browser figure out all the metrics.

Pros: no need for measurements and more semantical HTML. In addition, this enriches the power of styling customizations (e.g. the user has the possibility to style a piece of text surrounded by a decoration).
Cons: worse performance, because the DOM gets more complicated and nesting increases. Overlays need to be displayed on top of lines node, which forces us to employ tricks to let them appear in front of everything.

The Present

In this PR we have implemented DOM-based measurements. The idea here is to append the real lines to the DOM, measure from them, and render everything else that needs measurements.

Every time TextEditorElement::pixelPositionForScreenPosition or TextEditorElement::screenPositionForPixelPosition is called, we trigger a reflow (a bit like the DOM does when you try to read from it when its information is stale).

In TextEditorPresenter we update the DOM with all lines we'll need to measure before computing any state, in order to avoid multiple reflows caused by reading from line nodes more than once.

What's missing:

  • Autoscroll when reaching the right end of the editor.
  • Implement screenPositionForPixelPosition in LinesYardstick.
  • Fix remaining specs.
  • Avoid using NodeIterator as it is slow. Cache text nodes and measure those instead.
  • Evaluate if caching already computed positions yields some improvements.
  • Remove measurement in TextEditorComponent::readAfterUpdateSync.
  • Ensure we have no significant regression performance-wise.
  • Investigate why Hasklig behaves oddly.
  • Fix bug where the cursor disappears when moving it left/right.
  • ⬆️ autocomplete-plus.
  • ⬆️ wrap-guide.

Feedback is super welcome and appreciated! Thanks! 🙇

/cc: @nathansobo @atom/feedback for 👀

@benogle

This comment has been minimized.

Show comment
Hide comment
@benogle

benogle Sep 16, 2015

Contributor

Excited for this!

Contributor

benogle commented Sep 16, 2015

Excited for this!

@wismill

This comment has been minimized.

Show comment
Hide comment
@wismill

wismill Oct 6, 2015

Sounds very good! I am testing it with the amazing font PragmataPro.

I highly recommend following the instruction from Laurian that deactivate the ligature on the current line, in order to see exactly on which character the cursor is positioned while moving over a ligature (some ligatures involve 3 characters).

A package could be very useful to manage text-rendering: optimizeLegibility; and the option to deactivate it on the current line: simple options on/off.

wismill commented Oct 6, 2015

Sounds very good! I am testing it with the amazing font PragmataPro.

I highly recommend following the instruction from Laurian that deactivate the ligature on the current line, in order to see exactly on which character the cursor is positioned while moving over a ligature (some ligatures involve 3 characters).

A package could be very useful to manage text-rendering: optimizeLegibility; and the option to deactivate it on the current line: simple options on/off.

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Oct 7, 2015

Contributor

Bravo @as-cii. An incredible amount of work went into this PR, much of it not even visible in the commit history. This is the first big step toward good support for internationalization!

Contributor

nathansobo commented Oct 7, 2015

Bravo @as-cii. An incredible amount of work went into this PR, much of it not even visible in the commit history. This is the first big step toward good support for internationalization!

nathansobo added a commit that referenced this pull request Oct 7, 2015

@nathansobo nathansobo merged commit 3636eb3 into master Oct 7, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nathansobo nathansobo deleted the as-double-reflow-measurements branch Oct 7, 2015

@benogle

This comment has been minimized.

Show comment
Hide comment
@benogle

benogle Oct 7, 2015

Contributor

💥

Contributor

benogle commented Oct 7, 2015

💥

@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Oct 7, 2015

Contributor

🎆 This is huge!

Contributor

maxbrunsfeld commented Oct 7, 2015

🎆 This is huge!

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Oct 7, 2015

Member

Yesss! 💯

Member

50Wliu commented Oct 7, 2015

Yesss! 💯

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Oct 8, 2015

Member

So awesome 👍 👍 👍

Member

kevinsawicki commented Oct 8, 2015

So awesome 👍 👍 👍

@rajadain

This comment has been minimized.

Show comment
Hide comment
@rajadain

rajadain Oct 9, 2015

Ligature support! Incredible! I've been waiting for this for more than a year!

rajadain commented Oct 9, 2015

Ligature support! Incredible! I've been waiting for this for more than a year!

@dikiaap

This comment has been minimized.

Show comment
Hide comment
@dikiaap

dikiaap commented Oct 9, 2015

🤘

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment