Skip to content
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

[WIP] New unified TextLayout #1950

Closed
wants to merge 15 commits into from

Conversation

Gillibald
Copy link
Contributor

@Gillibald Gillibald commented Oct 6, 2018

  • What does the pull request do?
    It introduces an abstraction to the text layout and makes it possible to support the inlines and complex shaped text.
  • What is the current behavior?
    Currently, there is no abstraction for the text layout and only some basic stuff is done.
  • What is the updated/expected behavior with this PR?
    The new text layout supports fallbacks for glyphs. That means if a glyph isn't present on the current typeface the layout tries to find one that has the glyph and uses it. That way you can mix typefaces in a text.
  • How was the solution implemented (if it's not obvious)?
    This is quite complex and I will add some comments to the code and try to explain how things work. In general, the layout splits a text into text lines and each line consists of text runs. Each run only contains glyphs that are present on one typeface and only has one specific set of font properties.

The layout is able to perform line breaks if needed (TextWrapping) and also breaks line on \r etc. Text that is split by TextWrapping is only split at a whitespace. We can add some more advanced word breaking in the future.

This adds TextTrimming support for FormattedText

Fixed issues

#1817
#1897
#1925
#2452

Initial work for: #1809

@Gillibald Gillibald changed the title WIP: Skia text layout Skia text layout Oct 11, 2018
@Gillibald Gillibald changed the title Skia text layout WIP : Skia text layout Oct 12, 2018
src/Skia/Avalonia.Skia/Text/SKTextLayout.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SKTextLayout.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SKTextLayout.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SKTextLayout.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SKTextLayout.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SKTextLayout.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SKTextLayout.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SKTextLine.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SKTextLayout.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SkGlyphRun.cs Outdated Show resolved Hide resolved
@Gillibald
Copy link
Contributor Author

Gillibald commented Nov 4, 2018

DirectWrite
emoji_directwrite

Skia
emoji_skia

@Gillibald
Copy link
Contributor Author

Looks like I have to add the needed features myself. To support all the BiDi stuff I also need a more advanced wrapper of HarfBuzz will figure out how to achieve this. I am not very familiar with interop techniques but maybe now is the time to change that 😀

src/Skia/Avalonia.Skia/Avalonia.Skia.csproj Outdated Show resolved Hide resolved
Avalonia.sln Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SKTextLayout.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SKTextLayout.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/Text/SKTextRun.cs Outdated Show resolved Hide resolved
@kekekeks kekekeks mentioned this pull request Jan 11, 2019
2 tasks
@Gillibald
Copy link
Contributor Author

I have refactored everything to use ReadOnlySpan and only store a SKTextPointer struct in each SKTextRun to reference a section within the text. This is not polished and can probably be simplified. If you find something obvious let me know. I am open for performance improvements. In terms of memory consumption, this should be quite good already.

@Gillibald Gillibald changed the title WIP : Skia text layout Skia text layout Jan 21, 2019
@Gillibald Gillibald mentioned this pull request Mar 22, 2019
3 tasks
@Gillibald Gillibald mentioned this pull request Apr 20, 2019
3 tasks
@grokys
Copy link
Member

grokys commented May 1, 2019

@Gillibald I've tried to review this PR but it's really hard to review because it introduces so many formatting changes unrelated to the functionality.

Would it be possible to undo the arbitrary changes so that it's possible to see what has changed functionally? I'd be happy to submit a PR to this PR doing so if you're busy.

@Gillibald
Copy link
Contributor Author

Gillibald commented May 1, 2019

Would help me a lot if you could do that for me. Sry for all the unrelated changes. The TextBox.cs is a mess. I should really disable all auto formatting.

TextBox itself has changed a lot. Sadly I didn't know how to write unit tests for it. A lot of the functionality is executed by events.

Is there a tool I can work with to revert these formatting changes?

I know this will be hard to review just let me know if something isn't clear to you.

@Gillibald
Copy link
Contributor Author

Text hit testing now supports clusters of glyphs. That way selection and navigation works properly for sequences of codepoints that are treated as one unit. Emojis for example are represented as a surrogate pair and should be removed or navigated to as a pair. More complex sequences like ligatures are also handled properly.

@Gillibald
Copy link
Contributor Author

Gillibald commented May 1, 2019

I have removed FontSize from Typeface to support the reuse of it at different font sizes.

We can fix all the formatting, naming etc in a different PR. It just felt right to fix it when I touched parts of the code base but makes it hard to review. My bad.

If you want to run this you have to download recent artifacts from the SkiaSharp github.

@grokys
Copy link
Member

grokys commented May 1, 2019

The TextBox.cs is a mess. Sadly I didn't know how to write unit tests for it. A lot of the functionality is executed by events.

Yeah, I agree - it really needs an overhaul and proper unit tests...

Is there a tool I can work with to revert these formatting changes?

Unfortunately I don't know of any. I'll do it manually.

We can fix all the formatting, naming etc in a different PR. It just felt right to fix it when I touched parts of the code base but makes it hard to review. My bad.

Yeah, and to an extent it's ok to do that with smaller PRs. However for bigger PRs like this, it's best to avoid formatting changes/refactors if possible.

Anyway, not a problem - I'll submit a PR which targets this PR to remove the formatting changes -- it'll be useful as a way of reviewing the PR even.

@Gillibald
Copy link
Contributor Author

I could revert most of the formatting changes. Should be easier to review now.

@mat1jaczyyy
Copy link
Contributor

Any plans on merging this soon? I would love to utilize the TextTrimming property this introduces.

@Gillibald
Copy link
Contributor Author

Gillibald commented May 28, 2019

It is almost ready but still needs some work. I hope I can find some time this week to fix glyph fallbacks for missing fonts.

This also needs heavy testing and more unit tests.

Corrupted encoding could also be an issue and has to be handled properly.
I will most likely implement it in a way that the layout process stops when the encoding is corrupted and render replacements instead.

@Gillibald Gillibald mentioned this pull request Jun 9, 2019
@Gillibald Gillibald changed the title Skia text layout New unified TextLayout Jul 17, 2019
src/Avalonia.Visuals/Media/FontFamily.cs Outdated Show resolved Hide resolved
samples/ControlCatalog/Pages/GlyphRunPage.xaml.cs Outdated Show resolved Hide resolved
samples/ControlCatalog/Pages/GlyphRunPage.xaml.cs Outdated Show resolved Hide resolved
samples/ControlCatalog/Pages/GlyphRunPage.xaml.cs Outdated Show resolved Hide resolved
src/Avalonia.Visuals/Media/FormattedTextStyleSpan.cs Outdated Show resolved Hide resolved
src/Avalonia.Visuals/Media/GlyphRun.cs Outdated Show resolved Hide resolved
src/Avalonia.Visuals/Media/GlyphRun.cs Outdated Show resolved Hide resolved
src/Avalonia.Visuals/Media/Text/FontMetrics.cs Outdated Show resolved Hide resolved
src/Avalonia.Visuals/Media/Text/GlyphCluster.cs Outdated Show resolved Hide resolved
@Gillibald Gillibald changed the title New unified TextLayout [WIP] New unified TextLayout Sep 10, 2019
src/Avalonia.Controls/Presenters/TextPresenter.cs Outdated Show resolved Hide resolved
src/Skia/Avalonia.Skia/LineBreakIterator.cs Outdated Show resolved Hide resolved
src/Avalonia.Visuals/Media/Text/TextLayout.cs Outdated Show resolved Hide resolved
@kronic
Copy link
Contributor

kronic commented Dec 25, 2019

@Gillibald Any news?

@Gillibald
Copy link
Contributor Author

Currently I am working on a minor TextBox rework to support text editing in advanced scenarios. Static text works just fine.

@aguahombre
Copy link
Contributor

Any idea when this PR will get merges back into the master?
I would like to port some WPF code that uses Inlines for hyperlinks within a TextBlock

@Gillibald Gillibald closed this Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants