Skip to content

parse_camera rendering bug #1547

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

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from

Conversation

nicolemarsaglia
Copy link
Contributor

the order we parse camera params matters

@cyrush
Copy link
Member

cyrush commented Jun 9, 2025

unhappy windows test case:

2025-06-05T00:26:18.0385228Z   Testing 3D Rendering with Default PipelineInvalid color table name 'blue'. Defaulting to cool to warm
2025-06-05T00:26:18.0385808Z   test_file: 
2025-06-05T00:26:18.0386102Z     path: "D:\\a\\ascent\\ascent\\build\\tests\\_output\\render_1100.png"
2025-06-05T00:26:18.0386453Z     exists: "true"
2025-06-05T00:26:18.0386645Z   baseline_file: 
2025-06-05T00:26:18.0386957Z     path: "D:\\a\\ascent\\ascent\\src\\tests\\_baseline_images\\render_1100.png"
2025-06-05T00:26:18.0387341Z     exists: "true"
2025-06-05T00:26:18.0387527Z   dims_match: "true"
2025-06-05T00:26:18.0387743Z   percent_diff: 0.727937519550323
2025-06-05T00:26:18.0387995Z   tolerance: 0.00999999977648258
2025-06-05T00:26:18.0388224Z   pass: "false"
2025-06-05T00:26:18.0388553Z   diff_image: "D:\\a\\ascent\\ascent\\build\\tests\\_output\\diff_render_1100.png"
2025-06-05T00:26:18.0388936Z   
2025-06-05T00:26:18.0389614Z D:\a\ascent\ascent\src\tests\ascent\t_ascent_render_3d.cpp(1956): error : Value of: check_test_image(output_file1, 0.01f) [D:\a\ascent\ascent\build\RUN_TESTS.vcxproj]
2025-06-05T00:26:18.0390312Z     Actual: false
2025-06-05T00:26:18.0390505Z   Expected: true
2025-06-05T00:26:18.0390684Z   
2025-06-05T00:26:18.0390948Z   [  FAILED  ] ascent_render_3d.test_render_3d_multi_render (353 ms)

@cyrush
Copy link
Member

cyrush commented Jul 3, 2025

vtk Camera Elevation + Azimuth (and Roll, which are are not using) need position, lookat, and up to function.

void Camera::Roll(vtkm::Float32 angleDegrees)
{
  vtkm::Vec3f_32 directionOfProjection = this->GetLookAt() - this->GetPosition();
  vtkm::Matrix<vtkm::Float32, 4, 4> rotateTransform =
    vtkm::Transform3DRotate(angleDegrees, directionOfProjection);

  this->SetViewUp(vtkm::Transform3DVector(rotateTransform, this->GetViewUp()));
}

void Camera::Azimuth(vtkm::Float32 angleDegrees)
{
  // Translate to the focal point (LookAt), rotate about view up, and
  // translate back again.
  vtkm::Matrix<vtkm::Float32, 4, 4> transform = vtkm::Transform3DTranslate(this->GetLookAt());
  transform =
    vtkm::MatrixMultiply(transform, vtkm::Transform3DRotate(angleDegrees, this->GetViewUp()));
  transform = vtkm::MatrixMultiply(transform, vtkm::Transform3DTranslate(-this->GetLookAt()));

  this->SetPosition(vtkm::Transform3DPoint(transform, this->GetPosition()));
}

void Camera::Elevation(vtkm::Float32 angleDegrees)
{
  vtkm::Vec3f_32 axisOfRotation =
    vtkm::Cross(this->GetPosition() - this->GetLookAt(), this->GetViewUp());

  // Translate to the focal point (LookAt), rotate about the defined axis,
  // and translate back again.
  vtkm::Matrix<vtkm::Float32, 4, 4> transform = vtkm::Transform3DTranslate(this->GetLookAt());
  transform =
    vtkm::MatrixMultiply(transform, vtkm::Transform3DRotate(angleDegrees, axisOfRotation));
  transform = vtkm::MatrixMultiply(transform, vtkm::Transform3DTranslate(-this->GetLookAt()));

  this->SetPosition(vtkm::Transform3DPoint(transform, this->GetPosition()));
}

