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

[bugfix] Update camera aspect ratio on window resize #738

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

vincentfretin
Copy link
Contributor

Updating the aspect ratio of the editor camera worked only for the active perspective camera, it didn't work when when you were using orthographic camera, and also the case using ortho, resize and then switching back to perspective camera didn't work either. This PR fixes updating the camera aspect in all cases.

For more details on the issue, this was discussed in 3DStreet/3dstreet#635

@@ -118,5 +118,4 @@ function setOrthoCamera(camera, dir, ratio) {
camera.bottom = info.bottom || -10;
camera.position.copy(info.position);
camera.rotation.copy(info.rotation);
camera.updateProjectionMatrix();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is now done in the updateAspectRatio() call that is done on cameratoggle event.

if (cameraHelper) cameraHelper.update();
}

inspector.sceneEl.addEventListener('rendererresize', updateAspectRatio);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using rendererresize that is emitted by the sceneEl.resize() function. sceneEl.resize() is called after switching the active camera when you close or open the inspector.

@dmarcos
Copy link
Member

dmarcos commented Jul 22, 2024

Thank you!

@dmarcos dmarcos merged commit 8ba9c15 into aframevr:master Jul 22, 2024
1 check passed
@vincentfretin vincentfretin deleted the update-aspect-ratio branch July 22, 2024 17:27
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