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.

Sign up for GitHub

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

Jump to bottom

Change aspect to AspectRatio() #635

Merged
merged 12 commits into from
Jun 9, 2022
Merged

Change aspect to AspectRatio() #635

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
df5d6d9
change aspect to AspectRatio()
AzulRadio May 26, 2022
a7cdf54
thermal and depth cam aspect ratio, didnt test
AzulRadio May 26, 2022
9723e19
add comment to aspect in BaseCamera
AzulRadio May 26, 2022
2286b60
update aspect in SetHFOV
AzulRadio May 26, 2022
8133de4
codecheck fix
AzulRadio May 26, 2022
fbbbc4b
update aspect ratio after resizing
AzulRadio May 31, 2022
932053a
remove change of aspect after changing HFOV
AzulRadio Jun 1, 2022
dcd5983
add aspect ratio for camera test
AzulRadio Jun 3, 2022
666638b
Merge branch 'ign-rendering6' into azulradio/HFOV_fix
AzulRadio Jun 3, 2022
2ad35e3
fix camera test
AzulRadio Jun 6, 2022
8d4e542
add SetAspectRatio to SetImageHeight and SetImageWidth
AzulRadio Jun 7, 2022
545a534
static cast to double
AzulRadio Jun 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/ignition/rendering/base/BaseCamera.hh
View file
Open in desktop
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->SetAspectRatio(1.0 * _width / this->ImageHeight());
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume you are multiplying by 1.0 to cast the result to a double. It's better to use static_cast<double> to be explicit with the cast making the purpose clear. Same for SetImageHeight

Suggested change
this->SetAspectRatio(1.0 * _width / this->ImageHeight());
this->SetAspectRatio(
static_cast<double>(_width) / static_cast<double>(this->ImageHeight()));

}

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

//////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion ogre/src/OgreCamera.cc
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
11 changes: 11 additions & 0 deletions src/Camera_TEST.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,17 @@ void CameraTest::RenderTexture(const std::string &_renderEngine)
camera->SetImageHeight(80u);
EXPECT_EQ(80u, camera->ImageHeight());

double height = 80;
camera->SetImageHeight(height);
double aspectRatio = camera->ImageWidth() / height;
EXPECT_NEAR(aspectRatio, camera->AspectRatio(), 1e-6);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since your new tests are almost the same as the previous test, lets remove the new test and continue on the previous one. E.g.,

Suggested change
camera->SetImageHeight(80u);
EXPECT_EQ(80u, camera->ImageHeight());
double height = 80;
camera->SetImageHeight(height);
double aspectRatio = camera->ImageWidth() / height;
EXPECT_NEAR(aspectRatio, camera->AspectRatio(), 1e-6);
unsigned int height = 80;
camera->SetImageHeight(height);
EXPECT_EQ(height, camera->ImageHeight());
double aspectRatio =
static_cast<double>(camera->ImageWidth()) / static_cast<double>(height));
EXPECT_NEAR(aspectRatio, camera->AspectRatio(), 1e-6);

The same should be done for width.

Also, go ahead and update this expectation below to use the new variables:

gz-rendering/src/Camera_TEST.cc

Line 195 in 8d4e542

EXPECT_EQ(100u*80u*3u, camera->ImageMemorySize());

e.g., EXPECT_EQ(width*height*3u, ...)


double width = 100;
camera->SetImageWidth(width);
aspectRatio = width / camera->ImageHeight();
EXPECT_NEAR(aspectRatio, camera->AspectRatio(), 1e-6);


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