Skip to content
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

Change aspect to AspectRatio() #635

Merged
merged 12 commits into from
Jun 9, 2022
2 changes: 2 additions & 0 deletions include/ignition/rendering/base/BaseCamera.hh
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ namespace ignition
void BaseCamera<T>::SetImageWidth(const unsigned int _width)
{
this->RenderTarget()->SetWidth(_width);
this->aspect = this->AspectRatio();
}

//////////////////////////////////////////////////
Expand All @@ -300,6 +301,7 @@ namespace ignition
void BaseCamera<T>::SetImageHeight(const unsigned int _height)
{
this->RenderTarget()->SetHeight(_height);
this->aspect = this->AspectRatio();
}

//////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion ogre/src/OgreCamera.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void OgreCamera::SetHFOV(const math::Angle &_angle)
{
BaseCamera::SetHFOV(_angle);
double angle = _angle.Radian();
double vfov = 2.0 * atan(tan(angle / 2.0) / this->aspect);
double vfov = 2.0 * atan(tan(angle / 2.0) / this->AspectRatio());
this->ogreCamera->setFOVy(Ogre::Radian(vfov));
}

Expand Down
2 changes: 1 addition & 1 deletion ogre2/src/Ogre2Camera.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ void Ogre2Camera::SetHFOV(const math::Angle &_angle)
{
BaseCamera::SetHFOV(_angle);
double angle = _angle.Radian();
double vfov = 2.0 * atan(tan(angle / 2.0) / this->aspect);
double vfov = 2.0 * atan(tan(angle / 2.0) / this->AspectRatio());
jennuine marked this conversation as resolved.
Show resolved Hide resolved
this->ogreCamera->setFOVy(Ogre::Radian(vfov));
}

Expand Down
4 changes: 2 additions & 2 deletions ogre2/src/Ogre2DepthCamera.cc
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,8 @@ void Ogre2DepthCamera::CreateRenderTexture()
void Ogre2DepthCamera::CreateDepthTexture()
{
// set aspect ratio and fov
double vfov = 2.0 * atan(tan(this->HFOV().Radian() / 2.0) / this->aspect);
this->ogreCamera->setAspectRatio(this->aspect);
double vfov;
vfov = 2.0 * atan(tan(this->HFOV().Radian() / 2.0) / this->AspectRatio());
this->ogreCamera->setFOVy(Ogre::Radian(this->LimitFOV(vfov)));

// Load depth material
Expand Down
4 changes: 2 additions & 2 deletions ogre2/src/Ogre2ThermalCamera.cc
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,8 @@ void Ogre2ThermalCamera::CreateRenderTexture()
void Ogre2ThermalCamera::CreateThermalTexture()
{
// set aspect ratio and fov
double vfov = 2.0 * atan(tan(this->HFOV().Radian() / 2.0) / this->aspect);
this->ogreCamera->setAspectRatio(this->aspect);
double vfov;
vfov = 2.0 * atan(tan(this->HFOV().Radian() / 2.0) / this->AspectRatio());
this->ogreCamera->setFOVy(Ogre::Radian(vfov));

// Load thermal material
Expand Down
5 changes: 5 additions & 0 deletions src/Camera_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@ void CameraTest::RenderTexture(const std::string &_renderEngine)
camera->SetImageHeight(80u);
EXPECT_EQ(80u, camera->ImageHeight());

double width = 100
camera.SetImageWidth(width)
double aspectRatio = width / camera.ImageHeight()
EXPECT_NEAR(aspectRatio, camera.AspectRatio(), 1e-6)
Copy link
Contributor

@jennuine jennuine Jun 6, 2022

Choose a reason for hiding this comment

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

This is missing ; at the end of each line and camera is a pointer. This test fails though, you will need to try to investigate why and how to correct this

Since you are using ogre on your system, temporarily update ogre2 to ogre here:

static const std::vector<const char *> kRenderEngineTestValues{"ogre2", "optix"};

To compile the test, run colcon build --cmake-args -DBUILD_TESTING=ON --merge-install --packages-select ignition-rendering6 in the root of your workspace. To run the test you can do ./build/ignition-rendering6/bin/UNIT_Camera_Test

Copy link
Contributor

Choose a reason for hiding this comment

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

You should also add a similar test with SetImageHeight

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in 8d4e542


EXPECT_NE(PixelFormat::PF_UNKNOWN, camera->ImageFormat());
camera->SetImageFormat(PixelFormat::PF_B8G8R8);
EXPECT_EQ(PixelFormat::PF_B8G8R8, camera->ImageFormat());
Expand Down