tei/MapLibreParticleLab
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[core] Replace GlyphRangeSet in onGlyphsAvailable with optionals in the map

Browse Source 
GlyphRangeSet isn't keyed by FontStack, so using it to indicate that a particular range was loaded could have produced false positives.
This commit is contained in:
John Firebaugh
2017-04-06 09:20:07 -07:00
parent 09a2271576
commit a50d7a331b
10 changed files with 56 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
src/mbgl/text/glyph.hpp
View File

@@ -4,6 +4,7 @@
#include <mbgl/util/font_stack.hpp>
#include <mbgl/util/rect.hpp>
#include <mbgl/util/traits.hpp>
#include <mbgl/util/optional.hpp>
#include <cstdint>
#include <vector>
@@ -19,17 +20,11 @@ typedef std::set<GlyphID> GlyphIDs;
GlyphRange getGlyphRange(GlyphID glyph);
struct GlyphMetrics {
explicit operator bool() const {
return !(width == 0 && height == 0 && advance == 0);
}
// Glyph metrics.
uint32_t width = 0;
uint32_t height = 0;
int32_t left = 0;
int32_t top = 0;
uint32_t advance = 0;
};
inline bool operator==(const GlyphMetrics& lhs, const GlyphMetrics& rhs) {
@@ -41,19 +36,12 @@ inline bool operator==(const GlyphMetrics& lhs, const GlyphMetrics& rhs) {
}
struct Glyph {
explicit Glyph() : rect(0, 0, 0, 0), metrics() {}
explicit Glyph(Rect<uint16_t> rect_, GlyphMetrics metrics_)
: rect(std::move(rect_)), metrics(std::move(metrics_)) {}
explicit operator bool() const {
return metrics || rect.hasArea();
}
const Rect<uint16_t> rect;
const GlyphMetrics metrics;
Rect<uint16_t> rect;
GlyphMetrics metrics;
};
typedef std::map<GlyphID, Glyph> GlyphPositions;
typedef std::map<GlyphID, optional<Glyph>> GlyphPositions;
typedef std::map<FontStack, GlyphPositions> GlyphPositionMap;
class PositionedGlyph {
public:
@@ -111,7 +99,6 @@ constexpr WritingModeType operator~(WritingModeType value) {
typedef std::map<FontStack,GlyphIDs> GlyphDependencies;
typedef std::map<FontStack,GlyphRangeSet> GlyphRangeDependencies;
typedef std::map<FontStack,GlyphPositions> GlyphPositionMap;
} // end namespace mbgl

21
src/mbgl/text/glyph_atlas.cpp
View File

@@ -137,7 +137,6 @@ void GlyphAtlas::setObserver(GlyphAtlasObserver* observer_) {
void GlyphAtlas::addGlyphs(GlyphRequestor& requestor, const GlyphDependencies& glyphDependencies) {
GlyphPositionMap glyphPositions;
GlyphRangeSet loadedRanges;
for (const auto& dependency : glyphDependencies) {
const FontStack& fontStack = dependency.first;
@@ -147,27 +146,23 @@ void GlyphAtlas::addGlyphs(GlyphRequestor& requestor, const GlyphDependencies& g
Entry& entry = entries[fontStack];
for (const auto& glyphID : glyphIDs) {
loadedRanges.insert(getGlyphRange(glyphID));
optional<Glyph>& glyph = positions[glyphID];
auto it = entry.sdfs.find(glyphID);
if (it == entry.sdfs.end())
continue;
addGlyph(requestor, fontStack, it->second);
// It's possible to have an SDF without a valid position (if the SDF was malformed).
// We indicate this case with Rect<uint16_t>(0,0,0,0).
auto glyphRect = entry.glyphValues.find(glyphID);
const Rect<uint16_t> rect = glyphRect == entry.glyphValues.end()
? Rect<uint16_t>(0,0,0,0)
: glyphRect->second.rect;
positions.emplace(std::piecewise_construct,
std::forward_as_tuple(glyphID),
std::forward_as_tuple(rect, it->second.metrics));
auto valueIt = entry.glyphValues.find(glyphID);
glyph = Glyph {
valueIt == entry.glyphValues.end() ? Rect<uint16_t>() : valueIt->second.rect,
it->second.metrics
};
}
}
requestor.onGlyphsAvailable(glyphPositions, loadedRanges);
requestor.onGlyphsAvailable(glyphPositions);
}
void GlyphAtlas::addGlyph(GlyphRequestor& requestor,

2
src/mbgl/text/glyph_atlas.hpp
View File

@@ -31,7 +31,7 @@ class Context;
class GlyphRequestor {
public:
virtual void onGlyphsAvailable(GlyphPositionMap, GlyphRangeSet) = 0;
virtual void onGlyphsAvailable(GlyphPositionMap) = 0;
};
class GlyphAtlas : public util::noncopyable {

12
src/mbgl/text/quads.cpp
View File

@@ -303,17 +303,11 @@ SymbolQuads getGlyphQuads(Anchor& anchor,
for (const PositionedGlyph &positionedGlyph: shapedText.positionedGlyphs) {
auto face_it = face.find(positionedGlyph.glyph);
if (face_it == face.end())
continue;
const Glyph &glyph = face_it->second;
const Rect<uint16_t> &rect = glyph.rect;
if (!glyph)
continue;
if (!rect.hasArea())
if (face_it == face.end() || !face_it->second || !(*face_it->second).rect.hasArea())
continue;
const Glyph& glyph = *face_it->second;
const Rect<uint16_t>& rect = glyph.rect;
const float centerX = (positionedGlyph.x + glyph.metrics.advance / 2.0f) * boxScale;
GlyphInstances glyphInstances;

16
src/mbgl/text/shaping.cpp
View File

@@ -56,8 +56,8 @@ void justifyLine(std::vector<PositionedGlyph>& positionedGlyphs,
PositionedGlyph& glyph = positionedGlyphs[end];
auto it = glyphs.find(glyph.glyph);
if (it != glyphs.end()) {
const uint32_t lastAdvance = it->second.metrics.advance;
if (it != glyphs.end() && it->second) {
const uint32_t lastAdvance = it->second->metrics.advance;
const float lineIndent = float(glyph.x + lastAdvance) * justify;
for (std::size_t j = start; j <= end; j++) {
@@ -74,8 +74,8 @@ float determineAverageLineWidth(const std::u16string& logicalInput,
for (char16_t chr : logicalInput) {
auto it = glyphs.find(chr);
if (it != glyphs.end()) {
totalWidth += it->second.metrics.advance + spacing;
if (it != glyphs.end() && it->second) {
totalWidth += it->second->metrics.advance + spacing;
}
}
@@ -185,8 +185,8 @@ std::set<std::size_t> determineLineBreaks(const std::u16string& logicalInput,
for (std::size_t i = 0; i < logicalInput.size(); i++) {
const char16_t codePoint = logicalInput[i];
auto it = glyphs.find(codePoint);
if (it != glyphs.end() && !boost::algorithm::is_any_of(u" \t\n\v\f\r")(codePoint)) {
currentX += it->second.metrics.advance + spacing;
if (it != glyphs.end() && it->second && !boost::algorithm::is_any_of(u" \t\n\v\f\r")(codePoint)) {
currentX += it->second->metrics.advance + spacing;
}
// Ideographic characters, spaces, and word-breaking punctuation that often appear without
@@ -234,11 +234,11 @@ void shapeLines(Shaping& shaping,
std::size_t lineStartIndex = shaping.positionedGlyphs.size();
for (char16_t chr : line) {
auto it = glyphs.find(chr);
if (it == glyphs.end()) {
if (it == glyphs.end() || !it->second) {
continue;
}
const Glyph& glyph = it->second;
const Glyph& glyph = *it->second;
if (writingMode == WritingModeType::Horizontal || !util::i18n::hasUprightVerticalOrientation(chr)) {
shaping.positionedGlyphs.emplace_back(chr, x, y, 0);

4
src/mbgl/tile/geometry_tile.cpp
View File

@@ -133,8 +133,8 @@ void GeometryTile::onError(std::exception_ptr err) {
observer->onTileError(*this, err);
}
void GeometryTile::onGlyphsAvailable(GlyphPositionMap glyphPositions, GlyphRangeSet loadedRanges) {
worker.invoke(&GeometryTileWorker::onGlyphsAvailable, std::move(glyphPositions), std::move(loadedRanges));
void GeometryTile::onGlyphsAvailable(GlyphPositionMap glyphPositions) {
worker.invoke(&GeometryTileWorker::onGlyphsAvailable, std::move(glyphPositions));
}
void GeometryTile::getGlyphs(GlyphDependencies glyphDependencies) {

2
src/mbgl/tile/geometry_tile.hpp
View File

@@ -40,7 +40,7 @@ public:
void setPlacementConfig(const PlacementConfig&) override;
void redoLayout() override;
void onGlyphsAvailable(GlyphPositionMap, GlyphRangeSet) override;
void onGlyphsAvailable(GlyphPositionMap) override;
void onIconsAvailable(SpriteAtlas*, IconMap) override;
void getGlyphs(GlyphDependencies);

35
src/mbgl/tile/geometry_tile_worker.cpp
View File

@@ -196,30 +196,21 @@ void GeometryTileWorker::coalesce() {
self.invoke(&GeometryTileWorker::coalesced);
}
void GeometryTileWorker::onGlyphsAvailable(GlyphPositionMap newGlyphPositions) {
for (auto& newFontGlyphs : newGlyphPositions) {
const FontStack& fontStack = newFontGlyphs.first;
GlyphPositions& newPositions = newFontGlyphs.second;
void GeometryTileWorker::onGlyphsAvailable(GlyphPositionMap newGlyphPositions, GlyphRangeSet loadedRanges) {
GlyphDependencies loadedGlyphs;
for (auto& pendingFontGlyphs : pendingGlyphDependencies) {
auto newFontGlyphs = newGlyphPositions.find(pendingFontGlyphs.first);
for (auto glyphID : pendingFontGlyphs.second) {
if (newFontGlyphs != newGlyphPositions.end()) {
auto newFontGlyph = newFontGlyphs->second.find(glyphID);
if (newFontGlyph != newFontGlyphs->second.end()) {
glyphPositions[pendingFontGlyphs.first].emplace(glyphID, newFontGlyph->second);
}
GlyphPositions& positions = glyphPositions[fontStack];
GlyphIDs& pendingGlyphIDs = pendingGlyphDependencies[fontStack];
for (auto& newPosition : newPositions) {
const GlyphID& glyphID = newPosition.first;
optional<Glyph>& glyph = newPosition.second;
if (pendingGlyphIDs.erase(glyphID)) {
positions.emplace(glyphID, std::move(glyph));
}
if (loadedRanges.find(getGlyphRange(glyphID)) != loadedRanges.end()) {
// Erase the glyph from our pending font set as long as its range is loaded
// If the glyph itself is missing, that means we can't get a glyph for
// this fontstack, and we go ahead and render with missing glyphs
loadedGlyphs[pendingFontGlyphs.first].insert(glyphID);
}
}
}
for (auto& loadedFont : loadedGlyphs) {
for (auto loadedGlyph : loadedFont.second) {
pendingGlyphDependencies[loadedFont.first].erase(loadedGlyph);
}
}
symbolDependenciesChanged();

2
src/mbgl/tile/geometry_tile_worker.hpp
View File

@@ -35,7 +35,7 @@ public:
void setData(std::unique_ptr<const GeometryTileData>, uint64_t correlationID);
void setPlacementConfig(PlacementConfig, uint64_t correlationID);
void onGlyphsAvailable(GlyphPositionMap glyphs, GlyphRangeSet loadedRanges);
void onGlyphsAvailable(GlyphPositionMap glyphs);
void onIconsAvailable(IconAtlasMap icons);
private:

32
test/text/glyph_atlas.test.cpp
View File

@@ -12,11 +12,11 @@ using namespace mbgl;
class StubGlyphRequestor : public GlyphRequestor {
public:
void onGlyphsAvailable(GlyphPositionMap positions, GlyphRangeSet ranges) override {
if (glyphsAvailable) glyphsAvailable(std::move(positions), std::move(ranges));
void onGlyphsAvailable(GlyphPositionMap positions) override {
if (glyphsAvailable) glyphsAvailable(std::move(positions));
}
std::function<void (GlyphPositionMap, GlyphRangeSet)> glyphsAvailable;
std::function<void (GlyphPositionMap)> glyphsAvailable;
};
class GlyphAtlasTest {
@@ -63,15 +63,14 @@ TEST(GlyphAtlas, LoadingSuccess) {
ASSERT_EQ(range, GlyphRange(0, 255));
};
test.requestor.glyphsAvailable = [&] (GlyphPositionMap positions, GlyphRangeSet ranges) {
test.requestor.glyphsAvailable = [&] (GlyphPositionMap positions) {
const auto& testPositions = positions.at({{"Test Stack"}});
ASSERT_EQ(testPositions.size(), 2u);
ASSERT_EQ(testPositions.size(), 3u);
ASSERT_EQ(testPositions.count(u'a'), 1u);
ASSERT_EQ(testPositions.count(u'å'), 1u);
ASSERT_EQ(ranges.size(), 1u);
ASSERT_EQ((ranges.count({0, 255})), 1u);
ASSERT_EQ(testPositions.count(u' '), 1u);
ASSERT_TRUE(bool(testPositions.at(u' ')));
test.end();
};
@@ -79,7 +78,7 @@ TEST(GlyphAtlas, LoadingSuccess) {
test.run(
"test/fixtures/resources/glyphs.pbf",
GlyphDependencies {
{{{"Test Stack"}}, {u'a', u'å'}}
{{{"Test Stack"}}, {u'a', u'å', u' '}}
});
}
@@ -104,7 +103,7 @@ TEST(GlyphAtlas, LoadingFail) {
test.end();
};
test.requestor.glyphsAvailable = [&] (GlyphPositionMap, GlyphRangeSet) {
test.requestor.glyphsAvailable = [&] (GlyphPositionMap) {
FAIL();
test.end();
};
@@ -135,7 +134,7 @@ TEST(GlyphAtlas, LoadingCorrupted) {
test.end();
};
test.requestor.glyphsAvailable = [&] (GlyphPositionMap, GlyphRangeSet) {
test.requestor.glyphsAvailable = [&] (GlyphPositionMap) {
FAIL();
test.end();
};
@@ -186,15 +185,12 @@ TEST(GlyphAtlas, LoadingInvalid) {
ASSERT_EQ(range, GlyphRange(0, 255));
};
test.requestor.glyphsAvailable = [&] (GlyphPositionMap positions, GlyphRangeSet ranges) {
test.requestor.glyphsAvailable = [&] (GlyphPositionMap positions) {
const auto& testPositions = positions.at({{"Test Stack"}});
ASSERT_EQ(testPositions.size(), 1u);
ASSERT_EQ(testPositions.count(u'A'), 0u);
ASSERT_EQ(testPositions.count(u'E'), 1u);
ASSERT_EQ(ranges.size(), 1u);
ASSERT_EQ((ranges.count({0, 255})), 1u);
ASSERT_EQ(testPositions.size(), 2u);
ASSERT_FALSE(bool(testPositions.at(u'A')));
ASSERT_TRUE(bool(testPositions.at(u'E')));
test.end();
};