Skip to content

fix(view): Adjust default camera height to compensate for screen aspect ratio#1711

Open
Mauller wants to merge 1 commit intoTheSuperHackers:mainfrom
Mauller:fix-default-camera-height
Open

fix(view): Adjust default camera height to compensate for screen aspect ratio#1711
Mauller wants to merge 1 commit intoTheSuperHackers:mainfrom
Mauller:fix-default-camera-height

Conversation

@Mauller
Copy link

@Mauller Mauller commented Oct 18, 2025

Merge with Rebase

This PR adjusts the default max camera height to compensate for different screen aspect ratios.

Previous work fixed the vertical field of view to match retail and adjusted the horizontal fields of view.
But this resulted in significant distortion being observed in the periphery of the view and lower corners.
This distortion was also observed at the 16:9 aspect ratio and became significantly worse as the aspect ratio increased.

By maintaining the original 60 degrees horizontal field of view, while allowing the game to narrow the vertical field of view, increasing the camera height results in minimal to no distortion being observed.

This PR does not fix any other known camera issues and focuses on giving a playable default view.


TODO

  • Implement in generals

@Mauller Mauller self-assigned this Oct 18, 2025
@Mauller Mauller added Bug Something is not working right, typically is user facing GUI For graphical user interface Major Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour labels Oct 18, 2025
@Mauller Mauller added this to the Major bug fixes milestone Oct 18, 2025
@ElTioRata
Copy link

Comparison

@xezon
Copy link

xezon commented Oct 19, 2025

At 4:3 "This Change" gives identical view to "Original Game + GenTool" ✅

At 16:9 "This Change" gives a bit more view than "Original Game + GenTool" ❓

Is this correct? I do not recall if GenTool view is correct.

zoom16x9

@xezon xezon added Enhancement Is new feature or request and removed GUI For graphical user interface labels Oct 19, 2025
@Mauller
Copy link
Author

Mauller commented Oct 19, 2025

At 4:3 "This Change" gives identical view to "Original Game + GenTool" ✅

At 16:9 "This Change" gives a bit more view than "Original Game + GenTool" ❓

Is this correct? I do not recall if GenTool view is correct.

zoom16x9

My goal was to try match the vertical view as close as possible to retail, I think it's slightly less than compared to 4:3 but slightly more than 16:9 gentool + retail.

I think it has a reasonable tradeoff considering the extended horizontal view.

It also gives a reasonable view for aspect ratios between 4:3 and 1:1 as well. Giving a little more vertical view.

Under 1:1 things are still quite broken but a lot needs changing in the view handling to properly support portrait mode.

@xezon
Copy link

xezon commented Oct 19, 2025

Yes GenTool did take some vertical view away to compensate for the wider view, perhaps in an attempt to keep conditions fair. Additional camera height does give a competitive advantage. Are we ok with giving advantages to Wide Screen?

@Mauller
Copy link
Author

Mauller commented Oct 19, 2025

I wouldn't consider it an issue since most screens have been wide aspect For the past decade. If anything widescreen is the standard now.

Ultrawide is where it becomes more of an issue. But we could look at implementing a locked aspect mode for the tactical view in future.
This could give a standardised view to all players in a competition etc.

Or another option could be implementing tactical zoom to let people zoom out to view the entire map etc.

@Mauller Mauller force-pushed the fix-default-camera-height branch from 02a64e0 to 57fe78d Compare October 19, 2025 15:37
Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Simple implementation, but man the EA code is confusing.

@Mauller Mauller force-pushed the fix-default-camera-height branch from 57fe78d to 271ca5b Compare October 19, 2025 19:09
@Mauller
Copy link
Author

Mauller commented Oct 19, 2025

Tweaked the code so setDefaultView is now setDefaultCameraHeight with a default argument of 1.0f for the heightScale.

Still need to look at the zoom default handling.

@Mauller Mauller force-pushed the fix-default-camera-height branch from 271ca5b to f6c96d8 Compare October 19, 2025 19:36
@Mauller
Copy link
Author

Mauller commented Oct 19, 2025

Updated with tweaked max and default zoom handling code.

