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

[core] Replace three maps/mutexes in GlyphAtlas with a single map and mutex #8199

Merged
merged 1 commit into from
Feb 28, 2017
Merged

[core] Replace three maps/mutexes in GlyphAtlas with a single map and mutex #8199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/mbgl/layout/symbol_layout.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
43 changes: 16 additions & 27 deletions src/mbgl/text/glyph_atlas.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,26 @@ 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) {
if (glyphRanges.empty()) {
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) {
Expand All @@ -55,7 +56,7 @@ bool GlyphAtlas::hasGlyphRanges(const FontStack& fontStack, const GlyphRangeSet&
continue;
}

if (!rangeSetsIt->second->isParsed()) {
if (!rangeSetsIt->second.isParsed()) {
hasRanges = false;
}
}
Expand All @@ -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_) {
Expand All @@ -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)
{
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
29 changes: 13 additions & 16 deletions src/mbgl/text/glyph_atlas.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
#include <unordered_set>
#include <unordered_map>
#include <mutex>
#include <exception>
#include <vector>

namespace mbgl {

Expand Down Expand Up @@ -57,7 +55,7 @@ class GlyphAtlas : public util::noncopyable {
void addGlyphs(uintptr_t tileUID,
const std::u16string& text,
const FontStack&,
const GlyphSet&,
const util::exclusive<GlyphSet>&,
GlyphPositions&);
void removeGlyphs(uintptr_t tileUID);

Expand All @@ -77,30 +75,29 @@ class GlyphAtlas : public util::noncopyable {
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 }) {}
Rect<uint16_t> rect;
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;
Expand Down
22 changes: 10 additions & 12 deletions test/text/glyph_atlas.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -144,21 +142,21 @@ TEST(GlyphAtlas, LoadingCancel) {
}

TEST(GlyphAtlas, InvalidSDFGlyph) {
GlyphSet glyphSet;
glyphSet.insert(66, SDFGlyph{ 66 /* ASCII 'B' */,
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' */,
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;
test.glyphAtlas.addGlyphs(1, std::u16string{u"ABC"}, fontStack, glyphSet, positions);

ASSERT_EQ(2u, positions.size());
Expand Down