Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 45 additions & 27 deletions src/tables/glyf.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ import glyphset from '../glyphset.mjs';
import parse from '../parse.mjs';
import Path from '../path.mjs';

// Track glyph indices currently being resolved to detect circular references
// in composite glyphs. Keyed by GlyphSet to avoid cross-font collisions.
// Module-scoped because the recursion crosses through the lazy glyph loader
// (glyphset.mjs), so a parameter can't be threaded through.
const _resolving = new WeakMap();

// Parse the coordinate data for a glyph.
function parseGlyphCoordinate(p, flag, previousValue, shortVectorBitMask, sameBitMask) {
let v;
Expand Down Expand Up @@ -262,36 +268,48 @@ function getPath(points) {

function buildPath(glyphs, glyph) {
if (glyph.isComposite) {
for (let j = 0; j < glyph.components.length; j += 1) {
const component = glyph.components[j];
const componentGlyph = glyphs.get(component.glyphIndex);
// Force the ttfGlyphLoader to parse the glyph.
componentGlyph.getPath();
if (componentGlyph.points) {
let transformedPoints;
if (component.matchedPoints === undefined) {
// component positioned by offset
transformedPoints = transformPoints(componentGlyph.points, component);
} else {
// component positioned by matched points
if ((component.matchedPoints[0] > glyph.points.length - 1) ||
(component.matchedPoints[1] > componentGlyph.points.length - 1)) {
throw Error('Matched points out of range in ' + glyph.name);
if (!_resolving.has(glyphs)) {
_resolving.set(glyphs, new Set());
}
const resolving = _resolving.get(glyphs);
resolving.add(glyph.index);
try {
for (let j = 0; j < glyph.components.length; j += 1) {
const component = glyph.components[j];
if (resolving.has(component.glyphIndex)) {
continue; // skip circular reference
}
const componentGlyph = glyphs.get(component.glyphIndex);
// Force the ttfGlyphLoader to parse the glyph.
componentGlyph.getPath();
if (componentGlyph.points) {
let transformedPoints;
if (component.matchedPoints === undefined) {
// component positioned by offset
transformedPoints = transformPoints(componentGlyph.points, component);
} else {
// component positioned by matched points
if ((component.matchedPoints[0] > glyph.points.length - 1) ||
(component.matchedPoints[1] > componentGlyph.points.length - 1)) {
throw Error('Matched points out of range in ' + glyph.name);
}
const firstPt = glyph.points[component.matchedPoints[0]];
let secondPt = componentGlyph.points[component.matchedPoints[1]];
const transform = {
xScale: component.xScale, scale01: component.scale01,
scale10: component.scale10, yScale: component.yScale,
dx: 0, dy: 0
};
secondPt = transformPoints([secondPt], transform)[0];
transform.dx = firstPt.x - secondPt.x;
transform.dy = firstPt.y - secondPt.y;
transformedPoints = transformPoints(componentGlyph.points, transform);
}
const firstPt = glyph.points[component.matchedPoints[0]];
let secondPt = componentGlyph.points[component.matchedPoints[1]];
const transform = {
xScale: component.xScale, scale01: component.scale01,
scale10: component.scale10, yScale: component.yScale,
dx: 0, dy: 0
};
secondPt = transformPoints([secondPt], transform)[0];
transform.dx = firstPt.x - secondPt.x;
transform.dy = firstPt.y - secondPt.y;
transformedPoints = transformPoints(componentGlyph.points, transform);
glyph.points = glyph.points.concat(transformedPoints);
}
glyph.points = glyph.points.concat(transformedPoints);
}
} finally {
resolving.delete(glyph.index);
}
Comment on lines 270 to 313
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_resolving is module-scoped and keyed only by glyph index. If two different fonts (or separate GlyphSet instances) resolve glyphs with the same index interleaved in the same process, this can cause false circular-reference detection and silently drop components. Also, using a plain Set can be unsafe under re-entrant calls: adding an already-present index and then unconditionally deleting it in finally can clear the marker while an outer frame is still resolving. Consider scoping the tracking to the font/glyphset (e.g., store a Set/Map on glyphs/glyph.font, or use a WeakMap<GlyphSet, Map<number, count>>) and only remove the marker if this frame added it (or decrement a refcount).

Copilot uses AI. Check for mistakes.
}

Expand Down
Binary file added test/fonts/circular-composite.ttf
Binary file not shown.
Loading
Loading