Skip to content

refactor: make camera aspect ratio auto-refresh in Camera class#8633

Merged
mvaligursky merged 4 commits into
mainfrom
mv-camera-aspect-auto-refresh
Apr 22, 2026
Merged

refactor: make camera aspect ratio auto-refresh in Camera class#8633
mvaligursky merged 4 commits into
mainfrom
mv-camera-aspect-auto-refresh

Conversation

@mvaligursky
Copy link
Copy Markdown
Contributor

@mvaligursky mvaligursky commented Apr 22, 2026

Follow-up to #8632. Moves the automatic aspect ratio handling from CameraComponent into the Camera class itself, so it stays up to date whenever any of its inputs change — not just at onEnable time.

Motivation:
#8632 fixed the "stale aspect ratio before the first frame" problem inside CameraComponent.onEnable, but the same staleness still occurred when:

  • the camera's renderTarget was assigned at runtime,
  • the camera's rect was changed at runtime,
  • the backbuffer was resized between frames,
  • the user switched from ASPECT_MANUAL back to ASPECT_AUTO,
  • the camera was not yet attached to the scene tree (so onEnable never ran).

Changes:

  • Camera constructor now takes a GraphicsDevice (asserted non-null) and stores it on camera.device, so the camera can fall back to the backbuffer size when no render target is assigned.
  • Camera now owns calculateAspectRatio(rt); when rt is omitted it falls back to the camera's own renderTarget, then the backbuffer. rect.z/rect.w is included so the aspect matches the actually-rendered viewport.
  • get aspectRatio recomputes on every read in ASPECT_AUTO (the calculation is trivially cheap — a few multiplies and a divide — and this avoids any need for a device-event subscription). If the recomputed value differs from the cached one, _projMatDirty is flipped so the projection matrix is rebuilt on its next read.
  • _evaluateProjectionMatrix now force-reads aspectRatio before checking the dirty flag, so a backbuffer resize (with no camera setter touched) is picked up on the next projectionMatrix access.
  • CameraComponent.calculateAspectRatio now forwards to the underlying Camera, with clarified JSDoc (falls back to the camera's render target before the backbuffer, includes rect). The onEnable block added in Calculate camera aspect ration in CameraComponent onEnable #8632 is removed (the lazy getter supersedes it).
  • All new Camera() sites updated to pass a device: CameraComponent, Camera.clone(), LightCamera.create (and its callers in ShadowRenderer.createShadowCamera, render-pass-cookie-renderer, evalSpotCookieMatrix), and Lightmapper.

API Changes:

  • Camera constructor — new Camera()new Camera(graphicsDevice). Camera is internal, so this is not a public API break, but any code instantiating Camera directly must pass a GraphicsDevice.
  • LightCamera.create(name, lightType, face)LightCamera.create(device, name, lightType, face) (internal).
  • ShadowRenderer.createShadowCamera(shadowType, type, face)ShadowRenderer.createShadowCamera(device, shadowType, type, face) (internal).

Tests:

  • New test/scene/camera.test.mjs covering AUTO recompute on renderTarget, rect, backbuffer resize, MANUAL→AUTO switch, MANUAL preservation, projectionMatrix refresh after backbuffer resize, clone(), and the component-level forwarders.
  • element-drag-helper.test.mjs updated to explicitly lock ASPECT_MANUAL + 16/9, so its expected values don't drift with device dimensions.

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
@mvaligursky mvaligursky self-assigned this Apr 22, 2026
@mvaligursky mvaligursky requested a review from Copilot April 22, 2026 13:17
@mvaligursky mvaligursky added the area: graphics Graphics related issue label Apr 22, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors camera aspect ratio auto-updating so Camera can lazily recompute its aspect ratio when inputs change (render target, rect, backbuffer resize, mode switches), rather than relying on CameraComponent.onEnable.

Changes:

  • Camera now takes a GraphicsDevice, tracks aspect-ratio dirtiness, listens for resizecanvas, and exposes calculateAspectRatio.
  • Internal camera creation sites were updated to pass a device (camera component, cloning, light/shadow/cookie cameras, lightmapper).
  • New unit tests cover AUTO/MANUAL aspect behavior and cleanup; an existing UI drag test locks aspect ratio to avoid resolution-dependent expectations.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test/scene/camera.test.mjs Adds coverage for AUTO recomputation triggers, MANUAL preservation, destroy/unsubscribe, clone, and component forwarding.
test/framework/components/element/element-drag-helper.test.mjs Locks camera aspect to MANUAL 16:9 to make drag math independent of canvas size.
src/scene/camera.js Moves aspect auto-refresh logic into Camera (device-backed) with dirty flag + resize listener + forwarding method.
src/framework/components/camera/component.js Instantiates Camera with a device; removes onEnable aspect refresh; forwards calculateAspectRatio to Camera.
src/scene/renderer/light-camera.js Updates LightCamera.create to accept a device and constructs Camera(device).
src/scene/renderer/shadow-renderer.js Threads device into shadow camera creation via LightCamera.create(device, ...).
src/scene/renderer/render-pass-cookie-renderer.js Updates cookie renderer to pass device into temporary LightCamera creation.
src/scene/light.js Updates shadow camera creation call to pass light.device.
src/framework/lightmapper/lightmapper.js Updates baking camera creation to new Camera(this.device).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/scene/camera.js Outdated
Comment thread src/scene/renderer/render-pass-cookie-renderer.js
Comment thread src/scene/renderer/shadow-renderer.js
Comment thread src/framework/components/camera/component.js
Comment thread test/scene/camera.test.mjs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/scene/camera.js
Comment thread src/scene/renderer/light-camera.js
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/scene/camera.js
Comment thread src/scene/camera.js Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mvaligursky mvaligursky merged commit 31e0272 into main Apr 22, 2026
8 checks passed
@mvaligursky mvaligursky deleted the mv-camera-aspect-auto-refresh branch April 22, 2026 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: graphics Graphics related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants