-
Notifications
You must be signed in to change notification settings - Fork 172
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
base: dev
Are you sure you want to change the base?
Conversation
… parameters. Addresses DOI-USGS#3602.
…updated documentation. Addresses issue DOI-USGS#3602.
The build and test suite have started for your pull request. To view your build log, please reference the build with source version: "PR_5704". Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch. |
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.
Overall looks good! Couple small things to address
* | ||
* @author 2024-10-27 Kris J. Becker | ||
*/ | ||
class Cam2CamMapper { |
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
The build and test suite have started for your pull request. To view your build log, please reference the build with source version: "PR_5704". Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch. |
The build and test suite have started for your pull request. To view your build log, please reference the build with source version: "PR_5704". Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch. |
Add new parameters OFFBODY and OFFBODYTRIM to cam2cam. Updated functional tests. Updated documentation.
Description
Enhance cam2cam to project all pixels in the focal plane - on and/or off the target body - to the output image. This has been extremely useful in detecting and tracking ejected particles from Bennu's surface.
Two program parameters were added - OFFBODY and OFFBODYTRIM.
Enhancement to cam2cam is necessary to support upcoming addition of the same new parameters (OFFBODY, OFFBODYTRIM) to noproj.
Related Issue
#3602
How Has This Been Validated?
Existing functional test passed.
New tests added to exercise the functionality of the new parameters, confirmed passing.
ctest -R Cam2Cam --output-on-failure
Test project /Users/ssutton/ISIS/GitCheckOuts/cam2camKeywords/ISIS3/build
Start 1354: DefaultCube.FunctionalTestCam2CamNoChange
1/2 Test #1354: DefaultCube.FunctionalTestCam2CamNoChange ... Passed 2.74 sec
Start 1355: DefaultCube.FunctionalTestCam2CamOffbody
2/2 Test #1355: DefaultCube.FunctionalTestCam2CamOffbody .... Passed 9.33 sec
100% tests passed, 0 tests failed out of 2
Total Test time (real) = 12.26 sec
Types of changes
Checklist:
Licensing
This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words: