Skip to content

script: Do not load all fonts for SVG font database - #46476

Merged
jdm merged 4 commits into
servo:mainfrom
Narfinger:svg-font-resolver-2
Aug 7, 2026
Merged

jdm merged 4 commits into
servo:mainfrom
Narfinger:svg-font-resolver-2

Conversation

@Narfinger

@Narfinger Narfinger commented Jul 13, 2026 •

Copy link
Copy Markdown
Contributor

Currently the SVG font database loads all system fonts at the start and also inserts any fonts it loads.
This removes both of these.
Additionally, we add a small cache that handles the returned fontdb::IDs if they are already entered.
This will also improve the startup time of constellation (and hence everything depending on it) as we do not need to wait for all fonts to be loaded.

Testing: WPT test run https://github.com/Narfinger/servo/actions/runs/30888001779

@Narfinger
Narfinger marked this pull request as ready for review July 14, 2026 07:21
@Narfinger
Narfinger requested a review from gterzian as a code owner July 14, 2026 07:21
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 14, 2026
Comment thread components/script/svg_font.rs Outdated
Comment thread components/script/svg_font.rs Outdated
@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 28, 2026
@Narfinger
Narfinger marked this pull request as draft July 31, 2026 08:55
@Narfinger
Narfinger force-pushed the svg-font-resolver-2 branch from 60380f8 to c1fa8f0 Compare July 31, 2026 08:55
@mrobinson mrobinson changed the title script: Do not load all fonts for Svg font database script: Do not load all fonts for SVG font database Jul 31, 2026
@mrobinson

Copy link
Copy Markdown
Member

@Narfinger Is this ready for review?

@Narfinger

Narfinger commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor Author

No there are some problems with the current design.
First, it doesn't render correctly which I think is related to the second problem.
Second, It copiies the fontdb::Database which it should usvg gives us the guarantee that fontdb::Database is shared exclusively but we are violating this and that leads to either the above problem or needlesly copying of the fontdb.

@Narfinger

Copy link
Copy Markdown
Contributor Author

So update after I worked a bit on it. I think I am doing something wrong with the way I matching the font families in svg_font.

This website ( https://upload.wikimedia.org/wikipedia/commons/e/ea/SVG_Text_Font_Test.svg) currently looks like this:
image

@Narfinger
Narfinger force-pushed the svg-font-resolver-2 branch from 6611382 to 38caf0f Compare August 3, 2026 11:23
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
@Narfinger
Narfinger force-pushed the svg-font-resolver-2 branch from 38caf0f to 6f1e940 Compare August 3, 2026 11:32
@Narfinger

Copy link
Copy Markdown
Contributor Author

Ok this works now. To get feedback I am going to mark it ready for review. I am wondering if there is a better way to not put the svg_resolver into script crate but I couldn't figure it out at the moment.

@Narfinger
Narfinger marked this pull request as ready for review August 3, 2026 11:33
@Narfinger
Narfinger requested a review from nicoburns as a code owner August 3, 2026 11:33
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Aug 3, 2026
@yezhizhen
yezhizhen dismissed their stale review August 4, 2026 08:43

Not my expertise; don't have time

Comment thread components/script/svg_font.rs Outdated
Comment thread components/script/svg_font.rs Outdated
Comment thread components/shared/net/image_cache.rs Outdated
Comment thread components/script/svg_font.rs Outdated
Comment thread components/script/svg_font.rs Outdated
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Aug 4, 2026
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Aug 5, 2026
Narfinger and others added 2 commits August 5, 2026 10:20
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Aug 6, 2026
@jdm
jdm added this pull request to the merge queue Aug 6, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 6, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Aug 6, 2026
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 6, 2026
@jdm
jdm added this pull request to the merge queue Aug 7, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 7, 2026
Merged via the queue into servo:main with commit b3a63fd Aug 7, 2026
35 of 36 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 7, 2026
@Narfinger
Narfinger deleted the svg-font-resolver-2 branch August 7, 2026 07:16
RichardTjokroutomo pushed a commit to RichardTjokroutomo/servo that referenced this pull request Oct 6, 2026
Currently the SVG font database loads all system fonts at the start and
also inserts any fonts it loads.
This removes both of these.
Additionally, we add a small cache that handles the returned fontdb::IDs
if they are already entered.
This will also improve the startup time of constellation (and hence
everything depending on it) as we do not need to wait for all fonts to
be loaded.

Testing: WPT test run
https://github.com/Narfinger/servo/actions/runs/30888001779

---------

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
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.

5 participants