[core] Replace three maps/mutexes in GlyphAtlas with a single map and mutex
This commit is contained in:
@@ -256,7 +256,7 @@ void SymbolLayout::prepare(uintptr_t tileUID,
|
||||
|
||||
// Add the glyphs we need for this label to the glyph atlas.
|
||||
if (result) {
|
||||
glyphAtlas.addGlyphs(tileUID, text, layout.get<TextFont>(), **glyphSet, face);
|
||||
glyphAtlas.addGlyphs(tileUID, text, layout.get<TextFont>(), glyphSet, face);
|
||||
}
|
||||
|
||||
return result;
|
||||
|
||||
@@ -23,16 +23,17 @@ GlyphAtlas::GlyphAtlas(const Size size, FileSource& fileSource_)
|
||||
GlyphAtlas::~GlyphAtlas() = default;
|
||||
|
||||
void GlyphAtlas::requestGlyphRange(const FontStack& fontStack, const GlyphRange& range) {
|
||||
std::lock_guard<std::mutex> lock(rangesMutex);
|
||||
auto& rangeSets = ranges[fontStack];
|
||||
std::lock_guard<std::mutex> lock(mutex);
|
||||
auto& rangeSets = entries[fontStack].ranges;
|
||||
|
||||
const auto& rangeSetsIt = rangeSets.find(range);
|
||||
if (rangeSetsIt != rangeSets.end()) {
|
||||
return;
|
||||
}
|
||||
|
||||
rangeSets.emplace(range,
|
||||
std::make_unique<GlyphPBF>(this, fontStack, range, observer, fileSource));
|
||||
rangeSets.emplace(std::piecewise_construct,
|
||||
std::forward_as_tuple(range),
|
||||
std::forward_as_tuple(this, fontStack, range, observer, fileSource));
|
||||
}
|
||||
|
||||
bool GlyphAtlas::hasGlyphRanges(const FontStack& fontStack, const GlyphRangeSet& glyphRanges) {
|
||||
@@ -40,8 +41,8 @@ bool GlyphAtlas::hasGlyphRanges(const FontStack& fontStack, const GlyphRangeSet&
|
||||
return true;
|
||||
}
|
||||
|
||||
std::lock_guard<std::mutex> lock(rangesMutex);
|
||||
const auto& rangeSets = ranges[fontStack];
|
||||
std::lock_guard<std::mutex> lock(mutex);
|
||||
const auto& rangeSets = entries[fontStack].ranges;
|
||||
|
||||
bool hasRanges = true;
|
||||
for (const auto& range : glyphRanges) {
|
||||
@@ -55,7 +56,7 @@ bool GlyphAtlas::hasGlyphRanges(const FontStack& fontStack, const GlyphRangeSet&
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!rangeSetsIt->second->isParsed()) {
|
||||
if (!rangeSetsIt->second.isParsed()) {
|
||||
hasRanges = false;
|
||||
}
|
||||
}
|
||||
@@ -64,16 +65,8 @@ bool GlyphAtlas::hasGlyphRanges(const FontStack& fontStack, const GlyphRangeSet&
|
||||
}
|
||||
|
||||
util::exclusive<GlyphSet> GlyphAtlas::getGlyphSet(const FontStack& fontStack) {
|
||||
auto lock = std::make_unique<std::lock_guard<std::mutex>>(glyphSetsMutex);
|
||||
|
||||
auto it = glyphSets.find(fontStack);
|
||||
if (it == glyphSets.end()) {
|
||||
it = glyphSets.emplace(fontStack, std::make_unique<GlyphSet>()).first;
|
||||
}
|
||||
|
||||
// FIXME: We lock all GlyphSets, but what we should
|
||||
// really do is lock only the one we are returning.
|
||||
return { it->second.get(), std::move(lock) };
|
||||
auto lock = std::make_unique<std::lock_guard<std::mutex>>(mutex);
|
||||
return { &entries[fontStack].glyphSet, std::move(lock) };
|
||||
}
|
||||
|
||||
void GlyphAtlas::setObserver(GlyphAtlasObserver* observer_) {
|
||||
@@ -83,12 +76,10 @@ void GlyphAtlas::setObserver(GlyphAtlasObserver* observer_) {
|
||||
void GlyphAtlas::addGlyphs(uintptr_t tileUID,
|
||||
const std::u16string& text,
|
||||
const FontStack& fontStack,
|
||||
const GlyphSet& glyphSet,
|
||||
const util::exclusive<GlyphSet>& glyphSet,
|
||||
GlyphPositions& face)
|
||||
{
|
||||
std::lock_guard<std::mutex> lock(mtx);
|
||||
|
||||
const std::map<uint32_t, SDFGlyph>& sdfs = glyphSet.getSDFs();
|
||||
const std::map<uint32_t, SDFGlyph>& sdfs = glyphSet->getSDFs();
|
||||
|
||||
for (char16_t chr : text)
|
||||
{
|
||||
@@ -107,7 +98,7 @@ Rect<uint16_t> GlyphAtlas::addGlyph(uintptr_t tileUID,
|
||||
const FontStack& fontStack,
|
||||
const SDFGlyph& glyph)
|
||||
{
|
||||
std::map<uint32_t, GlyphValue>& face = index[fontStack];
|
||||
std::map<uint32_t, GlyphValue>& face = entries[fontStack].glyphValues;
|
||||
auto it = face.find(glyph.id);
|
||||
|
||||
// The glyph is already in this texture.
|
||||
@@ -151,10 +142,10 @@ Rect<uint16_t> GlyphAtlas::addGlyph(uintptr_t tileUID,
|
||||
}
|
||||
|
||||
void GlyphAtlas::removeGlyphs(uintptr_t tileUID) {
|
||||
std::lock_guard<std::mutex> lock(mtx);
|
||||
std::lock_guard<std::mutex> lock(mutex);
|
||||
|
||||
for (auto& faces : index) {
|
||||
std::map<uint32_t, GlyphValue>& face = faces.second;
|
||||
for (auto& entry : entries) {
|
||||
std::map<uint32_t, GlyphValue>& face = entry.second.glyphValues;
|
||||
for (auto it = face.begin(); it != face.end(); /* we advance in the body */) {
|
||||
GlyphValue& value = it->second;
|
||||
value.ids.erase(tileUID);
|
||||
@@ -190,8 +181,6 @@ Size GlyphAtlas::getSize() const {
|
||||
}
|
||||
|
||||
void GlyphAtlas::upload(gl::Context& context, gl::TextureUnit unit) {
|
||||
std::lock_guard<std::mutex> lock(mtx);
|
||||
|
||||
if (!texture) {
|
||||
texture = context.createTexture(image, unit);
|
||||
} else if (dirty) {
|
||||
|
||||
@@ -17,8 +17,6 @@
|
||||
#include <unordered_set>
|
||||
#include <unordered_map>
|
||||
#include <mutex>
|
||||
#include <exception>
|
||||
#include <vector>
|
||||
|
||||
namespace mbgl {
|
||||
|
||||
@@ -57,7 +55,7 @@ public:
|
||||
void addGlyphs(uintptr_t tileUID,
|
||||
const std::u16string& text,
|
||||
const FontStack&,
|
||||
const GlyphSet&,
|
||||
const util::exclusive<GlyphSet>&,
|
||||
GlyphPositions&);
|
||||
void removeGlyphs(uintptr_t tileUID);
|
||||
|
||||
@@ -77,20 +75,9 @@ private:
|
||||
const FontStack&,
|
||||
const SDFGlyph&);
|
||||
|
||||
|
||||
FileSource& fileSource;
|
||||
std::string glyphURL;
|
||||
|
||||
std::unordered_map<FontStack, std::map<GlyphRange, std::unique_ptr<GlyphPBF>>, FontStackHash> ranges;
|
||||
std::mutex rangesMutex;
|
||||
|
||||
std::unordered_map<FontStack, std::unique_ptr<GlyphSet>, FontStackHash> glyphSets;
|
||||
std::mutex glyphSetsMutex;
|
||||
|
||||
util::WorkQueue workQueue;
|
||||
|
||||
GlyphAtlasObserver* observer = nullptr;
|
||||
|
||||
struct GlyphValue {
|
||||
GlyphValue(Rect<uint16_t> rect_, uintptr_t id)
|
||||
: rect(std::move(rect_)), ids({ id }) {}
|
||||
@@ -98,9 +85,19 @@ private:
|
||||
std::unordered_set<uintptr_t> ids;
|
||||
};
|
||||
|
||||
std::mutex mtx;
|
||||
struct Entry {
|
||||
std::map<GlyphRange, GlyphPBF> ranges;
|
||||
GlyphSet glyphSet;
|
||||
std::map<uint32_t, GlyphValue> glyphValues;
|
||||
};
|
||||
|
||||
std::unordered_map<FontStack, Entry, FontStackHash> entries;
|
||||
std::mutex mutex;
|
||||
|
||||
util::WorkQueue workQueue;
|
||||
GlyphAtlasObserver* observer = nullptr;
|
||||
|
||||
BinPack<uint16_t> bin;
|
||||
std::unordered_map<FontStack, std::map<uint32_t, GlyphValue>, FontStackHash> index;
|
||||
AlphaImage image;
|
||||
std::atomic<bool> dirty;
|
||||
mbgl::optional<gl::Texture> texture;
|
||||
|
||||
@@ -83,8 +83,7 @@ TEST(GlyphAtlas, LoadingFail) {
|
||||
EXPECT_TRUE(error != nullptr);
|
||||
EXPECT_EQ(util::toString(error), "Failed by the test case");
|
||||
|
||||
auto glyphSet = test.glyphAtlas.getGlyphSet({{"Test Stack"}});
|
||||
ASSERT_TRUE(glyphSet->getSDFs().empty());
|
||||
ASSERT_TRUE(test.glyphAtlas.getGlyphSet({{"Test Stack"}})->getSDFs().empty());
|
||||
ASSERT_FALSE(test.glyphAtlas.hasGlyphRanges({{"Test Stack"}}, {{0, 255}}));
|
||||
|
||||
test.end();
|
||||
@@ -112,8 +111,7 @@ TEST(GlyphAtlas, LoadingCorrupted) {
|
||||
EXPECT_TRUE(error != nullptr);
|
||||
EXPECT_EQ(util::toString(error), "unknown pbf field type exception");
|
||||
|
||||
auto glyphSet = test.glyphAtlas.getGlyphSet({{"Test Stack"}});
|
||||
ASSERT_TRUE(glyphSet->getSDFs().empty());
|
||||
ASSERT_TRUE(test.glyphAtlas.getGlyphSet({{"Test Stack"}})->getSDFs().empty());
|
||||
ASSERT_FALSE(test.glyphAtlas.hasGlyphRanges({{"Test Stack"}}, {{0, 255}}));
|
||||
|
||||
test.end();
|
||||
@@ -144,21 +142,21 @@ TEST(GlyphAtlas, LoadingCancel) {
|
||||
}
|
||||
|
||||
TEST(GlyphAtlas, InvalidSDFGlyph) {
|
||||
GlyphSet glyphSet;
|
||||
glyphSet.insert(66, SDFGlyph{ 66 /* ASCII 'B' */,
|
||||
AlphaImage({7, 7}), /* correct */
|
||||
{ 1 /* width */, 1 /* height */, 0 /* left */, 0 /* top */,
|
||||
0 /* advance */ } });
|
||||
glyphSet.insert(67, SDFGlyph{ 67 /* ASCII 'C' */,
|
||||
AlphaImage({518, 8}), /* correct */
|
||||
{ 512 /* width */, 2 /* height */, 0 /* left */, 0 /* top */,
|
||||
0 /* advance */ } });
|
||||
|
||||
|
||||
const FontStack fontStack{ "Mock Font" };
|
||||
|
||||
GlyphAtlasTest test;
|
||||
GlyphPositions positions;
|
||||
|
||||
auto glyphSet = test.glyphAtlas.getGlyphSet(fontStack);
|
||||
glyphSet->insert(66, SDFGlyph{ 66 /* ASCII 'B' */,
|
||||
AlphaImage({7, 7}), /* correct */
|
||||
{ 1 /* width */, 1 /* height */, 0 /* left */, 0 /* top */,
|
||||
0 /* advance */ } });
|
||||
glyphSet->insert(67, SDFGlyph{ 67 /* ASCII 'C' */,
|
||||
AlphaImage({518, 8}), /* correct */
|
||||
{ 512 /* width */, 2 /* height */, 0 /* left */, 0 /* top */,
|
||||
0 /* advance */ } });
|
||||
|
||||
test.glyphAtlas.addGlyphs(1, std::u16string{u"ABC"}, fontStack, glyphSet, positions);
|
||||
|
||||
ASSERT_EQ(2u, positions.size());
|
||||
|
||||
Reference in New Issue
Block a user