Repository navigation
feat(labels): draw labels in an installed font family, by name #824
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
Changes from all commits
ed18c78
d9caff3
79cf902
44b2d77
9e51082
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1272,6 +1272,37 @@ pub struct LabelClass { | |
| /// Slanted (an italic face where the font has one). | ||
| #[serde(default, skip_serializing_if = "std::ops::Not::not")] | ||
| pub italic: bool, | ||
| /// The installed font family the labels are drawn in, by name ("Liberation Sans Narrow"), as | ||
| /// QGIS names a text format's; none is the label font. A machine without the family draws | ||
| /// them in the label font too. | ||
| #[serde(default, deserialize_with = "de_font", skip_serializing_if = "Option::is_none")] | ||
| pub font: Option<String>, | ||
| } | ||
|
|
||
| /// The longest font family name kept (characters). | ||
| pub const MAX_FONT_NAME: usize = 128; | ||
|
|
||
| /// A label font's family name as it is kept: trimmed, at most [`MAX_FONT_NAME`] characters, and | ||
| /// none when blank. | ||
| pub fn font_name(name: &str) -> Option<String> { | ||
| let name: String = name.trim().chars().filter(|c| !c.is_control()).take(MAX_FONT_NAME).collect(); | ||
| let name = name.trim(); | ||
| (!name.is_empty()).then(|| name.to_string()) | ||
| } | ||
|
|
||
| /// A label class's font as a project has it, kept as [`font_name`] keeps it: a name, or none for | ||
| /// anything else (a number, a list), so a damaged font doesn't make the project unreadable. | ||
| fn de_font<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Option<String>, D::Error> { | ||
| #[derive(Deserialize)] | ||
| #[serde(untagged)] | ||
| enum Font { | ||
| Name(String), | ||
| Other(serde::de::IgnoredAny), | ||
| } | ||
| Ok(match Option::<Font>::deserialize(d)? { | ||
| Some(Font::Name(n)) => font_name(&n), | ||
| Some(Font::Other(_)) | None => None, | ||
| }) | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid, fixed in 44b2d77. |
||
| fn default_class_name() -> String { | ||
|
|
@@ -1415,6 +1446,7 @@ impl LabelClass { | |
| farthest: None, | ||
| placement: LabelPlacement::Auto, | ||
| italic: false, | ||
| font: None, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1540,6 +1572,34 @@ mod tests { | |
| assert_eq!(serde_json::from_value::<Labeling>(v).unwrap(), with); | ||
| } | ||
|
|
||
| /// A label font is kept by its family name, trimmed and bounded (a project file is hostile | ||
| /// input); a blank one is the label font, and isn't written. | ||
| #[test] | ||
| fn a_label_font_is_kept_by_name() { | ||
| let read = |font: serde_json::Value| { | ||
| serde_json::from_value::<Labeling>(serde_json::json!({ | ||
| "enabled": true, "field": "name", "size": 9.0, "color": [0, 0, 0, 255], "halo": false, "bold": false, "font": font | ||
| })) | ||
| .unwrap() | ||
| }; | ||
| let l = read(" Liberation Sans Narrow \n".into()); | ||
| assert_eq!(l.default.font.as_deref(), Some("Liberation Sans Narrow")); | ||
| assert_eq!(serde_json::to_value(&l).unwrap()["font"], "Liberation Sans Narrow"); | ||
| // Not a name (a number, a list, an object): no font, and the rest of the labels still read. | ||
| for blank in [ | ||
| serde_json::json!(" "), | ||
| serde_json::Value::Null, | ||
| serde_json::json!(7), | ||
| serde_json::json!(["Arial"]), | ||
| serde_json::json!({"a": 1}), | ||
| ] { | ||
| let l = read(blank); | ||
| assert_eq!(l.default.font, None); | ||
| assert!(serde_json::to_value(&l).unwrap().get("font").is_none()); | ||
| } | ||
| assert_eq!(read("x".repeat(10_000).into()).default.font.map(|f| f.chars().count()), Some(MAX_FONT_NAME)); | ||
| } | ||
|
|
||
| /// A project file is read with at most `MAX_LABEL_CLASSES` classes, and only that many count | ||
| /// in a `Labeling` built with more. A class on | ||
| /// a field the data doesn't have is pointed at Default's, and kept. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
labels.fontscallsfamilies::installed(), which blocks on theOnceLockif the backgroundfont-indexthread is still walking. Commands run on the UI thread in the app, so calling this from the command palette or an assistant tool right after startup freezes the UI until the index finishes. The panes avoid this withinstalled_now(). Consider returning an empty list (or an "indexing" flag) when the index isn't ready and warming has started. Confidence: medium.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid, fixed in 9e51082. I had kept this blocking in the earlier replies, but commands do run on the UI thread in the app.
labels.fontsnow usesfamilies::available(), whichfind_readyuses too. While the app'swarm_upindex is still running, it answers{"fonts": [], "indexing": true}at once, and asking again later lists them. Where nothing warmed up (scripts, the command line, the engine tests), the families are indexed on the spot, as before. The params string documentsindexing.