Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
Add new parameters OFFBODY and OFFBODYTRIM to cam2cam #5704
Add new parameters OFFBODY and OFFBODYTRIM to cam2cam #5704
Changes from 8 commits
a24237f
af481f8
e86433c
805971c
d52d27e
1f2780b
fd8a1c6
80bd6e4
ae2b05b
033e367
f599aa4
78cbf3c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this class is only used in the tests, can it be moved into
FunctionalTestsCam2cam.cpp
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this class has general utility other than strictly testing. All one has to do is add the include file in their own code.
It also keeps the class with the context in which it is designed to model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If cam2cam was updated to use it I would agree but it isn't currently being used in the codebase, only the test suite.
It also seems like a rewrite of the cam2camXform, could one not use the cam2camXform in the same way that one would use this new class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you may be right.
It is odd that it appears the only way to use this externally would require a link to cam2cam.o. It would be more better/useful if the implementation of cam2camXform is entirely in the header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could move the xform implementation into the .h as well