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

Adding default point color to SfmTrack to fix equality check #659

Merged
merged 15 commits into from
Jan 7, 2021
Merged

Adding default point color to SfmTrack to fix equality check #659

Changes from 13 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
b5db391
adding serialization and other functions to enable testing
ayushbaid Jan 1, 2021
df4419b
adding track serialization and testable trait
ayushbaid Jan 2, 2021
9d61e5b
improving formatting
ayushbaid Jan 2, 2021
f88b456
fixing variable names and comments
ayushbaid Jan 2, 2021
c46e3db
adding serialization functions to wrapper
ayushbaid Jan 2, 2021
e0cda60
fixing xml serialization issues
ayushbaid Jan 3, 2021
2e86921
reverting SfmTrack to struct
ayushbaid Jan 3, 2021
0ce1db2
printing out the 3d point
ayushbaid Jan 3, 2021
915b439
adding equals function to wrapper
ayushbaid Jan 4, 2021
1d41754
adding inline comment for round trip
ayushbaid Jan 4, 2021
7ae050c
adding default color values to fix equality check
ayushbaid Jan 6, 2021
d403006
Merge branch 'develop' into fix/sfmtrack-color
ayushbaid Jan 6, 2021
ecb2226
make r,g,b part of constructor w/ default values
johnwlambert Jan 6, 2021
758ece7
add getter for rgb
johnwlambert Jan 6, 2021
7daf01b
make getter const
johnwlambert Jan 6, 2021
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
5 changes: 3 additions & 2 deletions gtsam/slam/dataset.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,9 @@ typedef std::pair<size_t, size_t> SiftIndex;

/// Define the structure for the 3D points
struct SfmTrack {
SfmTrack(): p(0,0,0) {}
SfmTrack(const gtsam::Point3& pt) : p(pt) {}
SfmTrack(float r = 0, float g = 0, float b = 0): p(0,0,0), r(r), g(g), b(b) {}
SfmTrack(const gtsam::Point3& pt, float r = 0, float g = 0, float b = 0) : p(pt), r(r), g(g), b(b) {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Formatting please. Also, a good convention is to have struct/class variables be something like r_ and then have the initializer list be r_(r). This is consistent with Google C++ style which we follow.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the style suggestion, but this would involve renaming all of the variables in the class, since none adhere to that guideline. @dellaert would you like to see a big change like that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would vote for it since due to the previous lack of getters, it won't be an API change, so we should be able to maintain backwards compatibility.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, for classes the convention is underscore, for structs it is not (and should not).
It is an API change as these fields are public.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah you're right. My mistake.


Point3 p; ///< 3D position of the point
float r, g, b; ///< RGB color of the 3D point
johnwlambert marked this conversation as resolved.
Show resolved Hide resolved
std::vector<SfmMeasurement> measurements; ///< The 2D image projections (id,(u,v))
Expand Down