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

Commit

Permalink
[core] Safeguard PositionedIcon usage via optional
Browse files Browse the repository at this point in the history
  • Loading branch information
@brunoabinader @jfirebaugh
brunoabinader authored and jfirebaugh committed Apr 14, 2017
1 parent 2f4d162 commit 98e2e59
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 62 deletions.
9 changes: 2 additions & 7 deletions src/mbgl/layout/symbol_instance.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ using namespace style;
SymbolInstance::SymbolInstance(Anchor& anchor,
const GeometryCoordinates& line,
const std::pair<Shaping, Shaping>& shapedTextOrientations,
const PositionedIcon& shapedIcon,
optional<PositionedIcon> shapedIcon,
const SymbolLayoutProperties::Evaluated& layout,
const float layoutTextSize,
const bool addToBuffers,
Expand All @@ -27,18 +27,15 @@ SymbolInstance::SymbolInstance(Anchor& anchor,
hasText(shapedTextOrientations.first || shapedTextOrientations.second),
hasIcon(shapedIcon),


// Create the collision features that will be used to check whether this symbol instance can be placed
textCollisionFeature(line, anchor, shapedTextOrientations.second ?: shapedTextOrientations.first, textBoxScale, textPadding, textPlacement, indexedFeature),
iconCollisionFeature(line, anchor, shapedIcon, iconBoxScale, iconPadding, iconPlacement, indexedFeature),
featureIndex(featureIndex_) {



// Create the quads used for rendering the icon and glyphs.
if (addToBuffers) {
if (shapedIcon) {
iconQuad = getIconQuad(anchor, shapedIcon, line, layout, layoutTextSize, iconPlacement, shapedTextOrientations.first);
iconQuad = getIconQuad(anchor, *shapedIcon, line, layout, layoutTextSize, iconPlacement, shapedTextOrientations.first);
}
if (shapedTextOrientations.first) {
auto quads = getGlyphQuads(anchor, shapedTextOrientations.first, textBoxScale, line, layout, textPlacement, face);
Expand All @@ -59,8 +56,6 @@ SymbolInstance::SymbolInstance(Anchor& anchor,
} else {
writingModes = WritingModeType::None;
}


}

} // namespace mbgl
2 changes: 1 addition & 1 deletion src/mbgl/layout/symbol_instance.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class SymbolInstance {
SymbolInstance(Anchor& anchor,
const GeometryCoordinates& line,
const std::pair<Shaping, Shaping>& shapedTextOrientations,
const PositionedIcon& shapedIcon,
optional<PositionedIcon> shapedIcon,
const style::SymbolLayoutProperties::Evaluated&,
const float layoutTextSize,
const bool inside,
Expand Down
10 changes: 5 additions & 5 deletions src/mbgl/layout/symbol_layout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ void SymbolLayout::prepare(const GlyphPositionMap& glyphs, const IconAtlasMap& i
if (feature.geometry.empty()) continue;

std::pair<Shaping, Shaping> shapedTextOrientations;
PositionedIcon shapedIcon;
optional<PositionedIcon> shapedIcon;
GlyphPositions face;

// if feature has text, shape the text
Expand Down Expand Up @@ -262,7 +262,7 @@ void SymbolLayout::prepare(const GlyphPositionMap& glyphs, const IconAtlasMap& i
if (icons != iconMap.end()) {
auto image = icons->second.find(*feature.icon);
if (image != icons->second.end()) {
shapedIcon = shapeIcon(image->second,
shapedIcon = PositionedIcon::shapeIcon(image->second,
layout.evaluate<IconOffset>(zoom, feature),
layout.evaluate<IconRotate>(zoom, feature) * util::DEG2RAD);
if (image->second.sdf) {
Expand Down Expand Up @@ -292,7 +292,7 @@ void SymbolLayout::prepare(const GlyphPositionMap& glyphs, const IconAtlasMap& i
void SymbolLayout::addFeature(const std::size_t index,
const SymbolFeature& feature,
const std::pair<Shaping, Shaping>& shapedTextOrientations,
const PositionedIcon& shapedIcon,
optional<PositionedIcon> shapedIcon,
const GlyphPositions& glyphs) {
const float minScale = 0.5f;
const float glyphSize = 24.0f;
Expand Down Expand Up @@ -363,8 +363,8 @@ void SymbolLayout::addFeature(const std::size_t index,
textMaxAngle,
(shapedTextOrientations.second ?: shapedTextOrientations.first).left,
(shapedTextOrientations.second ?: shapedTextOrientations.first).right,
shapedIcon.left,
shapedIcon.right,
(shapedIcon ? shapedIcon->left() : 0),
(shapedIcon ? shapedIcon->right() : 0),
glyphSize,
textMaxBoxScale,
overscaling);
Expand Down
2 changes: 1 addition & 1 deletion src/mbgl/layout/symbol_layout.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class SymbolLayout {
void addFeature(const size_t,
const SymbolFeature&,
const std::pair<Shaping, Shaping>& shapedTextOrientations,
const PositionedIcon& shapedIcon,
optional<PositionedIcon> shapedIcon,
const GlyphPositions& face);

bool anchorIsTooClose(const std::u16string& text, const float repeatDistance, const Anchor&);
Expand Down
4 changes: 2 additions & 2 deletions src/mbgl/text/collision_feature.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ CollisionFeature::CollisionFeature(const GeometryCoordinates& line,
const float padding,
const style::SymbolPlacementType placement,
IndexedSubfeature indexedFeature_,
const bool straight)
const AlignmentType alignment)
: indexedFeature(std::move(indexedFeature_)) {
if (top == 0 && bottom == 0 && left == 0 && right == 0) return;

Expand All @@ -32,7 +32,7 @@ CollisionFeature::CollisionFeature(const GeometryCoordinates& line,

GeometryCoordinate anchorPoint = convertPoint<int16_t>(anchor.point);

if (straight) {
if (alignment == AlignmentType::Straight) {
// used for icon labels that are aligned with the line, but don't curve along it
const GeometryCoordinate vector = convertPoint<int16_t>(util::unit(convertPoint<double>(line[anchor.segment + 1] - line[anchor.segment])) * length);
const GeometryCoordinates newLine({ anchorPoint - vector, anchorPoint + vector });
Expand Down
22 changes: 16 additions & 6 deletions src/mbgl/text/collision_feature.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ class CollisionBox {

class CollisionFeature {
public:
enum class AlignmentType : bool {
Straight = 0,
Curved
};

// for text
CollisionFeature(const GeometryCoordinates& line,
const Anchor& anchor,
Expand All @@ -41,29 +46,34 @@ class CollisionFeature {
const float padding,
const style::SymbolPlacementType placement,
const IndexedSubfeature& indexedFeature_)
: CollisionFeature(line, anchor, shapedText.top, shapedText.bottom, shapedText.left, shapedText.right, boxScale, padding, placement, indexedFeature_, false) {}
: CollisionFeature(line, anchor, shapedText.top, shapedText.bottom, shapedText.left, shapedText.right, boxScale, padding, placement, indexedFeature_, AlignmentType::Curved) {}

// for icons
CollisionFeature(const GeometryCoordinates& line,
const Anchor& anchor,
const PositionedIcon& shapedIcon,
optional<PositionedIcon> shapedIcon,
const float boxScale,
const float padding,
const style::SymbolPlacementType placement,
const IndexedSubfeature& indexedFeature_)
: CollisionFeature(line, anchor, shapedIcon.top, shapedIcon.bottom, shapedIcon.left, shapedIcon.right, boxScale, padding, placement, indexedFeature_, true) {}
: CollisionFeature(line, anchor,
(shapedIcon ? shapedIcon->top() : 0),
(shapedIcon ? shapedIcon->bottom() : 0),
(shapedIcon ? shapedIcon->left() : 0),
(shapedIcon ? shapedIcon->right() : 0),
boxScale, padding, placement, indexedFeature_, AlignmentType::Straight) {}

CollisionFeature(const GeometryCoordinates& line,
const Anchor& anchor,
const Anchor&,
const float top,
const float bottom,
const float left,
const float right,
const float boxScale,
const float padding,
const style::SymbolPlacementType placement,
const style::SymbolPlacementType,
IndexedSubfeature,
const bool straight);
const AlignmentType);

std::vector<CollisionBox> boxes;
IndexedSubfeature indexedFeature;
Expand Down
8 changes: 4 additions & 4 deletions src/mbgl/text/quads.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ SymbolQuad getIconQuad(const Anchor& anchor,
const float layoutTextSize,
const style::SymbolPlacementType placement,
const Shaping& shapedText) {
auto image = *(shapedIcon.image);
auto image = *shapedIcon.image();

const float border = 1.0;
auto left = shapedIcon.left - border;
auto left = shapedIcon.left() - border;
auto right = left + image.pos.w / image.relativePixelRatio;
auto top = shapedIcon.top - border;
auto top = shapedIcon.top() - border;
auto bottom = top + image.pos.h / image.relativePixelRatio;
Point<float> tl;
Point<float> tr;
Expand Down Expand Up @@ -67,7 +67,7 @@ SymbolQuad getIconQuad(const Anchor& anchor,
bl = {left, bottom};
}

float angle = shapedIcon.angle;
float angle = shapedIcon.angle();
if (placement == style::SymbolPlacementType::Line) {
assert(static_cast<unsigned int>(anchor.segment) < line.size());
const GeometryCoordinate &prev= line[anchor.segment];
Expand Down
8 changes: 6 additions & 2 deletions src/mbgl/text/shaping.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,19 @@

namespace mbgl {

PositionedIcon shapeIcon(const SpriteAtlasElement& image, const std::array<float, 2>& iconOffset, const float iconRotation) {
optional<PositionedIcon> PositionedIcon::shapeIcon(const SpriteAtlasElement& image, const std::array<float, 2>& iconOffset, const float iconRotation) {
if (!image.pos.hasArea()) {
return {};
}

float dx = iconOffset[0];
float dy = iconOffset[1];
float x1 = dx - image.width/ 2.0f;
float x2 = x1 + image.width;
float y1 = dy - image.height / 2.0f;
float y2 = y1 + image.height;

return PositionedIcon(image, y1, y2, x1, x2, iconRotation);
return { PositionedIcon { image, y1, y2, x1, x2, iconRotation } };
}

void align(Shaping& shaping,
Expand Down
43 changes: 24 additions & 19 deletions src/mbgl/text/shaping.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,33 +12,38 @@ class SymbolFeature;
class BiDi;

class PositionedIcon {
public:
PositionedIcon() = default;
private:
PositionedIcon(const SpriteAtlasElement& image_,
float top_,
float bottom_,
float left_,
float right_,
float angle_)
: image(image_),
top(top_),
bottom(bottom_),
left(left_),
right(right_),
angle(angle_) {}

optional<SpriteAtlasElement> image;
float top = 0;
float bottom = 0;
float left = 0;
float right = 0;
float angle = 0;

explicit operator bool() const { return image && (*image).pos.hasArea(); }
: _image(image_),
_top(top_),
_bottom(bottom_),
_left(left_),
_right(right_),
_angle(angle_) {}

optional<SpriteAtlasElement> _image;
float _top;
float _bottom;
float _left;
float _right;
float _angle;

public:
static optional<PositionedIcon> shapeIcon(const class SpriteAtlasElement&, const std::array<float, 2>& iconOffset, const float iconRotation);

optional<class SpriteAtlasElement> image() const { return _image; }
float top() const { return _top; }
float bottom() const { return _bottom; }
float left() const { return _left; }
float right() const { return _right; }
float angle() const { return _angle; }
};

PositionedIcon shapeIcon(const SpriteAtlasElement&, const std::array<float, 2>& iconOffset, const float iconRotation);

const Shaping getShaping(const std::u16string& string,
float maxWidth,
float lineHeight,
Expand Down
34 changes: 20 additions & 14 deletions test/text/quads.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@ TEST(getIconQuads, normal) {
{ 0, 0 },
1.0f
};
PositionedIcon shapedIcon(image, -5.0, 6.0, -7.0, 8.0, 0);

auto shapedIcon = PositionedIcon::shapeIcon(image, {{ -6.5f, -4.5f }}, 0);
ASSERT_TRUE(shapedIcon);

GeometryCoordinates line;
Shaping shapedText;

SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 16.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 16.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.anchorPoint.x, 2);
ASSERT_EQ(quad.anchorPoint.y, 3);
Expand All @@ -48,7 +51,10 @@ TEST(getIconQuads, style) {
{ 0, 0 },
1.0f
};
PositionedIcon shapedIcon(image, -10.0, 10.0, -10.0, 10.0, 0);

auto shapedIcon = PositionedIcon::shapeIcon(image, {{ -9.5f, -9.5f }}, 0);
ASSERT_TRUE(shapedIcon);

GeometryCoordinates line;
Shaping shapedText;
shapedText.top = -10.0f;
Expand All @@ -61,7 +67,7 @@ TEST(getIconQuads, style) {
{
SymbolLayoutProperties::Evaluated layout;
SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.anchorPoint.x, 0);
ASSERT_EQ(quad.anchorPoint.y, 0);
Expand All @@ -84,7 +90,7 @@ TEST(getIconQuads, style) {
layout.get<TextSize>() = 24.0f;
layout.get<IconTextFit>() = IconTextFitType::Width;
SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 24.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 24.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.tl.x, -60);
ASSERT_EQ(quad.tl.y, 0);
Expand All @@ -102,7 +108,7 @@ TEST(getIconQuads, style) {
layout.get<TextSize>() = 12.0f;
layout.get<IconTextFit>() = IconTextFitType::Width;
SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.tl.x, -30);
ASSERT_EQ(quad.tl.y, -5);
Expand All @@ -124,7 +130,7 @@ TEST(getIconQuads, style) {
layout.get<IconTextFitPadding>()[2] = 5.0f;
layout.get<IconTextFitPadding>()[3] = 10.0f;
SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.tl.x, -40);
ASSERT_EQ(quad.tl.y, -10);
Expand All @@ -142,7 +148,7 @@ TEST(getIconQuads, style) {
layout.get<TextSize>() = 24.0f;
layout.get<IconTextFit>() = IconTextFitType::Height;
SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 24.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 24.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.tl.x, -30);
ASSERT_EQ(quad.tl.y, -10);
Expand All @@ -160,7 +166,7 @@ TEST(getIconQuads, style) {
layout.get<TextSize>() = 12.0f;
layout.get<IconTextFit>() = IconTextFitType::Height;
SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.tl.x, -20);
ASSERT_EQ(quad.tl.y, -5);
Expand All @@ -182,7 +188,7 @@ TEST(getIconQuads, style) {
layout.get<IconTextFitPadding>()[2] = 5.0f;
layout.get<IconTextFitPadding>()[3] = 10.0f;
SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.tl.x, -30);
ASSERT_EQ(quad.tl.y, -10);
Expand All @@ -200,7 +206,7 @@ TEST(getIconQuads, style) {
layout.get<TextSize>() = 24.0f;
layout.get<IconTextFit>() = IconTextFitType::Both;
SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 24.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 24.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.tl.x, -60);
ASSERT_EQ(quad.tl.y, -10);
Expand All @@ -218,7 +224,7 @@ TEST(getIconQuads, style) {
layout.get<TextSize>() = 12.0f;
layout.get<IconTextFit>() = IconTextFitType::Both;
SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.tl.x, -30);
ASSERT_EQ(quad.tl.y, -5);
Expand All @@ -240,7 +246,7 @@ TEST(getIconQuads, style) {
layout.get<IconTextFitPadding>()[2] = 5.0f;
layout.get<IconTextFitPadding>()[3] = 10.0f;
SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.tl.x, -40);
ASSERT_EQ(quad.tl.y, -10);
Expand All @@ -262,7 +268,7 @@ TEST(getIconQuads, style) {
layout.get<IconTextFitPadding>()[2] = 10.0f;
layout.get<IconTextFitPadding>()[3] = 15.0f;
SymbolQuad quad =
getIconQuad(anchor, shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);
getIconQuad(anchor, *shapedIcon, line, layout, 12.0f, SymbolPlacementType::Point, shapedText);

ASSERT_EQ(quad.tl.x, -45);
ASSERT_EQ(quad.tl.y, -5);
Expand Down
Loading

0 comments on commit 98e2e59

Please sign in to comment.