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

web: Allow defining custom names for TTF fontSources #19321

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

danielhjacobs
Copy link
Contributor

@danielhjacobs danielhjacobs commented Jan 25, 2025

Per #19269 (comment), draft PR to try to fix that issue. Note, it is mostly human-reviewed AI generated content. For full transparency, the prompt was this:

I have this current code in web/src/builder.rs:

#[wasm_bindgen(inspectable)]
#[derive(Debug, Clone)]
pub struct RuffleInstanceBuilder {

...
pub(crate) custom_fonts: Vec<(String, Vec<u8>)>,
}
impl Default for RuffleInstanceBuilder {
fn default() -> Self {

...

...

ruffle/web/src/builder.rs

Lines 99 to 102 in 41e373c

custom_fonts: vec![],
}
}
}

...

ruffle/web/src/builder.rs

Lines 339 to 360 in 41e373c

impl RuffleInstanceBuilder {
pub fn setup_fonts(&self, player: &mut Player) {
for (font_name, bytes) in &self.custom_fonts {
let bytes_slice = &bytes[..];
if let Ok(face) = ttf_parser::Face::parse(bytes_slice, 0) {
tracing::debug!("Loading font {font_name} as TTF/OTF/TTC/OTC font");
// Check if font collection
let number_of_fonts = ttf_parser::fonts_in_collection(bytes_slice).unwrap_or(1u32);
Self::register_ttf_face_by_name(font_name, bytes.clone(), face, 0, player);
// Register all remaining fonts in the collection if it is a collection
for i in 1u32..number_of_fonts {
if let Ok(face) = ttf_parser::Face::parse(bytes_slice, i) {
Self::register_ttf_face_by_name(font_name, bytes.clone(), face, i, player);
} else {
tracing::warn!(
"Failed to parse font {font_name} at index {i} in font collection"
);
}
}

...

ruffle/web/src/builder.rs

Lines 406 to 407 in 41e373c

}
}

...

ruffle/web/src/builder.rs

Lines 412 to 442 in 41e373c

}
#[inline]
fn register_ttf_face_by_name(
url: &String,
bytes: Vec<u8>,
face: ttf_parser::Face<'_>,
index: u32,
player: &mut Player,
) {
let full_name = face
.names()
.into_iter()
.find(|name| name.name_id == ttf_parser::name_id::FULL_NAME)
.and_then(|name| name.to_string());
let name = if let Some(full_name) = full_name {
full_name
} else {
tracing::warn!("Font {url} at index {index} has no full name, using URL as font name");
url.to_string()
};
player.register_device_font(FontDefinition::FontFile {
name: name.to_string(),
is_bold: face.is_bold(),
is_italic: face.is_italic(),
data: bytes,
index,
});
}

...

...

ruffle/web/src/builder.rs

Lines 295 to 298 in 41e373c

#[wasm_bindgen(js_name = "addFont")]
pub fn add_font(&mut self, font_name: String, data: Vec<u8>) {
self.custom_fonts.push((font_name, data))
}

In a TS file, I have this code that uses that addFont wasm_bindgen function:
if (this.loadedConfig?.fontSources) {
for (const url of this.loadedConfig.fontSources) {
try {
const response = await fetch(url);
builder.addFont(
url,
new Uint8Array(await response.arrayBuffer()),
);
} catch (error) {
console.warn(
`Couldn't download font source from ${url}`,
error,
);
}
}
}

fontSources is defined in a different file as this:
fontSources?: Array<string>;

An example of fontSources is this:

fontSources: ["NotoSans-Bold.ttf", "NotoSans-Italic.ttf"]

Adjust the code's logic to allow either this structure with this current logic or this structure:

fontSources: [
   "fonts/Noto-Sans-Bold.ttf": {
       "name": "Noto Sans",
       "bold": true,
       "italics": false
    }
]

With that structure the logic should use the name as the name in register_device_font, the bold boolean in the is_bold of register_device_font, and the italics boolean as the is_italic in register_device_font
It should also allow not providing any of the three of those (name, bold, or italics), and should fall back to the current values from the the ttf file if they are not provided.

@danielhjacobs danielhjacobs changed the title web: Allow defining custom names for fontSources web: Allow defining custom names for TTF fontSources Jan 25, 2025
@danielhjacobs danielhjacobs force-pushed the fontSources-config-refactor branch from 821b0e8 to bf89e5c Compare January 26, 2025 02:17
@danielhjacobs danielhjacobs force-pushed the fontSources-config-refactor branch from bf89e5c to 6467690 Compare January 26, 2025 02:27
@danielhjacobs
Copy link
Contributor Author

danielhjacobs commented Jan 26, 2025

Allowed fontSources config options:

    "fontSources": [
        "fonts/Times New Roman.ttf",
        {
            "fonts/centurygothic.ttf": {
                name: "Century Gothic",
            },
            "fonts/arial.ttf": {},
        }
    ]
    "fontSources": [
        {
            "fonts/centurygothic.ttf": {
                name: "Century Gothic",
            }
        },
        {
            "fonts/arial.ttf": {},
        }
    ]
    "fontSources": ["fonts/centurygothic.ttf", "fonts/arial.ttf"]
    "fontSources": {
        "fonts/centurygothic.ttf": {
            name: "Century Gothic",
        },
        "fonts/arial.ttf": {},
    }

@danielhjacobs danielhjacobs force-pushed the fontSources-config-refactor branch from c4240ca to 730cc15 Compare January 26, 2025 02:59
@evilpie
Copy link
Collaborator

evilpie commented Jan 26, 2025

This really seems like too many different ways of defining fonts. We should at most have two, keeping the old one for compatibility of course.

@danielhjacobs
Copy link
Contributor Author

danielhjacobs commented Jan 27, 2025

Summarizing from Discord discussions:

  1. Re: web: Allow defining custom names for TTF fontSources #19321 (comment), the 1st and 2nd options are technically identical, that said I think I'd prefer to keep the 3rd and 4th instead of the 1st-3rd. That will mean when using some fonts that need their name/bold/italics values overridden and others that don't, you'll need an empty object value for the ones that don't.
  2. TTC files and SWF files can define multiple fonts per file. In that case, the current config idea can't really work. I can avoid using the name/bold/italics values from the config if the number of fonts in the TTF collection is greater than 1 or if using an SWF font source, but that doesn't solve the issue, it just skips dealing with it when it doesn't make sense.
  3. Instead, we can leave the current fontSources config option as it is now and add a new config option, fontMapping, a Map that has a key of the name, bold, and italics values requested by the SWF TextField and a value of the name, bold, and italics value that was registered by the fontSources (that name being the URL if the font source had no full name). The layout code would then need adjustments to check the config's fontMapping, and if the font name, bold value, and italics value the SWF is requesting in that part of the layout code is a key in the fontMapping config, it should load the device font matching the name, bold, and italics value from that entry of the fontMapping config instead. If not, it should load the device font it does now (the key values that we looked for).

@danielhjacobs
Copy link
Contributor Author

danielhjacobs commented Jan 27, 2025

  1. It would probably be better if the font fell back to the URL slug sans file extension if the TTF file has no full name, instead of the as-is URL as it does now.

@danielhjacobs danielhjacobs added A-web Area: Web & Extensions T-feature Type: New Feature (that Flash doesn't have) labels Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-web Area: Web & Extensions T-feature Type: New Feature (that Flash doesn't have)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants