fix(labels): Scale SDF glyphs to max label size
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:
https://github.com/CesiumGS/cesium/blob/aebb58ad06733fced57b2bc4a099c2ffd85f1540/packages/engine/Source/Scene/SDFSettings.js#L6-L13
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 .scale does 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 https://github.com/CesiumGS/cesium/issues/9767. I think it's only a minor issue at this stage.
Preview:
| fontSize=12 | fontSize=48 | fontSize=144 |
|---|---|---|
Issue number and link
- Fixes #11377
Testing plan
TODO
Author checklist
- [x] I have submitted a Contributor License Agreement
- [x] I have added my name to
CONTRIBUTORS.md - [ ] I have updated
CHANGES.mdwith a short summary of my change - [ ] I have added or updated unit tests to ensure consistent code coverage
- [ ] I have updated the inline documentation, and included code examples where relevant
- [ ] I have performed a self-review of my code
PR Dependency Tree
- PR #13070 👈
This tree was auto-generated by Charcoal
Thank you for the pull request, @donmccurdy!
:white_check_mark: We can confirm we have a CLA on file for you.
Mentioning that "custom measureText" triggered some memories about https://github.com/CesiumGS/cesium/pull/11747 . It should be easy (nearly trivial) to replace it, and IIRC that PR was already pretty complete, but ... there have been ~"some pixels somehow been different in some cases", with no clear reasoning for what is "better" or "worse", so it was abandoned. Maybe with all the other recent fixes in this area, it's really trivial now.
@javagl let's try to revive it!
- https://github.com/CesiumGS/cesium/pull/13081