@Mauller Mauller force-pushed the fix-default-camera-height branch from f6c96d8 to c3c7b85 Compare October 19, 2025 19:56
@Mauller
Copy link
Author

Mauller commented Oct 19, 2025

Small tweak to comments

@Mauller
Copy link
Author

Mauller commented Oct 24, 2025

Last one was a quick rebase with main

@Mauller Mauller force-pushed the fix-default-camera-height branch from c5fbb86 to 52f7de7 Compare October 26, 2025 09:56
@Mauller
Copy link
Author

Mauller commented Oct 26, 2025

Tweaked naming, should be good now

m_drawHeightY=m_height;
}
else {
//TheSuperHacker @bugfix Mauller 23/10/2025 Increase the terrain draw area for wide aspect ratios
Copy link

@xezon xezon Oct 25, 2025

Choose a reason for hiding this comment

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

@tweak or @fix, because in original game this is not a bug, because the widescreen camera can not see that far.

center.y -= bottom.y;

Real offset = center.length();
// TheSuperHackers @bugfix Mauller 22/10/2025 Halve the camera offset to allow the camera to move closer to the map edges
Copy link

Choose a reason for hiding this comment

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

@tweak

Copy link
Author

Choose a reason for hiding this comment

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

removing this for now till after this and your camera extents PR's are done

m_drawHeightY=m_height;
}
else {
//TheSuperHacker @bugfix Mauller 23/10/2025 Increase the terrain draw area for wide aspect ratios
Copy link

Choose a reason for hiding this comment

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

I think this does not work correctly in Shell Map. When changing aspect ratio from 4:3 to wide, then this code is not called. Can we run this after the WorldHeightMap was created?

Copy link
Author

@Mauller Mauller Oct 27, 2025

Choose a reason for hiding this comment

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

yeah i am just looking a bit more into this and it's not very straightforwards, the heightmap is nested inside BaseHeightMap inside TheTerrainVisual within TheTerrainLogic.

Just muddling through a way to cleanly get at the height map inside it all from the terrain logic which has a singleton.


Real offset = center.length();
// TheSuperHackers @bugfix Mauller 22/10/2025 Halve the camera offset to allow the camera to move closer to the map edges
Real offset = center.length() * 0.5f;
Copy link

Choose a reason for hiding this comment

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

I tested this behavior and I suspect this was done to address #1270. However, the black area that this creates is quite substantial. I have created new change #1752 to address #1270 without extending the constraints area into blackness.

There still is merit to experiment with a greater viewable area, for example center.length() * 0.9f, but I suggest to do that after #1752.

Copy link
Author

Choose a reason for hiding this comment

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

i will revert the offset change for now and we can look at that after this and #1752 are merged.

@Mauller Mauller force-pushed the fix-default-camera-height branch from 52f7de7 to ab0b316 Compare November 1, 2025 14:15
@Mauller Mauller changed the title fix(view): Adjust default camera height to compensate for screen aspect ratio, reduce camera map limits and increase terrain render distance. fix(view): Adjust default camera height to compensate for screen aspect ratio Nov 1, 2025
@Mauller
Copy link
Author

Mauller commented Nov 1, 2025

Reverted this to just have the camera height adjustments and nothing else.

@Mauller Mauller force-pushed the fix-default-camera-height branch 3 times, most recently from 968ad71 to e497ba8 Compare November 7, 2025 16:15
@xezon
Copy link

xezon commented Nov 15, 2025

Note for future:

When raising up the camera, the following things need to be scaled as well:

farZ in W3DView::setCameraTransform

m_x, m_y in HeightMapRenderObjClass

@Mauller Mauller force-pushed the fix-default-camera-height branch from e497ba8 to a0b7db7 Compare January 16, 2026 16:48
@greptile-apps
Copy link

greptile-apps bot commented Jan 16, 2026

Greptile Summary

This PR introduces aspect-ratio-aware camera height limits via the new setCameraHeightAboveGroundLimitsToDefault() method and separates zoom initialisation into an explicit setZoomToMax() call, so wider screens (e.g. 16:9) automatically receive a proportionally higher default camera position with minimal visual distortion.

Key changes and outstanding concerns:

  • Aspect-ratio scale formula is redundant — the if/else branches in setCameraHeightAboveGroundLimitsToDefault() produce mathematically identical results and can be collapsed into one expression (see inline comment).
  • InGameUI::init() and InGameUI::reset() are missing setZoomToMax() (pre-flagged) — height limits are updated but the zoom value is never synced, leaving the camera in an indeterminate state until the next startNewGame() call.
  • View::setZoomToMax() base-class implementation is incorrect (pre-flagged) — it adds m_maxHeightAboveGround to the current height rather than setting it; harmless in practice because W3DView::setZoomToMax() overrides it, but misleading.
  • heighScale typo in ScriptActions (pre-flagged) — both the header and implementation have the parameter misspelled.
  • doCameraSetDefault now stops scripted cameras via setPitch/setAngle side-effects (pre-flagged) — the original setDefaultView did not have this side-effect.
  • GameLogic::startNewGame() and OptionsMenu::saveOptions() correctly pair setCameraHeightAboveGroundLimitsToDefault() with setZoomToMax().
  • The TODO item ("Implement in Generals") notes this is not yet wired up for the vanilla Generals build.

Confidence Score: 2/5

  • Multiple pre-flagged bugs remain open; not ready to merge until the missing setZoomToMax calls, script-camera side-effects, and typo are resolved.
  • The core aspect-ratio scaling logic is sound and the game-start path works correctly, but at least four issues from prior review rounds are still unresolved: missing setZoomToMax() in InGameUI init/reset, the heighScale typo propagated into the public header, doCameraSetDefault silently stopping scripted cameras, and the base-class setZoomToMax() incorrectly adding instead of setting height. These collectively affect script-camera reliability and camera state correctness in non-game-start contexts.
  • Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp (redundant if/else), GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp (missing setZoomToMax), GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp and ScriptActions.h (typo + scripted-camera side-effects), Core/GameEngine/Source/GameClient/View.cpp (incorrect base setZoomToMax).

Important Files Changed

Filename Overview
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp Core of the PR: adds setCameraHeightAboveGroundLimitsToDefault() for aspect-ratio-aware height limits and splits setZoomToMax() out of setZoomToDefault(). The aspect-ratio scale formula has redundant if/else branches that compute identical values. Pre-flagged issues remain: setZoomToDefault() no longer applies zoom, and fabs() wraps already-positive expressions.
Core/GameEngine/Source/GameClient/View.cpp Adds the base-class setZoomToMax() stub, but the implementation (adding m_maxHeightAboveGround to the current height rather than setting it to max) is incorrect — pre-flagged and still unresolved. In practice the virtual dispatch routes to W3DView::setZoomToMax(), mitigating the impact.
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp Adds the correct call sequence for game start: setCameraHeightAboveGroundLimitsToDefault() → angle/pitch defaults → setZoomToDefault()setZoomToMax(). No issues found here.
GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp Both init() and reset() now call only setCameraHeightAboveGroundLimitsToDefault() without the companion setZoomToMax(). This leaves the camera zoom uninitialized relative to the new height limits — a pre-flagged, unresolved issue.
GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/OptionsMenu.cpp Correctly pairs setCameraHeightAboveGroundLimitsToDefault() with setZoomToMax() on resolution change, with an appropriate comment explaining why setZoomToDefault() is intentionally skipped to avoid stopping the shell map scripted camera.
GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp Signature of doCameraSetDefault changed to use heighScale (typo, missing 't') and now calls setPitch/setAngle which have the side effect of stopping scripted cameras. Both issues were pre-flagged and remain unresolved.
GeneralsMD/Code/GameEngine/Include/GameLogic/ScriptActions.h Declaration updated to use heighScale (typo, missing 't'). Pre-flagged and unresolved.
Core/GameEngine/Include/GameClient/View.h Adds setCameraHeightAboveGroundLimitsToDefault() and setZoomToMax() as new virtual methods with correct no-op base implementations in the header.
Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DView.h Declares the new setCameraHeightAboveGroundLimitsToDefault() and setZoomToMax() overrides. The updated setZoomToDefault() comment correctly warns that the function stops scripted cameras.
GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h Adds a clarifying comment explaining that m_maxCameraHeight and m_minCameraHeight are 4:3 baseline values. No functional changes.

Sequence Diagram

sequenceDiagram
    participant GL as GameLogic::startNewGame
    participant OM as OptionsMenu::saveOptions
    participant UI as InGameUI::init/reset
    participant TV as TheTacticalView (W3DView)

    Note over GL: Game start path (correct)
    GL->>TV: setCameraHeightAboveGroundLimitsToDefault()
    GL->>TV: setAngleToDefault()
    GL->>TV: setPitchToDefault()
    GL->>TV: setZoomToDefault()
    Note right of TV: stops scripted camera,<br/>but does NOT set zoom
    GL->>TV: setZoomToMax()
    Note right of TV: computes & applies zoom<br/>based on new height limits

    Note over OM: Resolution change path (correct)
    OM->>TV: setCameraHeightAboveGroundLimitsToDefault()
    OM->>TV: setZoomToMax()
    Note right of TV: skips setZoomToDefault()<br/>to preserve shell-map camera

    Note over UI: InGameUI path (incomplete)
    UI->>TV: setCameraHeightAboveGroundLimitsToDefault()
    Note right of TV: setZoomToMax() is never called —<br/>zoom remains uninitialized
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: 1906-1913

Comment:
**Both if/else branches are mathematically identical**

The two branches of the conditional compute exactly the same value:

- **if-branch:** `fabs(1 + (currentAspectRatio - baseAspectRatio))`
- **else-branch:** `fabs(1 - (baseAspectRatio - currentAspectRatio))` = `fabs(1 - baseAspectRatio + currentAspectRatio)` = `fabs(1 + (currentAspectRatio - baseAspectRatio))`

The entire `if`/`else` block is redundant and can be collapsed into one expression:

```suggestion
	Real aspectRatioScale = fabs(1.0f + (currentAspectRatio - baseAspectRatio));
```

As a side effect, this also fixes the misleading implication that the narrower-than-baseline case is handled differently.

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

Reviews (8): Last reviewed commit: "fix(view): Adjust default camera height ..." | Re-trigger Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

10 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

/** doCameraSetDefault */
//-------------------------------------------------------------------------------------------------
void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real maxHeight)
void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real heighScale)
Copy link

