Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add: ability to add/remove several coordinate systems #401

Merged
merged 3 commits into from
Jan 21, 2014
Merged

Add: ability to add/remove several coordinate systems #401

merged 3 commits into from
Jan 21, 2014

Conversation

nizar-sallem
Copy link
Contributor

This was a requested feature and bug fix on the mailing list since one could add several coordinate systems but remove only the last one.
I made it such as coordinate systems get a string id same way as for other shapes.

@nizar-sallem
Add: ability to add/remove several coordinate systems
7aa1640 
This was a requested feature on the mailing list
rbrusu
rbrusu reviewed Dec 16, 2013
View reviewed changes
visualization/include/pcl/visualization/pcl_visualizer.h
* \param[in] viewport the view port where the 3D axes should be added (default: all)
*/
void
addCoordinateSystem (double scale = 1.0, int viewport = 0);
addCoordinateSystem (double scale = 1.0, const std::string& id = "reference", int viewport = 0);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of breaking the API, we should add a new method with addCoordinateSystemWithReference or something.

@nizar-sallem
Copy link
Contributor Author

@rbrusu,
this doesn't break the API, since there is a default value for the string id so if user used the default viewport it doesn't break anything and if he set the viewport to something else we are safe too cause the id will be "reference" by default.
So on both sides we are safe. Also the change is to fix a bug, adding a feature is just a side effect. Thing is you could add several coordinate systems at different locations but delete only one (since we are using unordered maps).

@rbrusu
Copy link
Member

rbrusu commented Dec 17, 2013

:) It does break the API Nizar, because if I had a call like addCoordinateSystem (myScale, myViewPort), that will not work anymore.

@nizar-sallem
Copy link
Contributor Author

I just forgot to add the other functions redirection on the PR.
Will do it right now, Sorry.

@nizar-sallem
Copy link
Contributor Author

@rbrusu updated all occurrences and set previous version to deprecated

@nizar-sallem
Copy link
Contributor Author

@rbrusu is it ok now ?

jspricke added a commit that referenced this pull request Jan 21, 2014
@jspricke
Merge pull request #401 from nizar-sallem/several_coordinate_system
6bf4b81 
Add: ability to add/remove several coordinate systems
@jspricke jspricke merged commit 6bf4b81 into PointCloudLibrary:master Jan 21, 2014
@nizar-sallem nizar-sallem deleted the several_coordinate_system branch January 31, 2014 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nizar-sallem @rbrusu @jspricke