feat: support project font collection#3348
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for project-specific font preferences and custom font families, specifically adding basic Chinese font support and integrating it with SVG rendering. Feedback on the changes focuses on performance optimizations and code simplification: it is recommended to avoid redundant SVG render cache invalidation by checking for actual changes in font preferences, cache the network fetch promise for the basic Chinese font to prevent repeated network requests, simplify the font initialization logic in the project creation modal, and refactor manual DOM traversal in SVG font injection using the closest() API.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export function setSvgFontPreferences(files: Files, fontPreferences: string, fontCollection: FontFamily[]) { | ||
| Object.values(files).forEach((file) => { | ||
| if (file != null && isSvgMimeType(file.type)) { | ||
| derivedSvgCache.delete(file) | ||
| svgFontConfig.set(file, { fontPreferences, fontCollection }) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
Currently, setSvgFontPreferences unconditionally deletes files from derivedSvgCache and updates svgFontConfig every time it is called. Since exportFiles() is triggered frequently during project edits, this constantly invalidates the SVG render cache, leading to redundant SVG parsing, font injection, and blob URL creation. This causes significant performance degradation and memory churn. Checking if the font preferences or collection have actually changed before invalidating the cache will resolve this.
export function setSvgFontPreferences(files: Files, fontPreferences: string, fontCollection: FontFamily[]) {
Object.values(files).forEach((file) => {
if (file != null && isSvgMimeType(file.type)) {
const oldConfig = svgFontConfig.get(file)
const hasChanged = !oldConfig ||
oldConfig.fontPreferences !== fontPreferences ||
oldConfig.fontCollection.length !== fontCollection.length ||
oldConfig.fontCollection.some((font, index) => font !== fontCollection[index])
if (hasChanged) {
derivedSvgCache.delete(file)
svgFontConfig.set(file, { fontPreferences, fontCollection: [...fontCollection] })
}
}
})
}There was a problem hiding this comment.
Fixed in fc4e1ff. The SVG cache is now invalidated only when preferences or the effective font-family snapshot changes.
| export async function createBasicChineseFontFamily() { | ||
| const response = await fetch(basicChineseFontUrl) | ||
| if (!response.ok) throw new Error(`failed to load basic Chinese font: ${response.status}`) | ||
| return new FontFamily('basic-chinese', fromBlob('NotoSansCJKsc-Regular.otf', await response.blob())) | ||
| } |
There was a problem hiding this comment.
The createBasicChineseFontFamily function fetches the basic Chinese font from the network every time it is called. Since font files can be quite large, performing redundant fetches on every legacy project migration or new project creation can cause significant network overhead and delay project loading. Caching the Promise<Blob> of the fetched font ensures subsequent calls resolve instantly without making additional network requests.
| export async function createBasicChineseFontFamily() { | |
| const response = await fetch(basicChineseFontUrl) | |
| if (!response.ok) throw new Error(`failed to load basic Chinese font: ${response.status}`) | |
| return new FontFamily('basic-chinese', fromBlob('NotoSansCJKsc-Regular.otf', await response.blob())) | |
| } | |
| let basicChineseFontBlobPromise: Promise<Blob> | null = null | |
| export async function createBasicChineseFontFamily() { | |
| if (basicChineseFontBlobPromise == null) { | |
| basicChineseFontBlobPromise = fetch(basicChineseFontUrl).then((response) => { | |
| if (!response.ok) throw new Error(`failed to load basic Chinese font: ${response.status}`) | |
| return response.blob() | |
| }).catch((err) => { | |
| basicChineseFontBlobPromise = null | |
| throw err | |
| }) | |
| } | |
| const blob = await basicChineseFontBlobPromise | |
| return new FontFamily('basic-chinese', fromBlob('NotoSansCJKsc-Regular.otf', blob)) | |
| } |
There was a problem hiding this comment.
Fixed in fc4e1ff. The bundled basic Chinese font load now shares a cached Blob promise and resets it on failure for retry.
| if ( | ||
| defaultFontPreferences | ||
| .split(',') | ||
| .map((item) => item.trim()) | ||
| .includes('basic-chinese') | ||
| ) { | ||
| project.fontCollection.splice(0, project.fontCollection.length, await createBasicChineseFontFamily()) | ||
| } else { | ||
| project.fontCollection.splice(0) | ||
| } |
There was a problem hiding this comment.
Since project is a newly created instance, its fontCollection is guaranteed to be empty. We can simplify the logic by directly pushing the font family to the collection if the preferences include 'basic-chinese', avoiding the verbose and unnecessary splice operations.
const preferences = defaultFontPreferences.split(',').map((item) => item.trim())
if (preferences.includes('basic-chinese')) {
project.fontCollection.push(await createBasicChineseFontFamily())
}
There was a problem hiding this comment.
Fixed in fc4e1ff. The default project template has an explicit default preference, so a newly created project starts with an empty collection; the configured preset is now appended directly.
| function applyGlobalFontPreferences(svg: SVGSVGElement, fontPreferences: string) { | ||
| let applied = false | ||
| for (const text of Array.from(svg.querySelectorAll('text'))) { | ||
| let ancestor: Element | null = text | ||
| while (ancestor != null) { | ||
| if (ancestor.hasAttribute('font-family')) break | ||
| ancestor = ancestor.parentElement | ||
| } | ||
| if (ancestor == null && fontPreferences !== '') { | ||
| text.setAttribute('font-family', fontPreferences) | ||
| applied = true | ||
| } | ||
| } | ||
| return applied | ||
| } |
There was a problem hiding this comment.
The manual while loop traversing up the DOM tree to find an ancestor with a font-family attribute can be simplified and optimized using the standard Element.prototype.closest() API.
function applyGlobalFontPreferences(svg: SVGSVGElement, fontPreferences: string) {
let applied = false
if (fontPreferences === '') return false
for (const text of Array.from(svg.querySelectorAll('text'))) {
if (!text.closest('[font-family]')) {
text.setAttribute('font-family', fontPreferences)
applied = true
}
}
return applied
}There was a problem hiding this comment.
Fixed in fc4e1ff. The equivalent closest() check now preserves explicit font-family values on the text node or any ancestor.
1 participant
Closes #3342
Summary
assets/fonts/and serializefontPreferencesinassets/index.jsonbasic-chinese, defaultDependency
Runtime text registration requires the corresponding SPX implementation from goplus/spx#1674. This PR already passes the project font files through the existing runner file path.