Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Add eviction strategy for Glyphs #12388

Closed
kkaefer opened this issue Jul 16, 2018 · 1 comment
Closed

Add eviction strategy for Glyphs #12388

kkaefer opened this issue Jul 16, 2018 · 1 comment
Assignees
Labels
Core The cross-platform C++ core, aka mbgl refactor

Comments

@kkaefer
Copy link
Contributor

kkaefer commented Jul 16, 2018

In #9213, we factored out glyph management from the GlyphAtlas into its own class called GlyphManager. Previously, glyphs were stored and associated with a style, which meant that setting a new style also removed all cached glyphs. This isn't true anymore after #9213, since the GlyphManager is now owned by the Renderer::Impl, which persists across styles changes.

GlyphManager only ever has Glyphs/PBFs added to it, never removed, including glyphs from previously loaded styles that aren't used anymore. We should add an eviction strategy for glyphs that are unused so that they don't stick around forever in long-lived Map objects.

@jfirebaugh thinks that this could be fairly straightforward: promote style::Parser::fontStacks to a more general location, and then in Renderer::Impl::render, remove any unused font stacks from GlyphManager when the layer diff is non-empty.

@kkaefer kkaefer added refactor Core The cross-platform C++ core, aka mbgl labels Jul 16, 2018
@jfirebaugh
Copy link
Contributor

Note that the proposed change will turn what is currently a warning:

Layer '%s' has an invalid value for text-font and will not work offline. Output values must be contained as literals within the expression.

...into a hard error: symbol layers that don't conform to this constraint will not render text.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl refactor
Projects
None yet
Development

No branches or pull requests

2 participants