RedAuburn/organicmaps
1
Fork 0
forked from organicmaps/organicmaps
Code Pull requests Activity

Fixed bookmarks order: least recently added bookmark is displayed first in the list.

Browse source
This commit is contained in:
Yuri Gorshenin 2015-02-04 11:24:30 +03:00 • committed by Alex Zolotarev
parent 7fd7b95f35
commit 994873c081
8 changed files with 86 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
map/bookmark.cpp
View file

@ -63,10 +63,11 @@ Track const * BookmarkCategory::GetTrack(size_t index) const
return (index < m_tracks.size() ? m_tracks[index] : 0);
}
void BookmarkCategory::AddBookmark(m2::PointD const & ptOrg, BookmarkData const & bm)
Bookmark * BookmarkCategory::AddBookmark(m2::PointD const & ptOrg, BookmarkData const & bm)
{
UserMark * mark = base_t::GetController().CreateUserMark(ptOrg);
static_cast<Bookmark *>(mark)->SetData(bm);
Bookmark * bookmark = static_cast<Bookmark *>(base_t::GetController().CreateUserMark(ptOrg));
bookmark->SetData(bm);
return bookmark;
}
void BookmarkCategory::ReplaceBookmark(size_t index, BookmarkData const & bm)
@ -168,6 +169,11 @@ Bookmark * BookmarkCategory::GetBookmark(size_t index)
return static_cast<Bookmark *>(index < c.GetUserMarkCount() ? c.GetUserMarkForEdit(index) : 0);
}
size_t BookmarkCategory::FindBookmark(Bookmark const * bookmark) const
{
return base_t::GetController().FindUserMark(bookmark);
}
namespace
{
@ -703,7 +709,21 @@ void BookmarkCategory::SaveToKML(ostream & s)
s << " <visibility>" << (IsVisible() ? "1" : "0") <<"</visibility>\n";
for (size_t i = 0; i < GetBookmarksCount(); ++i)
// Bookmarks are stored to KML file in reverse order, so, least
// recently added bookmark will be stored last. The reason is that
// when bookmarks will be loaded from the KML file, most recently
// added bookmark will be loaded last and in accordance with current
// logic will added to the beginning of the bookmarks list. Thus,
// this method preserves LRU bookmarks order after store -> load
// actions.
//
// Loop invariant: on each iteration count means number of already
// stored bookmarks and i means index of the bookmark that should be
// processed during the iteration. That's why i is initially set to
// GetBookmarksCount() - 1, i.e. to the last bookmark in the
// bookmarks list.
for (size_t count = 0, i = GetBookmarksCount() - 1;
count < GetBookmarksCount(); ++count, --i)
{
Bookmark const * bm = GetBookmark(i);
s << " <Placemark>\n";

6
map/bookmark.hpp
View file

@ -142,7 +142,7 @@ public:
/// @name Theese functions are called from Framework only.
//@{
void AddBookmark(m2::PointD const & ptOrg, BookmarkData const & bm);
Bookmark* AddBookmark(m2::PointD const & ptOrg, BookmarkData const & bm);
void ReplaceBookmark(size_t index, BookmarkData const & bm);
//@}
@ -165,6 +165,10 @@ public:
Bookmark * GetBookmark(size_t index);
void DeleteBookmark(size_t index);
// Returns index of the bookmark if exists, otherwise returns
// total number of bookmarks.
size_t FindBookmark(Bookmark const * bookmark) const;
/// @name Theese fuctions are public for unit tests only.
/// You don't need to call them from client code.
//@{

4
map/bookmark_manager.cpp
View file

@ -184,7 +184,7 @@ size_t BookmarkManager::AddBookmark(size_t categoryIndex, const m2::PointD & ptO
bm.SetScale(m_framework.GetDrawScale());
BookmarkCategory * pCat = m_categories[categoryIndex];
pCat->AddBookmark(ptOrg, bm);
Bookmark * bookmark = pCat->AddBookmark(ptOrg, bm);
pCat->SetVisible(true);
pCat->SaveToKMLFile();
@ -192,7 +192,7 @@ size_t BookmarkManager::AddBookmark(size_t categoryIndex, const m2::PointD & ptO
m_lastType = bm.GetType();
SaveState();
return (pCat->GetBookmarksCount() - 1);
return pCat->FindBookmark(bookmark);
}
size_t BookmarkManager::MoveBookmark(size_t bmIndex, size_t curCatIndex, size_t newCatIndex)

32
map/map_tests/bookmarks_test.cpp
View file

@ -120,19 +120,19 @@ char const * kmlString =
{
TEST_EQUAL(cat.GetBookmarksCount(), 4, ());
Bookmark const * bm = cat.GetBookmark(0);
Bookmark const * bm = cat.GetBookmark(3);
TEST_EQUAL(bm->GetName(), "Nebraska", ());
TEST_EQUAL(bm->GetType(), "placemark-red", ());
TEST_EQUAL(bm->GetDescription(), "", ());
TEST_EQUAL(bm->GetTimeStamp(), my::INVALID_TIME_STAMP, ());
bm = cat.GetBookmark(1);
bm = cat.GetBookmark(2);
TEST_EQUAL(bm->GetName(), "Monongahela National Forest", ());
TEST_EQUAL(bm->GetType(), "placemark-pink", ());
TEST_EQUAL(bm->GetDescription(), "Huttonsville, WV 26273<br>", ());
TEST_EQUAL(bm->GetTimeStamp(), 524214643, ());
bm = cat.GetBookmark(2);
bm = cat.GetBookmark(1);
m2::PointD org = bm->GetOrg();
TEST_ALMOST_EQUAL(MercatorBounds::XToLon(org.x), 27.566765, ());
TEST_ALMOST_EQUAL(MercatorBounds::YToLat(org.y), 53.900047, ());
@ -141,7 +141,7 @@ char const * kmlString =
TEST_EQUAL(bm->GetDescription(), "", ());
TEST_EQUAL(bm->GetTimeStamp(), 888888888, ());
bm = cat.GetBookmark(3);
bm = cat.GetBookmark(0);
org = bm->GetOrg();
TEST_ALMOST_EQUAL(MercatorBounds::XToLon(org.x), 27.551532, ());
TEST_ALMOST_EQUAL(MercatorBounds::YToLat(org.y), 53.89306, ());
@ -286,11 +286,11 @@ UNIT_TEST(Bookmarks_Timestamp)
fm.AddCategory(arrCat[1]);
fm.AddBookmark(1, orgPoint, b3);
TEST_EQUAL(fm.GetBmCategory(0)->GetBookmark(0)->GetName(), "name", ());
TEST_EQUAL(fm.GetBmCategory(0)->GetBookmark(0)->GetType(), "type", ());
TEST_EQUAL(fm.GetBmCategory(0)->GetBookmark(1)->GetName(), "name", ());
TEST_EQUAL(fm.GetBmCategory(0)->GetBookmark(1)->GetType(), "type", ());
TEST_EQUAL(fm.GetBmCategory(0)->GetBookmark(1)->GetName(), "newName", ());
TEST_EQUAL(fm.GetBmCategory(0)->GetBookmark(1)->GetType(), "newType", ());
TEST_EQUAL(fm.GetBmCategory(0)->GetBookmark(0)->GetName(), "newName", ());
TEST_EQUAL(fm.GetBmCategory(0)->GetBookmark(0)->GetType(), "newType", ());
TEST_EQUAL(fm.GetBmCategory(1)->GetBookmark(0)->GetName(), "newName", ());
TEST_EQUAL(fm.GetBmCategory(1)->GetBookmark(0)->GetType(), "newType", ());
@ -356,9 +356,11 @@ UNIT_TEST(Bookmarks_Getting)
mark = GetBookmark(fm, m2::PointD(41, 40));
cat = GetCategory(mark);
//should find first valid result, there two results with the same coordinates 3 and 4
TEST_EQUAL(mark->GetName(), "3", ());
TEST_EQUAL(mark->GetType(), "placemark-red", ());
// Should find last added valid result, there two results with the
// same coordinates 3 and 4, but 4 was added later.
TEST_EQUAL(mark->GetName(), "4", ());
TEST_EQUAL(mark->GetType(), "placemark-blue", ());
TEST_EQUAL(cat->GetBookmarksCount(), 2, ());
@ -504,8 +506,8 @@ UNIT_TEST(Bookmarks_AddingMoving)
mark = GetBookmarkPxPoint(fm, pixelPoint);
cat = GetCategory(mark);
TEST_EQUAL(cat->GetName(), arrCat[0], ());
TEST_EQUAL(mark->GetName(), "name", ());
TEST_EQUAL(mark->GetType(), "placemark-red", ());
TEST_EQUAL(mark->GetName(), "name2", ());
TEST_EQUAL(mark->GetType(), "placemark-blue", ());
// Edit name, type and category of bookmark
bm = BookmarkData("name3", "placemark-green");
@ -518,8 +520,8 @@ UNIT_TEST(Bookmarks_AddingMoving)
TEST_EQUAL(cat->GetName(), arrCat[0], ());
TEST_EQUAL(fm.GetBmCategory(0)->GetBookmarksCount(), 2,
("Bookmark wasn't moved from one category to another"));
TEST_EQUAL(mark->GetName(), "name", ());
TEST_EQUAL(mark->GetType(), "placemark-red", ());
TEST_EQUAL(mark->GetName(), "name2", ());
TEST_EQUAL(mark->GetType(), "placemark-blue", ());
DeleteCategoryFiles(arrCat);
}

4
map/map_tests/kmz_unarchive_test.cpp
View file

@ -40,7 +40,7 @@ UNIT_TEST(Open_KMZ_Test)
TEST_EQUAL(cat.GetBookmarksCount(), 6, ("Category wrong number of bookmarks"));
{
Bookmark const * bm = cat.GetBookmark(0);
Bookmark const * bm = cat.GetBookmark(5);
TEST_EQUAL(bm->GetName(), ("Lahaina Breakwall"), ("KML wrong name!"));
TEST_EQUAL(bm->GetType(), "placemark-red", ("KML wrong type!"));
TEST_ALMOST_EQUAL(bm->GetOrg().x, -156.6777046791284, ("KML wrong org x!"));
@ -48,7 +48,7 @@ UNIT_TEST(Open_KMZ_Test)
TEST_EQUAL(bm->GetScale(), -1, ("KML wrong scale!"));
}
{
Bookmark const * bm = cat.GetBookmark(1);
Bookmark const * bm = cat.GetBookmark(4);
TEST_EQUAL(bm->GetName(), ("Seven Sacred Pools, Kipahulu"), ("KML wrong name!"));
TEST_EQUAL(bm->GetType(), "placemark-red", ("KML wrong type!"));
TEST_ALMOST_EQUAL(bm->GetOrg().x, -156.0405130750025, ("KML wrong org x!"));

15
map/map_tests/mwm_url_tests.cpp
View file

@ -142,12 +142,12 @@ UNIT_TEST(MapApiMultiplePoints)
ApiTest api("mwm://map?ll=1.1,1.2&n=A&LL=2.1,2.2&ll=-3.1,-3.2&n=C");
TEST(api.IsValid(), ());
TEST_EQUAL(api.GetPointCount(), 3, ());
TEST(api.TestLatLon(0, 1.1, 1.2), ());
TEST(api.TestName(0, "A"), ());
TEST(api.TestLatLon(2, 1.1, 1.2), ());
TEST(api.TestName(2, "A"), ());
TEST(api.TestLatLon(1, 2.1, 2.2), ());
TEST(api.TestName(1, ""), ());
TEST(api.TestLatLon(2, -3.1, -3.2), ());
TEST(api.TestName(2, "C"), ());
TEST(api.TestLatLon(0, -3.1, -3.2), ());
TEST(api.TestName(0, "C"), ());
}
UNIT_TEST(MapApiInvalidPointLatLonButValidOtherParts)
@ -298,9 +298,10 @@ void generateRandomTest(size_t numberOfPoints, size_t stringLength)
double lat = vect[i].m_lat;
double lon = vect[i].m_lon;
ToMercatoToLatLon(lat, lon);
TEST(api.TestLatLon(i, lat, lon), ());
TEST(api.TestName(i, vect[i].m_name), ());
TEST(api.TestID(i, vect[i].m_id), ());
size_t const ix = vect.size() - i - 1;
TEST(api.TestLatLon(ix, lat, lon), ());
TEST(api.TestName(ix, vect[i].m_name), ());
TEST(api.TestID(ix, vect[i].m_id), ());
}
TEST_EQUAL(api.GetApiVersion(), 1, ());
}

41
map/user_mark_container.cpp
View file

@ -202,8 +202,9 @@ MyPositionMarkPoint * UserMarkContainer::UserMarkForMyPostion()
UserMark * UserMarkContainer::CreateUserMark(m2::PointD const & ptOrg)
{
m_userMarks.push_back(AllocateUserMark(ptOrg));
return m_userMarks.back();
unique_ptr<UserMark> mark(AllocateUserMark(ptOrg));
m_userMarks.push_front(mark.get());
return mark.release();
}
size_t UserMarkContainer::GetUserMarkCount() const
@ -223,37 +224,33 @@ UserMark * UserMarkContainer::GetUserMark(size_t index)
return m_userMarks[index];
}
namespace
{
template <class T> void DeleteItem(vector<T> & v, size_t i)
{
if (i < v.size())
{
delete v[i];
v.erase(v.begin() + i);
}
else
{
LOG(LWARNING, ("Trying to delete non-existing item at index", i));
}
}
}
void UserMarkContainer::DeleteUserMark(size_t index)
{
ASSERT_LESS(index, m_userMarks.size(), ());
DeleteItem(m_userMarks, index);
if (index < m_userMarks.size())
{
delete m_userMarks[index];
m_userMarks.erase(m_userMarks.begin() + index);
}
else
{
LOG(LWARNING, ("Trying to delete non-existing item at index", index));
}
}
void UserMarkContainer::DeleteUserMark(UserMark const * mark)
{
vector<UserMark *>::iterator it = find(m_userMarks.begin(), m_userMarks.end(), mark);
UserMarksListT::iterator it = find(m_userMarks.begin(), m_userMarks.end(), mark);
if (it != m_userMarks.end())
DeleteUserMark(distance(m_userMarks.begin(), it));
}
size_t UserMarkContainer::FindUserMark(UserMark const * mark)
{
return distance(m_userMarks.begin(),
find(m_userMarks.begin(), m_userMarks.end(), mark));
}
SearchUserMarkContainer::SearchUserMarkContainer(double layerDepth, Framework & framework)
: UserMarkContainer(layerDepth, framework)
{

10
map/user_mark_container.hpp
View file

@ -7,6 +7,7 @@
#include "../geometry/point2d.hpp"
#include "../geometry/rect2d.hpp"
#include "../std/deque.hpp"
#include "../std/noncopyable.hpp"
class Framework;
@ -25,6 +26,8 @@ namespace graphics
class UserMarkContainer : private noncopyable
{
public:
using UserMarksListT = deque<UserMark *>;
class Controller
{
public:
@ -38,6 +41,10 @@ public:
void DeleteUserMark(size_t index) { m_container->DeleteUserMark(index); }
void DeleteUserMark(UserMark const * mark) { m_container->DeleteUserMark(mark); }
// Returns index of the mark if exists, otherwise returns
// number of user marks.
size_t FindUserMark(UserMark const * mark) const { return m_container->FindUserMark(mark); }
private:
UserMarkContainer * m_container;
};
@ -95,6 +102,7 @@ private:
UserMark * GetUserMark(size_t index);
void DeleteUserMark(size_t index);
void DeleteUserMark(UserMark const * mark);
size_t FindUserMark(UserMark const * mark);
template <class ToDo> void ForEachInRect(m2::RectD const & rect, ToDo toDo) const;
@ -106,7 +114,7 @@ private:
bool m_isVisible;
bool m_isDrawable;
double m_layerDepth;
vector<UserMark *> m_userMarks;
UserMarksListT m_userMarks;
};
class SearchUserMarkContainer : public UserMarkContainer