Skip to content

fix(OrthoPerspectiveCamera): safeguard zero size - #790

Merged
agviegas merged 2 commits into
ThatOpen:mainfrom
ShaMan123:fix/nan-camera-aspect
Sep 25, 2026
Merged

agviegas merged 2 commits into
ThatOpen:mainfrom
ShaMan123:fix/nan-camera-aspect

Conversation

@ShaMan123

@ShaMan123 ShaMan123 commented Aug 22, 2026 •

Copy link
Copy Markdown
Contributor

Description

Safeguards against a zero size canvas.
The use case: collapsing a canvas and resizing back would permanently damage the camera.

I wanted to add a test but vitest is not setup.

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following:

  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Follow the Conventional Commits v1.0.0 standard for PR naming (e.g. feat(examples): add hello-world example).
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@ShaMan123

ShaMan123 commented Aug 22, 2026 •

Copy link
Copy Markdown
Contributor Author
iimport * as OBC from "@thatopen/components";
import * as THREE from "three";
import { describe, expect, it } from "vitest";

const W = 1600;
const H = 900;

/** The slice of a world the aspect path reads. */
function fakeWorld(size: THREE.Vector2) {
  return { renderer: { getSize: () => size, isResizeable: () => true } };
}

/**
 * Seed the state `worlds.onItemSet` would have set, without assigning
 * `currentWorld` — that path also builds the navigation modes and their
 * `CameraControls`, which the frustum math has nothing to do with.
 */
function attachWorld(camera: OBC.OrthoPerspectiveCamera, size: THREE.Vector2) {
  const internals = camera as unknown as {
    _currentWorld: unknown;
    previousSize: THREE.Vector2;
  };
  internals._currentWorld = fakeWorld(size);
  internals.previousSize = size.clone();
}

/** Both cameras the app can render through, whichever `three` currently is. */
function expectFinite(camera: OBC.OrthoPerspectiveCamera) {
  for (const three of [camera.threePersp, camera.threeOrtho]) {
    expect(three.projectionMatrix.elements.every(Number.isFinite)).toBe(true);
  }
}

describe("OrthoPerspectiveCamera aspect", () => {
  it("keeps both projections finite across a collapse and re-expand", () => {
    const camera = new OBC.OrthoPerspectiveCamera(new OBC.Components());
    const size = new THREE.Vector2(W, H);
    attachWorld(camera, size);

    // The app never switches projection, so this is the one being rendered.
    expect(camera.three).toBe(camera.threePersp);
    camera.updateAspect();
    const framed = camera.threeOrtho.right;
    const aspect = camera.threePersp.aspect;
    expect(framed).toBeGreaterThan(0);
    expect(aspect).toBeCloseTo(W / H);

    // Collapse: the panel reports no width at all.
    size.set(0, H);
    camera.updateAspect();
    expect(camera.threePersp.aspect).toBeCloseTo(aspect);
    expectFinite(camera);

    // Re-expand to the original size — the frustum must come back unchanged.
    size.set(W, H);
    camera.updateAspect();

    expect(camera.threeOrtho.right).toBeCloseTo(framed);
    expect(camera.threeOrtho.top).toBeGreaterThan(0);
    expect(camera.threePersp.aspect).toBeCloseTo(aspect);
    expectFinite(camera);
  });

  it("re-baselines a zero size cached before the first real resize", () => {
    // The layout is persisted, so the viewer can MOUNT collapsed: the baseline
    // `worlds.onItemSet` captures is already zero-width before any resize.
    const camera = new OBC.OrthoPerspectiveCamera(new OBC.Components());
    const size = new THREE.Vector2(0, H);
    attachWorld(camera, size);

    const framed = camera.threeOrtho.right;
    size.set(W, H);
    camera.updateAspect();

    expect(camera.threeOrtho.right).toBeCloseTo(framed);
    expect(camera.threePersp.aspect).toBeCloseTo(W / H);
    expectFinite(camera);
  });

  it("still tracks a real resize", () => {
    const camera = new OBC.OrthoPerspectiveCamera(new OBC.Components());
    const size = new THREE.Vector2(W, H);
    attachWorld(camera, size);

    const framed = camera.threeOrtho.right;
    size.set(W / 2, H);
    camera.updateAspect();

    expect(camera.threeOrtho.right).toBeCloseTo(framed / 2);
    expect(camera.threePersp.aspect).toBeCloseTo(W / 2 / H);
  });
});

@agviegas

Copy link
Copy Markdown
Contributor

Merged as-is. Both guards check out: the early return keeps aspect = 0 from poisoning the projection matrix, and re-baselining a zero previousSize covers the mount-collapsed case. They compose, so re-expanding restores the exact frustum, and the change is inert unless a dimension really is zero, where the current behaviour is corruption.

Thanks also for the test you posted in the thread. It had nowhere to run when you wrote it, but the vitest setup from #805 is landing now, so we will add it to the suite on top of that rather than leave it living in a comment.

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