-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix(labels): Scale SDF glyphs to max label size #13070
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for the pull request, @donmccurdy! ✅ We can confirm we have a CLA on file for you. |
| character, | ||
| label._fontFamily, | ||
| label._fontStyle, | ||
| label._fontSize, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: This line was included to force updates in the texture atlas, but managing the glyph cache correctly would take more investigation.
|
Mentioning that "custom |
|
@javagl let's try to revive it! |
8d8da7b to
8aeafac
Compare
a8ce35e to
6f1487b
Compare
9b547a2 to
d9c1565
Compare
Description
DRAFT - for discussion.
Currently label screen-space size is derived from
label.font x label.scale, while SDF size (used to display the font in WebGL) is fixed at 48px:cesium/packages/engine/Source/Scene/SDFSettings.js
Lines 6 to 13 in aebb58a
The idea explored by this PR is to use SDFSettings.FONT_SIZE as a minimum, scaling up the glyph size to match the maximum font size present in the label collection. Increasing label font size should then never result in deteriorating quality. Note that
.scaledoes not change the size of the SDF glyph, and could still magnify a low-resolution glyph, but I think that's acceptable.Two big tradeoffs:
Texture atlas size. Large font sizes require larger glyphs and a larger atlas. Using the max font size (not all requested font sizes) is a compromise so that only one font size is stored in the atlas at a time. I expect that label sizes larger than 48px are rare, so most users will continue to have current SDF size (48px) unchanged. That said, I'd be open to the argument that each unique size should be stored; in a collection with many small labels (and, say, 1000+ characters in some languages) and only 1–2 very large labels, it would reduce atlas size to store each label's glyphs at exactly the requested font size.
Custom
measureText()implementation performance. We iterate over pixels to compute glyph bounding boxes, and becomes slower as glyph sizes increase. For example, a 30-character label at fontSize=400px spends 100ms inrebindAllGlyphscompared to 25ms at fontSize/48px. Perhaps this could be resolved separately by InvestigatewriteTextToCanvasandmeasureTextfunction #9767. I think it's only a minor issue at this stage.Preview:
Issue number and link
Testing plan
TODO
Author checklist
CONTRIBUTORS.mdCHANGES.mdwith a short summary of my changePR Dependency Tree
This tree was auto-generated by Charcoal