Choose a reason for hiding this comment

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

syntax: typo: heighScale should be heightScale

Suggested change
void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real heighScale)
void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real heightScale)
Prompt To Fix With AI
This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp
Line: 4611:4611

Comment:
**syntax:** typo: `heighScale` should be `heightScale`

```suggestion
void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real heightScale)
```

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

void doCameraTetherNamed(const AsciiString& unit, Bool snapToUnit, Real play);
void doCameraStopTetherNamed(void);
void doCameraSetDefault(Real pitch, Real angle, Real maxHeight);
void doCameraSetDefault(Real pitch, Real angle, Real heighScale);
Copy link

Choose a reason for hiding this comment

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

syntax: typo: heighScale should be heightScale

Suggested change
void doCameraSetDefault(Real pitch, Real angle, Real heighScale);
void doCameraSetDefault(Real pitch, Real angle, Real heightScale);
Prompt To Fix With AI
This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Include/GameLogic/ScriptActions.h
Line: 114:114

Comment:
**syntax:** typo: `heighScale` should be `heightScale`

```suggestion
	void doCameraSetDefault(Real pitch, Real angle, Real heightScale);
```

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

@Mauller Mauller force-pushed the fix-default-camera-height branch from a0b7db7 to 22e6c6b Compare January 28, 2026 18:25
@Mauller Mauller force-pushed the fix-default-camera-height branch from 22e6c6b to 6ea3918 Compare February 9, 2026 17:57
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

