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

fix: the defaultFontFamily not working #208

Merged
merged 3 commits into from Feb 15, 2023
Merged

fix: the defaultFontFamily not working #208

merged 3 commits into from Feb 15, 2023

Conversation

yisibl
Copy link
Owner

@yisibl yisibl commented Feb 12, 2023

Fixed: #207

Test

npm i
npm run build
node example/text.js

@vercel
Copy link

vercel bot commented Feb 12, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
resvg-js ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 14, 2023 at 1:17PM (UTC)

@yisibl
Copy link
Owner Author

yisibl commented Feb 12, 2023

@RazrFalcon Sorry for pinging you here, can you help to see why the default font family can't be loaded after splitting into usvg-text-layout?

Our resvg upgrade code changes are here: #194

@RazrFalcon
Copy link

What are you using it for? Maybe this would help RazrFalcon/resvg#566

@yisibl
Copy link
Owner Author

yisibl commented Feb 13, 2023

@RazrFalcon I found that it is indeed related to RazrFalcon/resvg#566. The main change I think is caused here:

RazrFalcon/resvg@36b0e0b#diff-6d4d24ae259685abf64776b23b1c6e7af8c3a9718591d7db8d092e3b6cbb216fR684-R686

if families.is_empty() { // Removing this judgment will restore the effect to 0.27.0.
    families.push(state.opt.font_family.clone())
}

Expected

Consider the following SVG:

Case 1

<svg xmlns="http://www.w3.org/2000/svg" width="200" height="200" viewBox="0 0 200 200">
  <text fill="blue" font-family="serif" font-size="120">
    <tspan x="40" y="143">水</tspan>
  </text>
</svg>

When the font-family does not exist or is serif, resvg-js passes in the specified family name via the defaultFontFamily API, and I expect to be able to fallback the Font Family to Source Han Serif CN Light. This works fine for resvg <= 0.27.0.

const resvg = new Resvg(svg, {
    background: 'pink',
    font: {
      fontFiles: ['./example/SourceHanSerifCN-Light-subset.ttf'], // Load custom fonts.
      loadSystemFonts: false, // It will be faster to disable loading system fonts.
      defaultFontFamily: 'Source Han Serif CN Light',
    },
    logLevel: 'debug',
  })

I've done some debugging, can you help me see what the problem is?

npm install
npm run build:debug
node example/text.js

Generated PNG:

text2-out

Case 1 debug
[2023-02-13T05:33:34Z DEBUG resvg_js::fonts] Loaded 1 font faces in 0.741ms.
[2023-02-13T05:33:34Z DEBUG resvg_js::fonts] Font './example/SourceHanSerifCN-Light-subset.ttf':0 found in 0.016ms.
[/Users/yisibl/resvg/usvg/src/text.rs:677] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg/src/text.rs:678] &state.opt.font_family = "Source Han Serif CN Light"
[/Users/yisibl/resvg/usvg/src/text.rs:679] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg/src/text.rs:677] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg/src/text.rs:678] &state.opt.font_family = "Source Han Serif CN Light"
[/Users/yisibl/resvg/usvg/src/text.rs:679] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg/src/text.rs:677] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg/src/text.rs:678] &state.opt.font_family = "Source Han Serif CN Light"
[/Users/yisibl/resvg/usvg/src/text.rs:679] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:509] &span.font.families = [
    "serif",
]
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:686] &name_list = [
    Serif,
    Serif,
]
[2023-02-13T05:33:34Z WARN  usvg_text_layout] No match for 'serif' font-family.
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:509] resolve_font(&span.font, fontdb) = None
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:686] &name_list = [
    Serif,
    Serif,
]
[2023-02-13T05:33:34Z WARN  usvg_text_layout] No match for 'serif' font-family.

Case 2

Explicitly set font-family to the family name of the actual font

<svg xmlns="http://www.w3.org/2000/svg" width="200" height="200" viewBox="0 0 200 200">
    <text fill="blue" font-family="Source Han Serif CN Light" font-size="120">
      <tspan x="40" y="143">水</tspan>
    </text>
  </svg>

Generated PNG:
text2-out

