Skip to content

Conversation

Copy link

Copilot AI commented Aug 19, 2025

Problem

The setCameraParams function in CameraUtils.ts contained a variable shadowing issue that could cause incorrect camera calculations when horizontal field-of-view (FOV) clamping was required.

The bug occurred on line 79 where the focalLength parameter was being redeclared inside an if block:

function setCameraParams(
    camera: THREE.PerspectiveCamera,
    ppalPoint: Vector2Like,
    focalLength: number,  // Parameter
    viewportHeight: number,
    verticalFov: number
): void {
    // ... other code ...
    
    if (hFov < MIN_FOV_RAD || hFov > MAX_FOV_RAD) {
        hFov = THREE.MathUtils.clamp(hFov, MIN_FOV_RAD, MAX_FOV_RAD);
        const focalLength = computeFocalLengthFromFov(hFov, viewportWidth, ppalPoint.x); // BUG: Shadows parameter!
        verticalFov = computeFov(focalLength, ppalPoint.y, viewportHeight);
    }
    
    // Rest of function uses original parameter instead of adjusted value
    camera.userData.focalLength = focalLength;
}

This shadowing meant that when horizontal FOV needed to be clamped (typically with wide aspect ratios or extreme principal point offsets), the function would use the original focalLength parameter value in subsequent calculations instead of the newly computed adjusted value, leading to inconsistent camera state.

Solution

  • Introduced a new adjustedFocalLength variable initialized with the original focalLength parameter
  • Updated the horizontal FOV clamping code to assign to adjustedFocalLength instead of shadowing the parameter
  • Updated all subsequent uses of focal length to consistently use adjustedFocalLength
  • Added a comprehensive test case that exercises the horizontal FOV clamping code path to prevent regression

Testing

All existing tests continue to pass, and a new test case specifically verifies correct behavior when horizontal FOV clamping occurs with wide aspect ratios and off-center principal points.

The fix ensures camera calculations remain mathematically consistent when the system needs to adjust focal length due to FOV constraints, preventing potential rendering artifacts or incorrect camera positioning.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/722234/chrome-linux.zip
    • Triggering command: /usr/local/bin/node install.js (http block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Read the conversation here: https://github.com/copilot/c/6923fd36-95f5-4b95-8ca1-39ebbe8494bc and then try to fix the camera issue. Fix variable shadowing bug in CameraUtils.setCameraParams Aug 19, 2025
Copilot AI requested a review from trusktr August 19, 2025 19:25
Copilot finished work on behalf of trusktr August 19, 2025 19:25
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