Skip to content

bugfix(view): Fix and improve camera area constraints and camera pivoting#2480

Open
xezon wants to merge 2 commits intoTheSuperHackers:mainfrom
xezon:xezon/fix-view-constraints
Open

bugfix(view): Fix and improve camera area constraints and camera pivoting#2480
xezon wants to merge 2 commits intoTheSuperHackers:mainfrom
xezon:xezon/fix-view-constraints

Conversation

@xezon
Copy link

@xezon xezon commented Mar 21, 2026

Merge with Rebase

The camera area constraints are now recalculated when the camera zoom changes, for example because of terrain elevation changes in the camera's view. The camera will be smoothly pushed away from the constraints, but not while the user is scrolling, to make the scrolling along the map border a pleasant experience. This behavior ensures that the view can reach and see all areas of the map, and especially the bottom map border. The camera can now also be moved a bit closer to the map edges. All this is very useful to avoid issues where some units or structures are not possible to get into the view, for example a Chinook at a Supply at the top edge of a map (TheSuperHackers/GeneralsRankedMaps#7). Or a Dozer at the bottom edge of a valley on Defcon 6.

Additionally the camera pivoting was fixed. The camera now repositions correctly towards its ground pivot instead of zooming to a pivot that is not aligned to the terrain ground. User facing this looks identical, but technically it is different. Scrolling to different locations of the map will now keep the camera pivot always correctly centered to the ground. It is no longer necessary to press Numpad 5 to recenter the pivot.

This change also implicitly fixes the broken scripted camera in the USA 01 intro, which was introduced by change #2291.

Known issues

The scripted camera will not always position perfectly at the original locations when the ground pivot was changed. This will be fixed in a future change.

TODO

  • Replicate in Generals
  • Add pull id to commits

@xezon xezon added this to the Camera Improvements milestone Mar 21, 2026
@xezon xezon added Bug Something is not working right, typically is user facing Enhancement Is new feature or request Major Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour labels Mar 21, 2026
@greptile-apps
Copy link

greptile-apps bot commented Mar 21, 2026

Greptile Summary

This PR fixes and improves camera area constraints and camera pivoting in the W3D game view. The camera constraints are now recalculated dynamically when the zoom changes (e.g. due to terrain elevation), the camera pivot is smoothly repositioned to the terrain ground, and constraint updates are deferred while the user is scrolling to keep edge-scrolling consistent.\n\nKey changes:\n- Splits camera zoom logic into zoomCameraToDesiredHeight() (scale-based, as before) and movePivotToGround() (new: repositions the pivot point to the terrain surface, fixing rotation origin and border collisions)\n- Introduces updateCameraAreaConstraints() / clipCameraIntoAreaConstraints() / isWithinCameraAreaConstraints() as proper helper methods, removing the inline constraint enforcement from buildCameraPosition and setCameraTransform\n- forceCameraAreaConstraintRecalc() now just sets the invalidation flag instead of calling calcCameraAreaConstraints() directly, aligning it with the deferred recalculation model\n- calcCameraAreaOffset is simplified: uses isLookingDown to choose the reference edge and applies a 0.85× scaling factor to allow scrolling closer to map edges\n- initHeightForMap delegates to resetPivotToGround(), removing the previous 120-unit ground-level cap\n- InGameUI::resetCamera and the middle-button click handler in LookAtXlat.cpp now both call userResetPivotToGround() before restoring defaults\n- BaseType.h gains zero() helpers and isInRegion() on 2D region types; Region3D::isInRegionWithZ is renamed to isInRegion\n- WWMath gains Inverse_Lerp (float and double overloads) used for pitch-weighted repositioning strength

Confidence Score: 4/5

PR is well-structured and the logic is sound; only minor P2 issues remain (date comment and an unguarded division in a new utility function).

The camera constraint and pivot logic is carefully refactored with good separation of concerns. The deferred-constraint-while-scrolling design is intentional and well-documented. Two non-blocking P2 findings: a new comment references 2025 instead of 2026 (custom rule violation), and the new Inverse_Lerp utility has no a == b guard (safe at its current call-site due to compile-time constants, but a latent hazard for future callers). Neither issue affects runtime behaviour today.

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp (date comment), Core/Libraries/Source/WWVegas/WWMath/wwmath.h (Inverse_Lerp guard)

Important Files Changed

