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.

Sign up for GitHub

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

Jump to bottom

Basic rich text #1255

Merged
merged 2 commits into from
Sep 29, 2020
Merged

Basic rich text #1255

merged 2 commits into from
Sep 29, 2020

Conversation

cmyr
Copy link
Member

@cmyr cmyr commented Sep 25, 2020

This is a pair of commits that allow for the creation of a rich text object, that can be displayed in a RawLabel or anything else that uses a TextLayout object.

I don't love this code, but it at least covers the simple case, and I think it is worth checkpointing.

Details: for each annotation type, we maintain a sorted vec of non-overlapping spans. The annotation types in druid are slightly different from what is available in piet; in particular in druid we support FontDescriptor as well as (where possible) KeyOrValue. Druid annotations can be added in any order (not restricted to non-descending start order) and then when we rebuild the layout we resolve them to piet spans and construct the actual layout object.

stuff I don't like:

  • layout may not be invalidated correctly if the env changes (this is fixable, just not in here yet)
  • no functionality for editing the text; you have to recreate the whole object. I'm still not sure how this should interact with editing.
  • default styles are confused; they currently live on the TextLayout, not on the RichText; this should probably change in some way, so that if you're using RichText we ignore anything set on the label itself.

You can play with this at examples/text.rs:

    let text = RichText::new(TEXT.into())
        .with_attribute(0..9, Attribute::text_color(Color::rgb(1.0, 0.2, 0.1)))
        .with_attribute(0..9, Attribute::size(24.0))
        .with_attribute(0..9, Attribute::font_family(FontFamily::SERIF))
        .with_attribute(194..239, Attribute::weight(FontWeight::BOLD))
        .with_attribute(764.., Attribute::size(12.0))
        .with_attribute(764.., Attribute::style(FontStyle::Italic));

Screen Shot 2020-09-25 at 11 23 24 AM

Base automatically changed from raw-label to master September 25, 2020 19:13
@cmyr cmyr added the S-needs-review waits for review label Sep 27, 2020
cmyr
cmyr commented Sep 28, 2020
View reviewed changes
druid/src/text/layout.rs Outdated
/// Create a new `TextLayout` with the provided text.
///
/// This is useful when the text is not died to application data.
pub fn with_text(text: impl Into<T>) -> Self {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe rename this? with_ seems to suggest a builder method.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think from_ would be appropriate here? (It consumes the text, right?)

raphlinus
raphlinus approved these changes Sep 29, 2020
View reviewed changes
Copy link
Contributor

@raphlinus raphlinus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I can see a few things that might change long term, but nothing that I would consider blocking now.

@cmyr
Add text/attributes.rs and basic text attribute impl
a39187c 
This isn't actually used anywhere in this commit; it's just an
implementation of the types for representing text attributes
and storing them in spans.
@cmyr
Basic rich text support
2b446b3 
This adds a TextStorage trait for types that... store text.
On top of this, it implements a RichText type, that is
a string and a set of style spans.

This type is currently immutable, in the sense that it
cannot be edited. Editing is something that we would
definitely like, at some point, but it expands the scope
of this work significantly, and at the very least should
be a separate patch.
@cmyr
Copy link
Member Author

cmyr commented Sep 29, 2020

I think there is still some stuff wrong with this PR but I'll open issues for those things, and I think getting it in will unblock some other stuff.

@cmyr cmyr merged commit 49466e1 into master Sep 29, 2020
@cmyr cmyr deleted the text-storage branch September 29, 2020 17:32
@cmyr cmyr mentioned this pull request Sep 29, 2020
Open
3 tasks
@cmyr cmyr mentioned this pull request Oct 9, 2020
Open
7 tasks
@maan2003 maan2003 removed the S-needs-review waits for review label May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zarenor Zarenor Zarenor left review comments

@raphlinus raphlinus raphlinus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cmyr @raphlinus @Zarenor @maan2003