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

Implement checking if the font familly exists in pango #455

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Implement checking if the font familly exists in pango #455

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
7218cf2
Implement checking if the font familly exists in pango
JAicewizard Jul 12, 2021
43796b3
Update piet-cairo/src/text.rs
JAicewizard Jul 15, 2021
eb7b455
Font matching always matches the best font available
JAicewizard Jul 27, 2021
b3d9b85
Fix filtering badly
JAicewizard Jul 27, 2021
f0e6f1c
Debug things
JAicewizard Jul 27, 2021
6291c38
debug
JAicewizard Jul 27, 2021
154eaf6
Checkout fontmap
JAicewizard Jul 27, 2021
1ff7276
Use the default font as initial value, if exists
JAicewizard Jul 27, 2021
1101f30
Back to 1.44
JAicewizard Jul 27, 2021
f413247
Use system-ui as default font
JAicewizard Jul 28, 2021
d21f23a
Print attributes
JAicewizard Jul 28, 2021
7ca9372
Default to system-ui like before
JAicewizard Jul 28, 2021
88ccad2
Cleanup
JAicewizard Jul 28, 2021
bb0a741
Debug test
JAicewizard Jul 28, 2021
0372651
More debut
JAicewizard Jul 28, 2021
720c44c
This is what cairo should use
JAicewizard Jul 28, 2021
4f1fdbc
??
JAicewizard Jul 28, 2021
7eb17e8
Debugging again
JAicewizard Jul 28, 2021
bcedb2c
Perfectly match how pango loads fonts
JAicewizard Aug 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion piet-cairo/src/text.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::rc::Rc;

use glib::translate::{from_glib_full, ToGlibPtr};

use pango::prelude::FontFamilyExt;
use pango::prelude::FontMapExt;
use pango::AttrList;
use pango_sys::pango_attr_insert_hyphens_new;
Expand Down Expand Up @@ -166,7 +167,12 @@ impl Text for CairoText {
type TextLayoutBuilder = CairoTextLayoutBuilder;

fn font_family(&mut self, family_name: &str) -> Option<FontFamily> {
//TODO: Veryify that a family exists with the requested name
// The pango documentation says this is always a stirng, and never null. I trust them on that.
// Would be weird to have a font family without a name anyways.
JAicewizard marked this conversation as resolved.
Show resolved Hide resolved
self.pango_context
.list_families()
.iter()
.find(|family| family.name().unwrap().as_str() == family_name);
Copy link
Member

Choose a reason for hiding this comment

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

As a note: a fancier version of this might do things like return Helvetica Neue for Helvetica if Helvetica isn't present, and it might also want to be case-insensitive?

Additionally, it looks like we aren't using this check? If we don't find the font we should return None.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additionally, it looks like we aren't using this check?

What check do you mean? I can add the Helvetica -> Helvetica Neue check if thats what you mean.

Copy link
Member

Choose a reason for hiding this comment

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

What I mean is that we're looking at the families to see if family_name exists, but we're still always returning Some from this function.

We may also want to keep around a HashSet of family names so that we aren't doing a linear search each time, but that can be future work.

Copy link
Member

Choose a reason for hiding this comment

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

Just for fun here are some useful guidelines on font matching: https://drafts.csswg.org/css-fonts-3/#font-matching-algorithm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I mean is that we're looking at the families to see if family_name exists, but we're still always returning Some from this function.

I feel really stupid now. I just noticed this as well while looking at the code again.

As for the matching, I am honestly not sure how I would go about implementing that. Stripping away all the stuff we dont have/need, im left with the only restriction being "a case insensitive match". it should be easy to match case insensitively, but that doesnt solve the Helvetica -> Helvetica Neue match you wanted. I could check for substrings, I think thats easiest. It wont match the other way around, but we could check vise-versa as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There exists a function in pango for matching, I just use that.

Copy link
Member

Choose a reason for hiding this comment

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

Is this a change you're going to make, or is it in the current version? I didn't see it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I currently made a change so that it uses the pango matching function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(the better_match function)

Some(FontFamily::new_unchecked(family_name))
}

Expand Down
2 changes: 1 addition & 1 deletion piet/src/font.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl FontFamily {
FontFamilyInner::SansSerif => "sans-serif",
FontFamilyInner::SystemUi => "system-ui",
FontFamilyInner::Monospace => "monospace",
FontFamilyInner::Named(s) => &s,
FontFamilyInner::Named(s) => s,
}
}

Expand Down