Skip to content

Conversation

@Dtsitos
Copy link

@Dtsitos Dtsitos commented Jan 2, 2026

Describe your changes

This PR adds read-only getters to the camera API to retrieve derived world-space
camera parameters (azimuth, elevation, and distance).

The values are computed from the camera position and focal point and do not
modify camera state. This enables external applications and bindings to query
camera orientation consistently without duplicating math.

No existing behavior is changed.

Original PR: #2724

Issue ticket number and link if any

Fixes #1419

Checklist for finalizing the PR

  • I have performed a self-review of my code
  • I have added tests for new features and bugfixes
  • I have added documentation for new features
  • If it is a modifying the libf3d API, I have updated bindings
  • If it is a modifying the .github/workflows/versions.json, I have updated docker_timestamp

Continuous integration

Please write a comment to run CI, eg: \ci fast.
See here for more info.

…ickly for further processing

(cherry picked from commit 935740b)
…n to compute the distance from the camera position to the focal point

(cherry picked from commit a54c71e)
(cherry picked from commit 3f8671b)
…mproved getWorldAzimuth using SignedAngleBetweenVectors
@github-actions
Copy link

github-actions bot commented Jan 2, 2026

You are modifying libf3d public API! ⚠️Please update bindings accordingly⚠️!
You can find them in their respective directories: c, python, java, webassembly.

@Dtsitos
Copy link
Author

Dtsitos commented Jan 2, 2026

I think this should be correct now, will work on better testing ASAP

Comment on lines +136 to +142
static constexpr double EPS = 128 * std::numeric_limits<double>::epsilon();
if (vtkMath::Norm(horizontal.data()) < EPS)
{
return 0.0;
}

vtkMath::Normalize(horizontal.data());
Copy link
Contributor

Choose a reason for hiding this comment

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

Once you've got the tests you can double check if this is actually needed

Copy link
Member

Choose a reason for hiding this comment

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

so, is it needed @Dtsitos ?

@mwestphal
Copy link
Member

please let us know when its ready for review @Dtsitos

@Dtsitos
Copy link
Author

Dtsitos commented Jan 4, 2026

@mwestphal Hello there, I have implemented all the bindings and some basic testing for them. Full disclosure, I have never worked with bindings again and mostly deduced what to do based on the patterns from the existing implementations. Thank you for your patience

@mwestphal
Copy link
Member

@mwestphal Hello there, I have implemented all the bindings and some basic testing for them. Full disclosure, I have never worked with bindings again and mostly deduced what to do based on the patterns from the existing implementations. Thank you for your patience

Thats perfectly fine and it looks good to me :)

@mwestphal
Copy link
Member

@Dtsitos can you adress and resolve the suggestions made by @snoyer above ?
If you did adress them already, please resolve the discussions.

@mwestphal
Copy link
Member

\ci full

@github-actions
Copy link

github-actions bot commented Jan 4, 2026

Style Checks CI failed:

diff --git a/library/src/camera_impl.cxx b/library/src/camera_impl.cxx
index 7db2c14..a7ac8cd 100644
--- a/library/src/camera_impl.cxx
+++ b/library/src/camera_impl.cxx
@@ -7,7 +7,6 @@
 #include <vtkRenderer.h>
 #include <vtkVersion.h>
 
-
 namespace f3d::detail
 {
 class camera_impl::internals
@@ -145,7 +144,6 @@ double camera_impl::getWorldAzimuth() const
   double angleRad = vtkMath::SignedAngleBetweenVectors(right, horizontal.data(), up);
 
   return vtkMath::DegreesFromRadians(angleRad);
-
 }
 
 //----------------------------------------------------------------------------
diff --git a/library/testing/TestSDKCamera.cxx b/library/testing/TestSDKCamera.cxx
index 8bc6885..6fbfda4 100644
--- a/library/testing/TestSDKCamera.cxx
+++ b/library/testing/TestSDKCamera.cxx
@@ -243,8 +243,8 @@ int TestSDKCamera([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
 
   if (!compareDouble(elevation, 0.0))
   {
-    std::cerr << "getWorldElevation (horizontal) is not behaving as expected: "
-              << elevation << "\n";
+    std::cerr << "getWorldElevation (horizontal) is not behaving as expected: " << elevation
+              << "\n";
     return EXIT_FAILURE;
   }
 
@@ -259,22 +259,22 @@ int TestSDKCamera([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
 
   if (!compareDouble(distance, std::sqrt(11.0 * 11.0 + 11.0 * 11.0)))
   {
-    std::cerr << "getDistance (positive elevation) is not behaving as expected: "
-              << distance << "\n";
+    std::cerr << "getDistance (positive elevation) is not behaving as expected: " << distance
+              << "\n";
     return EXIT_FAILURE;
   }
 
   if (!compareDouble(azimuth, 0.0))
   {
-    std::cerr << "getWorldAzimuth (positive elevation) is not behaving as expected: "
-              << azimuth << "\n";
+    std::cerr << "getWorldAzimuth (positive elevation) is not behaving as expected: " << azimuth
+              << "\n";
     return EXIT_FAILURE;
   }
 
   if (!compareDouble(elevation, 45.0))
   {
-    std::cerr << "getWorldElevation (positive elevation) is not behaving as expected: "
-              << elevation << "\n";
+    std::cerr << "getWorldElevation (positive elevation) is not behaving as expected: " << elevation
+              << "\n";
     return EXIT_FAILURE;
   }
 
@@ -289,15 +289,15 @@ int TestSDKCamera([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
 
   if (!compareDouble(distance, std::sqrt(11.0 * 11.0 + 11.0 * 11.0)))
   {
-    std::cerr << "getDistance (negative elevation) is not behaving as expected: "
-              << distance << "\n";
+    std::cerr << "getDistance (negative elevation) is not behaving as expected: " << distance
+              << "\n";
     return EXIT_FAILURE;
   }
 
   if (!compareDouble(azimuth, 0.0))
   {
-    std::cerr << "getWorldAzimuth (negative elevation) is not behaving as expected: "
-              << azimuth << "\n";
+    std::cerr << "getWorldAzimuth (negative elevation) is not behaving as expected: " << azimuth
+              << "\n";
     return EXIT_FAILURE;
   }
 

@Dtsitos
Copy link
Author

Dtsitos commented Jan 7, 2026

\ci full

@mwestphal
Copy link
Member

not ready for full CI yet :)

@mwestphal mwestphal added ci:fast and removed ci:full labels Jan 7, 2026
@mwestphal
Copy link
Member

do you need help with the style check @Dtsitos ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add getters for camera parameters

3 participants