10 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@Mauller Mauller force-pushed the fix-default-camera-height branch from 6ea3918 to 09d7a25 Compare February 12, 2026 17:58
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

10 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

TheTacticalView->setHeight( TheDisplay->getHeight() );
}
TheTacticalView->setDefaultView(0.0f, 0.0f, 1.0f);
TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();
Copy link

Choose a reason for hiding this comment

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

Missing setZoomToMax() call after setCameraHeightAboveGroundLimitsToDefault(). Similar to GameLogic::startNewGame() and OptionsMenu::saveOptions(), this should call setZoomToMax() to actually set the camera height.

Suggested change
TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();
TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();
TheTacticalView->setZoomToMax();
Prompt To Fix With AI
This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp
Line: 1380:1380

Comment:
Missing `setZoomToMax()` call after `setCameraHeightAboveGroundLimitsToDefault()`. Similar to `GameLogic::startNewGame()` and `OptionsMenu::saveOptions()`, this should call `setZoomToMax()` to actually set the camera height.

```suggestion
	TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();
	TheTacticalView->setZoomToMax();
```

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

TheControlBar->reset();

TheTacticalView->setDefaultView(0.0f, 0.0f, 1.0f);
TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();
Copy link

Choose a reason for hiding this comment

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

Missing setZoomToMax() call after setCameraHeightAboveGroundLimitsToDefault(). Without this, the camera zoom won't be properly initialized during reset.

