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

Fix annotation icon replacement #3320

Merged
merged 1 commit into from
Dec 16, 2015
Merged

Fix annotation icon replacement #3320

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/map/map.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ void Map::addAnnotationIcon(const std::string& name, std::shared_ptr<const Sprit
}

void Map::removeAnnotationIcon(const std::string& name) {
removeAnnotationIcon(name);
context->invoke(&MapContext::removeAnnotationIcon, name);
}

double Map::getTopOffsetPixelsForAnnotationIcon(const std::string& symbol) {
Expand Down
5 changes: 2 additions & 3 deletions src/mbgl/sprite/sprite_atlas.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,10 @@ void SpriteAtlas::updateDirty() {
holder.texture = spriteIterator->second;
if (holder.texture != nullptr) {
copy(holder, imageIterator->first.second);
++imageIterator;
} else {
images.erase(imageIterator);
images.erase(imageIterator++);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be prefix increment -- images.erase(++imageIterator). (For anyone who doesn't know: the reason is that std::map::erase invalidates any iterators pointing to the erased element. Prefix ++ both returns the original value for passing to erase, and increments just before the call. This is both very subtle and a standard C++ idiom in this situation.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And of course, after writing that I realized I had it backwards. The current code is correct!

}

++imageIterator;
// Don't advance the spriteIterator because there might be another sprite with the same
// name, but a different wrap value.
}
Expand Down
31 changes: 25 additions & 6 deletions test/api/annotations.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

using namespace mbgl;

std::shared_ptr<SpriteImage> defaultMarker() {
PremultipliedImage image = decodeImage(util::read_file("test/fixtures/sprites/default_marker.png"));
std::shared_ptr<SpriteImage> namedMarker(const std::string &name) {
PremultipliedImage image = decodeImage(util::read_file("test/fixtures/sprites/" + name));
return std::make_shared<SpriteImage>(image.width, image.height, 1.0, std::string(reinterpret_cast<char*>(image.data.get()), image.size()));
}

Expand All @@ -31,7 +31,7 @@ TEST(Annotations, PointAnnotation) {

Map map(view, fileSource, MapMode::Still);
map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"), "");
map.addAnnotationIcon("default_marker", defaultMarker());
map.addAnnotationIcon("default_marker", namedMarker("default_marker.png"));
map.addPointAnnotation(PointAnnotation({ 0, 0 }, "default_marker"));

checkRendering(map, "point_annotation");
Expand Down Expand Up @@ -96,7 +96,7 @@ TEST(Annotations, AddMultiple) {

Map map(view, fileSource, MapMode::Still);
map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"), "");
map.addAnnotationIcon("default_marker", defaultMarker());
map.addAnnotationIcon("default_marker", namedMarker("default_marker.png"));
map.addPointAnnotation(PointAnnotation({ 0, -10 }, "default_marker"));

test::render(map);
Expand Down Expand Up @@ -126,14 +126,33 @@ TEST(Annotations, NonImmediateAdd) {
checkRendering(map, "non_immediate_add");
}

TEST(Annotations, UpdatePoint) {
auto display = std::make_shared<mbgl::HeadlessDisplay>();
HeadlessView view(display, 1);
DefaultFileSource fileSource(nullptr, test::getFileSourceRoot());

Map map(view, fileSource, MapMode::Still);
map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"), "");
map.addAnnotationIcon("flipped_marker", namedMarker("default_marker.png"));
map.addPointAnnotation(PointAnnotation({ 0, 0 }, "flipped_marker"));

test::render(map);

map.removeAnnotationIcon("flipped_marker");
map.addAnnotationIcon("flipped_marker", namedMarker("flipped_marker.png"));
map.update(Update::Annotations);

checkRendering(map, "update_point");
}

TEST(Annotations, RemovePoint) {
auto display = std::make_shared<mbgl::HeadlessDisplay>();
HeadlessView view(display, 1);
DefaultFileSource fileSource(nullptr, test::getFileSourceRoot());

Map map(view, fileSource, MapMode::Still);
map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"), "");
map.addAnnotationIcon("default_marker", defaultMarker());
map.addAnnotationIcon("default_marker", namedMarker("default_marker.png"));
uint32_t point = map.addPointAnnotation(PointAnnotation({ 0, 0 }, "default_marker"));

test::render(map);
Expand Down Expand Up @@ -184,7 +203,7 @@ TEST(Annotations, SwitchStyle) {

Map map(view, fileSource, MapMode::Still);
map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"), "");
map.addAnnotationIcon("default_marker", defaultMarker());
map.addAnnotationIcon("default_marker", namedMarker("default_marker.png"));
map.addPointAnnotation(PointAnnotation({ 0, 0 }, "default_marker"));

test::render(map);
Expand Down
Binary file added test/fixtures/annotations/update_point/expected.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/fixtures/sprites/flipped_marker.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.