Case 2 debug
[2023-02-13T05:44:43Z DEBUG resvg_js::fonts] Loaded 1 font faces in 0.131ms.
[2023-02-13T05:44:43Z DEBUG resvg_js::fonts] Font './example/SourceHanSerifCN-Light-subset.ttf':0 found in 0.009ms.
[/Users/yisibl/resvg/usvg/src/text.rs:677] &families = [
    "Source Han Serif CN Light",
]
[/Users/yisibl/resvg/usvg/src/text.rs:678] &state.opt.font_family = "Source Han Serif CN Light"
[/Users/yisibl/resvg/usvg/src/text.rs:679] &families = [
    "Source Han Serif CN Light",
]
[/Users/yisibl/resvg/usvg/src/text.rs:677] &families = [
    "Source Han Serif CN Light",
]
[/Users/yisibl/resvg/usvg/src/text.rs:678] &state.opt.font_family = "Source Han Serif CN Light"
[/Users/yisibl/resvg/usvg/src/text.rs:679] &families = [
    "Source Han Serif CN Light",
]
[/Users/yisibl/resvg/usvg/src/text.rs:677] &families = [
    "Source Han Serif CN Light",
]
[/Users/yisibl/resvg/usvg/src/text.rs:678] &state.opt.font_family = "Source Han Serif CN Light"
[/Users/yisibl/resvg/usvg/src/text.rs:679] &families = [
    "Source Han Serif CN Light",
]
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:509] &span.font.families = [
    "Source Han Serif CN Light",
]
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:686] &name_list = [
    Name(
        "Source Han Serif CN Light",
    ),
    Serif,
]
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:509] resolve_font(&span.font, fontdb) = Some(
    ResolvedFont {
        id: ID(
            2,
        ),
        units_per_em: 2048,
        ascent: 2357,
        descent: -586,
        x_height: 1053,
        underline_position: -307,
        underline_thickness: 102,
        line_through_position: 631,
        subscript_offset: 154,
        superscript_offset: 717,
    },
)
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:686] &name_list = [
    Name(
        "Source Han Serif CN Light",
    ),
    Serif,
]

Try

Does it need to be changed to this?

if families.is_empty() || !state.opt.font_family.is_empty() {
    families.push(state.opt.font_family.clone())
}
debug output
[2023-02-13T05:33:34Z DEBUG resvg_js::fonts] Loaded 1 font faces in 0.741ms.
[2023-02-13T05:33:34Z DEBUG resvg_js::fonts] Font './example/SourceHanSerifCN-Light-subset.ttf':0 found in 0.016ms.
[/Users/yisibl/resvg/usvg/src/text.rs:677] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg/src/text.rs:678] &state.opt.font_family = "Source Han Serif CN Light"
[/Users/yisibl/resvg/usvg/src/text.rs:679] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg/src/text.rs:677] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg/src/text.rs:678] &state.opt.font_family = "Source Han Serif CN Light"
[/Users/yisibl/resvg/usvg/src/text.rs:679] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg/src/text.rs:677] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg/src/text.rs:678] &state.opt.font_family = "Source Han Serif CN Light"
[/Users/yisibl/resvg/usvg/src/text.rs:679] &families = [
    "serif",
]
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:509] &span.font.families = [
    "serif",
]
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:686] &name_list = [
    Serif,
    Serif,
]
[2023-02-13T05:33:34Z WARN  usvg_text_layout] No match for 'serif' font-family.
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:509] resolve_font(&span.font, fontdb) = None
[/Users/yisibl/resvg/usvg-text-layout/src/lib.rs:686] &name_list = [
    Serif,
    Serif,
]
[2023-02-13T05:33:34Z WARN  usvg_text_layout] No match for 'serif' font-family.

@RazrFalcon
Copy link

You must not use/rely on Options::font_family to begin with. It is used as a worst case fallback. An SVG must define font-family, otherwise an SVG library can do whatever it wants.

In short, as the linked issue noted, the rules are simple: Options::font_family used only when there are no font-family property anywhere in the file.

In case of your file, you do define family as font-family="serif", meaning you have to set fontdb::Database::set_serif_family and not Options::font_family

The previous behavior wasn't technically correct.

@zimond zimond merged commit 1b7d7d2 into main Feb 15, 2023
@yisibl yisibl deleted the fix-load-default-font branch February 15, 2023 06:02
@yisibl
Copy link
Owner Author

yisibl commented Feb 15, 2023

@RazrFalcon Yes, I'm using set_serif_family, I just need to determine if defaultFontFamily exists. See ed4c850#diff-48b59335e054ce050f464a700ccd2a85bf1c4f203216f82ee487f2da42985bf3

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.

Font is no longer rendered since version 2.4
3 participants