Feature: Add getters for camera parameters #1419 #1663
Conversation
|
You are modifying libf3d public API! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1663 +/- ##
==========================================
- Coverage 95.69% 95.65% -0.04%
==========================================
Files 125 125
Lines 9926 9951 +25
==========================================
+ Hits 9499 9519 +20
- Misses 427 432 +5 ☔ View full report in Codecov by Sentry. |
library/public/camera.h
Outdated
| @@ -103,4 +112,4 @@ class F3D_EXPORT camera | |||
| }; | |||
| } | |||
|
|
|||
| #endif | |||
| #endif | |||
| double viewDirProj[2] = { viewDir[0], viewDir[2] }; | ||
| if (vtkMath::Dot2D(viewDirProj, viewDirProj) < VTK_DBL_EPSILON) | ||
| { | ||
| return 0.0; |
There was a problem hiding this comment.
please add a test for this
| double viewDirProj[2] = { viewDir[0], viewDir[1] }; | ||
| if (vtkMath::Dot2D(viewDirProj, viewDirProj) < VTK_DBL_EPSILON) | ||
| { | ||
| return 0.0; |
There was a problem hiding this comment.
please add a test for this
library/testing/TestSDKCamera.cxx
Outdated
| @@ -179,8 +179,30 @@ int TestSDKCamera(int argc, char* argv[]) | |||
| return EXIT_FAILURE; | |||
| } | |||
|
|
|||
| // Test getAzimuth | |||
| f3d::angle_deg_t addAzimuth = cam.getAzimuth(); | |||
There was a problem hiding this comment.
strange name, why not call the var "azimuth" ?
There was a problem hiding this comment.
I mean the name of the variable. not the name of the method.
There was a problem hiding this comment.
Oh that's there just to clarify it. I can remove it.
library/testing/TestSDKCamera.cxx
Outdated
| @@ -179,8 +179,30 @@ int TestSDKCamera(int argc, char* argv[]) | |||
| return EXIT_FAILURE; | |||
| } | |||
|
|
|||
| // Test getAzimuth | |||
| f3d::angle_deg_t addAzimuth = cam.getAzimuth(); | |||
| if (!compareDouble(addAzimuth, 0.0)) | |||
There was a problem hiding this comment.
I would expect this to be 90, not 0 ?
There was a problem hiding this comment.
also please put this test right after the addAzimuth test
There was a problem hiding this comment.
I changed it to 90 and added it after addAzimuth.
library/testing/TestSDKCamera.cxx
Outdated
|
|
||
| // Test getElevation | ||
| double addElevation = cam.getElevation(); | ||
| checkDouble(addElevation, 0.0, "getElevation"); |
There was a problem hiding this comment.
please put this test after the test of addElevation
There was a problem hiding this comment.
I added it after addElevation.
library/testing/TestSDKCamera.cxx
Outdated
| if (dotProduct < epsilon) | ||
| { | ||
| std::cerr << "Dot product is lesser than epsilon, returning 0.0 as expected." << std::endl; | ||
| return 0.0; |
There was a problem hiding this comment.
return 0.0 ? this is a bug.
|
@Schwarzemann any news on this ? |
Ah yes. Sorry work has been very busy lately. I will take care of it this week for sure. |
|
Do you need any help moving forward ? |
Actually yes kind of. I would really appreciate it. I just can't wrap my head around the tests also can't seem to include |
|
Do you need any help moving forward ? |
Not right now thanks. I just have problems with time management nowadays. |
Schwarzemann
changed the title
Opening a new PR.