Filename Overview
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp Core of the PR: refactors camera height management into zoomCameraToDesiredHeight/movePivotToGround, splits constraint calculation and clipping into dedicated helpers, and defers constraint recalculation while scrolling. One added comment references a prior-year date (2025).
Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DView.h Declares the new resetPivotToGround override, m_recalcCameraConstraintsAfterScrolling member, and the extracted private helpers; changes forceCameraAreaConstraintRecalc to only invalidate the flag instead of immediately recalculating.
Core/GameEngine/Include/GameClient/View.h Adds virtual resetPivotToGround() interface and userResetPivotToGround() user-action wrapper; minor doc comment whitespace fix.
Core/Libraries/Include/Lib/BaseType.h Adds zero() helpers and isInRegion() to Region2D/IRegion2D/IRegion3D; renames isInRegionWithZ to isInRegion in Region3D (no remaining callers of old name found); minor formatting fixes.
Core/Libraries/Source/WWVegas/WWMath/wwmath.h Adds Inverse_Lerp (float and double overloads) and renames Lerp parameter from 'lerp' to 't'; the new functions have no division-by-zero guard — safe at current call-site but a hazard for future callers.
GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp Replaces old resetCamera (which just re-applied the current view location) with proper pivot reset + default angle/pitch/zoom; cleaner and consistent with the middle-button click behaviour.
GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp Adds userResetPivotToGround() call on middle-button click (Numpad 5 equivalent) so the pivot is properly snapped to terrain before angle/pitch/zoom defaults are restored.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[W3DView::update] --> B{m_okToAdjustHeight?}
    B -- no --> E
    B -- yes --> C[zoomCameraToDesiredHeight]
    C --> D[movePivotToGround]
    D --> E{isScrolling?}
    E -- yes & zooming --> F[m_recalcCameraConstraintsAfterScrolling = true]
    E -- no & zooming --> G[m_cameraAreaConstraintsValid = false]
    F --> H{isTimeFast?}
    G --> H
    H -- yes --> Z[return early - no draw]
    H -- no --> I[updateCameraAreaConstraints]
    I --> J{m_cameraAreaConstraintsValid?}
    J -- no --> K[calcCameraAreaConstraints]
    K --> L{still outside constraints?}
    J -- yes --> L
    L -- yes --> M[clipCameraIntoAreaConstraints\nm_recalcCamera = true]
    L -- no --> N{m_recalcCamera?}
    M --> N
    N -- yes --> O[setCameraTransform]
    N -- no --> P[done]
    O --> P
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Line: 1419

Comment:
**Comment date references prior year (2025)**

The newly added comment references `26/10/2025`, which is prior to the current year (2026). Per project convention, newly created comments should reference the current year.

```suggestion
	// TheSuperHackers @bugfix xezon 26/10/2026 The camera area constraints are now recalculated when
```

**Rule Used:** What: Flag newly created code comments that refere... ([source](https://app.greptile.com/review/custom-context?memory=fd72a556-4fd8-4db4-8b08-8e51516a64ad))

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: Core/Libraries/Source/WWVegas/WWMath/wwmath.h
Line: 278-286

Comment:
**`Inverse_Lerp` has no division-by-zero guard**

When `a == b`, `(b - a)` is zero and the result will be `±inf` or `NaN`. The current call-site in `movePivotToGround` is safe because `lowerPitch` and `upperPitch` are distinct compile-time constants (`DEG_TO_RADF(15.f)` and `DEG_TO_RADF(30.f)`). However, as a general-purpose utility function added to the shared math library, future callers could inadvertently pass equal endpoints and get silent undefined behaviour.

Consider documenting the precondition, or adding a guard:

```cpp
WWINLINE float WWMath::Inverse_Lerp(float a, float b, float v)
{
	const float denom = (b - a);
	return (denom != 0.0f) ? (v - a) / denom : 0.0f;
}

WWINLINE double WWMath::Inverse_Lerp(double a, double b, float v)
{
	const double denom = (b - a);
	return (denom != 0.0) ? (v - a) / denom : 0.0;
}
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (5): Last reviewed commit: "bugfix(view): Fix and improve camera piv..." | Re-trigger Greptile

@xezon xezon force-pushed the xezon/fix-view-constraints branch 2 times, most recently from 04d2b61 to 8d78c38 Compare March 21, 2026 19:50
Copy link

@Mauller Mauller left a comment

Choose a reason for hiding this comment

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

I don't see any major issues at first glance, but i would perform some replay checking due to the new initialisation of class variables.

@xezon xezon force-pushed the xezon/fix-view-constraints branch 2 times, most recently from 3d23f96 to f56249d Compare March 25, 2026 18:43
@xezon xezon force-pushed the xezon/fix-view-constraints branch from f56249d to 7092cdf Compare March 26, 2026 18:41
Copy link

@Mauller Mauller left a comment

Choose a reason for hiding this comment

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

Looks good after a second look over

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is not working right, typically is user facing Enhancement Is new feature or request Gen Relates to Generals Major Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

2 participants