Calculate camera aspect ration in CameraComponent onEnable#8632
Conversation
There was a problem hiding this comment.
Pull request overview
Updates CameraComponent so aspectRatio is refreshed immediately when the component is enabled, fixing cases where ASPECT_AUTO previously reported a stale default until the first render.
Changes:
- Recalculate
aspectRatioinCameraComponent#onEnablewhenaspectRatioMode === ASPECT_AUTO.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| if (this.aspectRatioMode === ASPECT_AUTO) { | ||
| this.aspectRatio = this.calculateAspectRatio(); |
There was a problem hiding this comment.
ASPECT_AUTO is documented as using the current render target’s dimensions, but onEnable recalculates the aspect ratio using the backbuffer only (calculateAspectRatio() with no rt). This still leaves aspectRatio stale when the camera has renderTarget set until the first frameUpdate. Consider passing the current renderTarget (or equivalent) into calculateAspectRatio here so the value matches what frameUpdate(renderTarget) will compute.
| this.aspectRatio = this.calculateAspectRatio(); | |
| this.aspectRatio = this.calculateAspectRatio(this.renderTarget); |
| if (this.aspectRatioMode === ASPECT_AUTO) { | ||
| this.aspectRatio = this.calculateAspectRatio(); | ||
| } |
There was a problem hiding this comment.
This change alters CameraComponent initialization semantics (aspect ratio becomes correct immediately on enable). The repo has component-level tests (e.g. test/framework/components/model/component.test.mjs), but there doesn’t appear to be coverage for camera aspect ratio behavior. Consider adding a focused test that enables a camera with aspectRatioMode === ASPECT_AUTO and asserts aspectRatio is correct before the first render/update (and ideally also when renderTarget is set).
mvaligursky
left a comment
There was a problem hiding this comment.
This works, thanks. We'd ideally handle additional cases, for example renderTarget setter as well, but that can be done separatetely.
Thanks!
* refactor: make camera aspect ratio auto-refresh in Camera class Follow-up to #8632. Moves automatic aspect ratio handling into the Camera class so it stays up to date whenever inputs change, not just at onEnable. - Camera constructor now takes a GraphicsDevice (asserted non-null) - Camera owns calculateAspectRatio(rt); aspectRatio getter recomputes lazily in ASPECT_AUTO when rect / renderTarget / backbuffer change - Subscribes to graphicsDevice 'resizecanvas' and unsubscribes on destroy - CameraComponent.calculateAspectRatio forwards to Camera; the onEnable block added in #8632 is removed (lazy getter supersedes it) - All new Camera() sites plumbed with device (LightCamera.create, ShadowRenderer.createShadowCamera, cookie renderer, lightmapper) - Adds test/scene/camera.test.mjs and locks drag-helper test to ASPECT_MANUAL so its expectations don't drift with canvas size * updates * changes * Update src/scene/camera.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Martin Valigursky <mvaligursky@snapchat.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Description
Fixes an issue where a
CameraComponentwithaspectRatioModeset toASPECT_AUTOwould report a stale/incorrectaspectRatiountil the first frame was rendered. The aspect ratio is now additionally calculated inonEnable, so it is correct from the moment the component is enabled.Problem
When a camera is enabled with
aspectRatioMode === ASPECT_AUTO,aspectRatiowas only refreshed by the render loop. Any code that readcamera.aspectRatiobefore the first frame — e.g. during scene setup, picking, frustum tests, or screen-space math — would see the default value of16/9, not the actual current value for the target render surface.Fixes #5825
Checklist