Suggested change
TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();
TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();
TheTacticalView->setZoomToMax();
Prompt To Fix With AI
This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp
Line: 2150:2150

Comment:
Missing `setZoomToMax()` call after `setCameraHeightAboveGroundLimitsToDefault()`. Without this, the camera zoom won't be properly initialized during reset.

```suggestion
	TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();
	TheTacticalView->setZoomToMax();
```

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

Comment on lines +4607 to +4608
TheTacticalView->setPitch(pitch);
TheTacticalView->setAngle(angle);
Copy link

Choose a reason for hiding this comment

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

setPitch/setAngle unexpectedly stop scripted cameras

Both W3DView::setPitch and W3DView::setAngle call stopDoingScriptedCamera() as a side effect. The original setDefaultView only updated m_defaultPitch and m_maxHeightAboveGround without touching scripted camera state at all.

This means that any call to doCameraSetDefault from a script while a scripted camera animation is running will silently terminate that animation — a behavioural change not present before this PR. The function name implies updating defaults, not interrupting live camera paths.

Additionally, the old code stored the pitch in m_defaultPitch, so a later setPitchToDefault() would correctly restore it. The new setPitch() call only writes to m_pitch (the current pitch), leaving m_defaultPitch unchanged. If the engine resets to default pitch elsewhere, the value set by this script action will be lost.

Consider using a method that updates the defaults without calling stopDoingScriptedCamera(), or explicitly documenting the intended behavioural change if stopping the scripted camera is desired.

Prompt To Fix With AI
This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp
Line: 4607-4608

Comment:
**`setPitch`/`setAngle` unexpectedly stop scripted cameras**

Both `W3DView::setPitch` and `W3DView::setAngle` call `stopDoingScriptedCamera()` as a side effect. The original `setDefaultView` only updated `m_defaultPitch` and `m_maxHeightAboveGround` without touching scripted camera state at all.

This means that any call to `doCameraSetDefault` from a script while a scripted camera animation is running will silently terminate that animation — a behavioural change not present before this PR. The function name implies updating defaults, not interrupting live camera paths.

Additionally, the old code stored the pitch in `m_defaultPitch`, so a later `setPitchToDefault()` would correctly restore it. The new `setPitch()` call only writes to `m_pitch` (the *current* pitch), leaving `m_defaultPitch` unchanged. If the engine resets to default pitch elsewhere, the value set by this script action will be lost.

Consider using a method that updates the defaults without calling `stopDoingScriptedCamera()`, or explicitly documenting the intended behavioural change if stopping the scripted camera is desired.

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

@Mauller
Copy link
Author

Mauller commented Mar 10, 2026

This is only updating at times since i use this PR for the Legi build

@Mauller Mauller force-pushed the fix-default-camera-height branch from 27aa725 to f0dc49b Compare March 22, 2026 08:39
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

Development

Successfully merging this pull request may close these issues.

Camera height does not scale properly along with aspect ratio of game resolution

5 participants