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.

Sign up for GitHub

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

Jump to bottom

Upgrade to TinySDF v2 with a fix to CJK performance #11047

Merged
merged 2 commits into from
Sep 24, 2021
Merged

Upgrade to TinySDF v2 with a fix to CJK performance #11047

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76b587e
upgrade to TinySDF v2
mourner Sep 22, 2021
ce24708
update tiny-sdf to v2.0.2 to address a perf issue
mourner Sep 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"@mapbox/jsonlint-lines-primitives": "^2.0.2",
"@mapbox/mapbox-gl-supported": "^2.0.0",
"@mapbox/point-geometry": "^0.1.0",
"@mapbox/tiny-sdf": "^1.2.5",
"@mapbox/tiny-sdf": "^2.0.0",
"@mapbox/unitbezier": "^0.0.0",
"@mapbox/vector-tile": "^1.3.1",
"@mapbox/whoots-js": "^3.1.0",
Expand Down
36 changes: 16 additions & 20 deletions src/render/glyph_manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,8 @@ class GlyphManager {
}

_tinySDF(entry: Entry, stack: string, id: number): ?StyleGlyph {
const family = this.localFontFamily;
if (!family) {
return;
}

if (!this._doesCharSupportLocalGlyph(id)) {
return;
}
const fontFamily = this.localFontFamily;
if (!fontFamily || !this._doesCharSupportLocalGlyph(id)) return;

let tinySDF = entry.tinySDF;
if (!tinySDF) {
Expand All @@ -218,15 +212,20 @@ class GlyphManager {
} else if (/light/i.test(stack)) {
fontWeight = '200';
}
tinySDF = entry.tinySDF = new GlyphManager.TinySDF(24 * SDF_SCALE, 3 * SDF_SCALE, 8 * SDF_SCALE, .25, family, fontWeight);

const fontSize = 24 * SDF_SCALE;
const buffer = 3 * SDF_SCALE;
const radius = 8 * SDF_SCALE;
tinySDF = entry.tinySDF = new GlyphManager.TinySDF({fontFamily, fontWeight, fontSize, buffer, radius});
tinySDF.fontWeight = fontWeight;
Comment on lines +219 to +220
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, why do we need to set tinySDF.fontWeight when fontWeight is passed into the constructor?

Copy link
Member Author

Choose a reason for hiding this comment

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

@ryanhamley it was never a part of public TinySDF API, it just happened to be exposed there but is no longer the case with v2.

}

if (this.localGlyphs[tinySDF.fontWeight][id]) {
return this.localGlyphs[tinySDF.fontWeight][id];
}

const {data, metrics} = tinySDF.drawWithMetrics(String.fromCharCode(id));
const {sdfWidth, sdfHeight, width, height, left, top, advance} = metrics;
const char = String.fromCharCode(id);
const {data, width, height, glyphWidth, glyphHeight, glyphLeft, glyphTop, glyphAdvance} = tinySDF.draw(char);
/*
TinySDF's "top" is the distance from the alphabetic baseline to the
top of the glyph.
Expand All @@ -248,16 +247,13 @@ class GlyphManager {

const glyph = this.localGlyphs[tinySDF.fontWeight][id] = {
id,
bitmap: new AlphaImage({
width: sdfWidth,
height: sdfHeight
}, data),
bitmap: new AlphaImage({width, height}, data),
metrics: {
width: width / SDF_SCALE,
height: height / SDF_SCALE,
left: left / SDF_SCALE,
top: top / SDF_SCALE - baselineAdjustment,
advance: advance / SDF_SCALE,
width: glyphWidth / SDF_SCALE,
height: glyphHeight / SDF_SCALE,
left: glyphLeft / SDF_SCALE,
top: glyphTop / SDF_SCALE - baselineAdjustment,
advance: glyphAdvance / SDF_SCALE,
localGlyph: true
}
};
Expand Down
32 changes: 22 additions & 10 deletions test/unit/render/glyph_manager.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@ const TinySDF = class {
this.fontWeight = '400';
}
// Return empty 30x30 bitmap (24 fontsize + 3 * 2 buffer)
drawWithMetrics() {
draw() {
return {
alphaChannel: new Uint8ClampedArray(900),
metrics: {width: 48, height: 48, sdfWidth: 30, sdfHeight: 30, advance: 48}
data: new Uint8ClampedArray(900),
glyphWidth: 48,
glyphHeight: 48,
width: 30,
height: 30,
glyphAdvance: 48
};
}
};
Expand Down Expand Up @@ -168,11 +172,15 @@ test('GlyphManager caches locally generated glyphs', (t) => {
this.fontWeight = '400';
}
// Return empty 30x30 bitmap (24 fontsize + 3 * 2 buffer)
drawWithMetrics() {
draw() {
drawCallCount++;
return {
alphaChannel: new Uint8ClampedArray(900),
metrics: {width: 48, height: 48, sdfWidth: 30, sdfHeight: 30, advance: 48}
data: new Uint8ClampedArray(900),
glyphWidth: 48,
glyphHeight: 48,
width: 30,
height: 30,
glyphAdvance: 48
};
}
});
Expand All @@ -195,11 +203,15 @@ test('GlyphManager locally generates latin glyphs', (t) => {
constructor() {
this.fontWeight = '400';
}
// Return empty 18x24 bitmap (made up glyph size + 3 * 2 buffer)
drawWithMetrics() {
// Return empty 20x24 bitmap (made up glyph size + 3 * 2 buffer)
draw() {
return {
alphaChannel: new Uint8ClampedArray(480),
metrics: {width: 28, height: 36, sdfWidth: 20, sdfHeight: 24, advance: 20}
data: new Uint8ClampedArray(480),
glyphWidth: 28,
glyphHeight: 36,
width: 20,
height: 24,
glyphAdvance: 20
};
}
});
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1171,10 +1171,10 @@
resolved "https://registry.npmjs.org/@mapbox/sphericalmercator/-/sphericalmercator-1.1.0.tgz"
integrity sha512-pEsfZyG4OMThlfFQbCte4gegvHUjxXCjz0KZ4Xk8NdOYTQBLflj6U8PL05RPAiuRAMAQNUUKJuL6qYZ5Y4kAWA==

"@mapbox/tiny-sdf@^1.2.5":
version "1.2.5"
resolved "https://registry.npmjs.org/@mapbox/tiny-sdf/-/tiny-sdf-1.2.5.tgz"
integrity sha512-cD8A/zJlm6fdJOk6DqPUV8mcpyJkRz2x2R+/fYcWDYG3oWbG7/L7Yl/WqQ1VZCjnL9OTIMAn6c+BC5Eru4sQEw==
"@mapbox/tiny-sdf@^2.0.0":
version "2.0.0"
resolved "https://registry.yarnpkg.com/@mapbox/tiny-sdf/-/tiny-sdf-2.0.0.tgz#1e34a94b6f4c95d877652be4d399b4476792ac44"
integrity sha512-2+rBo+sFynCpXuEoM8aYq2pw6bqVTyXyl6ALUgJUmBfz293fK7y3n0BvxjdOKrx3PyZRAzNH32JoLbvU2BTx8Q==

"@mapbox/unitbezier@^0.0.0":
version "0.0.0"
Expand Down