suspected bug in landmark.cpp

[yan-lu]

Close to the end of the GraphManager::updateLandmarks(...) function definition in "landmark.cpp", the line "landmarks.erase(landmarks.begin()+lm_id_new)" seems to be a bug. This line tries to remove a landmark out of the vector "landmarks" since it has been merged to another landmark (i.e., lm_1). However, removing it from the vector will cause all elements following it to be shifted towards the vector head. As a result, their id's (id is a class field) would not correspond to their real position/index in the vector. Later on when querying a landmark by its id, e.g. landmarks[lm->id], the result will be wrong.

Aparently, all elements in vector "landmarks" should update their id's whenever they are shifted due to reasons like erasing some elements, though this would be kind of awkward. My thought is to avoid explicitly erasing a landmark from the vector; instead, add a bool field in the "Landmark" class, say "deleted", to indicated whether a landmark has been deleted or not.
Please correct me if I'm wrong.

[felixendres]

Hey, thanks for reporting this. It is code from Nikolas Engelhard though, which I am not using personally. IMHO performing bundle adjustment is more pain than gain and I would rather to keep my fingers off this code. I'd happily incorporate a pull request though.