Zoom is independent of pos, lookat, and up:

void Camera::Zoom(vtkm::Float32 zoom)
{
  vtkm::Float32 factor = vtkm::Pow(4.0f, zoom);
  this->Camera3D.Zoom *= factor;
  this->Camera3D.XPan *= factor;
  this->Camera3D.YPan *= factor;
  this->Camera2D.Zoom *= factor;
  this->Camera2D.XPan *= factor;
  this->Camera2D.YPan *= factor;
}

Given this, it seems that azimuth and elevation should be set after we set position, lookat, and up. ?

@nicolemarsaglia
Copy link
Contributor Author

vtk Camera Elevation + Azimuth (and Roll, which are are not using) need position, lookat, and up to function.

void Camera::Roll(vtkm::Float32 angleDegrees)
{
  vtkm::Vec3f_32 directionOfProjection = this->GetLookAt() - this->GetPosition();
  vtkm::Matrix<vtkm::Float32, 4, 4> rotateTransform =
    vtkm::Transform3DRotate(angleDegrees, directionOfProjection);

  this->SetViewUp(vtkm::Transform3DVector(rotateTransform, this->GetViewUp()));
}

void Camera::Azimuth(vtkm::Float32 angleDegrees)
{
  // Translate to the focal point (LookAt), rotate about view up, and
  // translate back again.
  vtkm::Matrix<vtkm::Float32, 4, 4> transform = vtkm::Transform3DTranslate(this->GetLookAt());
  transform =
    vtkm::MatrixMultiply(transform, vtkm::Transform3DRotate(angleDegrees, this->GetViewUp()));
  transform = vtkm::MatrixMultiply(transform, vtkm::Transform3DTranslate(-this->GetLookAt()));

  this->SetPosition(vtkm::Transform3DPoint(transform, this->GetPosition()));
}

void Camera::Elevation(vtkm::Float32 angleDegrees)
{
  vtkm::Vec3f_32 axisOfRotation =
    vtkm::Cross(this->GetPosition() - this->GetLookAt(), this->GetViewUp());

  // Translate to the focal point (LookAt), rotate about the defined axis,
  // and translate back again.
  vtkm::Matrix<vtkm::Float32, 4, 4> transform = vtkm::Transform3DTranslate(this->GetLookAt());
  transform =
    vtkm::MatrixMultiply(transform, vtkm::Transform3DRotate(angleDegrees, axisOfRotation));
  transform = vtkm::MatrixMultiply(transform, vtkm::Transform3DTranslate(-this->GetLookAt()));

  this->SetPosition(vtkm::Transform3DPoint(transform, this->GetPosition()));
}

Zoom is independent of pos, lookat, and up:

void Camera::Zoom(vtkm::Float32 zoom)
{
  vtkm::Float32 factor = vtkm::Pow(4.0f, zoom);
  this->Camera3D.Zoom *= factor;
  this->Camera3D.XPan *= factor;
  this->Camera3D.YPan *= factor;
  this->Camera2D.Zoom *= factor;
  this->Camera2D.XPan *= factor;
  this->Camera2D.YPan *= factor;
}

Given this, it seems that azimuth and elevation should be set after we set position, lookat, and up. ?

Actually, we're fine not setting pos, lookat, or up until after since those will already be set with the default. But if we change 'up' before we change elevation (i.e. a new pos), then we get a bogus camera: #1546

@cyrush
Copy link
Member

cyrush commented Jul 7, 2025

With these changes -- position is set before, but look at and up are set after azimuth and elevation.
If the user provides any of these, we don't want to use the default.

Maybe for the elevation case, we need logic that re-sets the up (again) after applied?

@cyrush
Copy link
Member

cyrush commented Jul 7, 2025

Yes, the issue is vtk-m elevate does not adjust up, so we need to think about the right logic to smooth this out for ascent.

@cyrush cyrush added this to the 0.9.5 milestone Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants