Skip to content

Commit 47aee48

Browse files
feat: AspectRatioOverride enum — Phase 2 of Screen Scaling standardization
- Add AspectRatioOverride enum to PVPrimitives (auto/4:3/16:9/1:1/8:7/stretch) with displayName, subtitle, symbolName, aspectRatioValue, and isWidescreen metadata - Extend CoreOptional protocol with supportedAspectRatioOverrides and preferredAspectRatioOverride; default impls return .auto (no-op for all existing cores) - Enable scalingModeRenderer feature flag by default (was: disabled, gated behind flag) - Add AspectRatioOverrideTests covering raw values, CaseIterable, Codable, and display metadata - Update reviewer-context.md with ScalingMode + AspectRatioOverride review rules Part of #2673 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent bc1f011 commit 47aee48

File tree

6 files changed

+272
-2
lines changed

6 files changed

+272
-2
lines changed

.changelog/3619.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
### Added
2+
- **AspectRatioOverride enum** — New `AspectRatioOverride` enum in `PVPrimitives` defines per-core aspect ratio overrides (Auto, 4:3, 16:9, 1:1, 8:7, Stretch) with display metadata and aspect ratio values.
3+
- **CoreOptional aspect ratio protocol**`CoreOptional` now declares `supportedAspectRatioOverrides` and `preferredAspectRatioOverride` static properties; default implementations return `.auto` so all existing cores are unaffected.
4+
5+
### Changed
6+
- **ScalingMode renderer enabled by default** — The `scalingModeRenderer` feature flag is now enabled by default; the new `ScalingMode`-driven renderer paths (Stretch, Aspect Fill, Integer Scale, Native Resolution) are active for all users. Legacy boolean shims remain for backwards compatibility.

.github/prompts/reviewer-context.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,16 @@ will fail Linux CI — flag as 🟡 MINOR if in a Tier 0–2 module, ⚪ NIT oth
418418
- **Flag 🟠 MAJOR** if new DSU code modifies `listener`/`browser` state outside the `self.queue` serial queue in the Discovery classes.
419419
- **Flag 🟠 MAJOR** if `DSUSocket.startListening()` is inlined into `init` — the two-step design is load-bearing.
420420

421+
### Screen Scaling — ScalingMode + AspectRatioOverride (Phase 1 #3616 / Phase 2 #3617)
422+
- `ScalingMode` enum is in `PVSettings/Sources/PVSettings/Settings/Model/ScalingMode.swift` — replaces the legacy `nativeScaleEnabled` / `integerScaleEnabled` boolean pair. The old booleans are `@available(*, deprecated)` shims.
423+
- `AspectRatioOverride` enum is in `PVPrimitives/Sources/PVPrimitives/System/AspectRatioOverride.swift` — per-core display geometry override. Default `.auto` means no override.
424+
- `CoreOptional.supportedAspectRatioOverrides` — static var declaring which `AspectRatioOverride` cases the core supports. Default: `[.auto]`.
425+
- `CoreOptional.preferredAspectRatioOverride` — static var returning the currently selected override. Default: `.auto`.
426+
- The `scalingModeRenderer` feature flag in `PVFeatureFlags` is **enabled by default** as of Phase 2. The renderer uses `ScalingMode` for layout; **do not** re-introduce the old boolean code paths.
427+
- **Flag 🔴 CRITICAL** if new code compares against `nativeScaleEnabled`/`integerScaleEnabled` directly instead of reading `scalingMode`.
428+
- **Flag 🟠 MAJOR** if a new core overrides `preferredAspectRatioOverride` without also declaring the case in `supportedAspectRatioOverrides`.
429+
- **Flag 🟡 MINOR** if a core's `supportedAspectRatioOverrides` includes `.ratio_16_9` but no widescreen option key is wired in the core options.
430+
421431
## GitHub Workflow Awareness
422432

423433
Reviewers should be aware of — but NOT flag as code issues — the following:

PVCoreBridge/Sources/PVCoreBridge/CoreOptions/Protocols/CoreOptional.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//
88

99
import Foundation
10+
import PVPrimitives
1011

1112

1213
public protocol CoreOptional {//where Self: EmulatorCoreIOInterface {
@@ -20,6 +21,24 @@ public protocol CoreOptional {//where Self: EmulatorCoreIOInterface {
2021
/// Set this when a game is loaded and clear it on unload.
2122
static var currentGameMD5: String? { get }
2223

24+
/// The set of aspect ratio overrides this core supports.
25+
///
26+
/// Cores that support widescreen hacks, aspect ratio selection, or other
27+
/// display geometry modifications should return the relevant cases here.
28+
/// The UI uses this list to show only the options the core actually supports.
29+
///
30+
/// The default implementation returns `[.auto]` (no overrides available).
31+
static var supportedAspectRatioOverrides: [AspectRatioOverride] { get }
32+
33+
/// The aspect ratio override currently selected for this core.
34+
///
35+
/// The renderer reads this property when compositing the frame. Returning
36+
/// `.auto` (the default) means the core's native reported aspect ratio is used.
37+
///
38+
/// Cores that store this preference in their own `CoreOption` key should
39+
/// override this property to read from `UserDefaults` via `string(forOption:)`.
40+
static var preferredAspectRatioOverride: AspectRatioOverride { get }
41+
2342
// static func bool(forOption option: String) -> Bool
2443
// static func int(forOption option: String) -> Int
2544
// static func float(forOption option: String) -> Float
@@ -40,6 +59,12 @@ public extension CoreOptional {
4059
/// Default implementation: no per-game MD5 override.
4160
static var currentGameMD5: String? { nil }
4261

62+
/// Default: only `.auto` is available (no override supported).
63+
static var supportedAspectRatioOverrides: [AspectRatioOverride] { [.auto] }
64+
65+
/// Default: use the core's natural aspect ratio.
66+
static var preferredAspectRatioOverride: AspectRatioOverride { .auto }
67+
4368
/// Reset a specific set of options to their default values
4469
/// - Parameter options: The options to reset
4570
static func resetOptions(_ options: [CoreOption]) {

PVFeatureFlags/Sources/PVFeatureFlags/PVFeatureFlags.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,8 @@ public struct FeatureFlag: Codable, Sendable {
305305
)
306306

307307
public static let scalingModeRenderer = FeatureFlag(
308-
enabled: false,
309-
description: "Activates the ScalingMode-enum renderer paths (Stretch, Aspect Fill, Integer Scale, Native Resolution). When disabled the legacy nativeScaleEnabled/integerScaleEnabled boolean code paths are used — existing scaling is unchanged. Enable in Settings > Advanced > Feature Flags to test new modes (Phase 2 of screen-scaling standardisation)."
308+
enabled: true,
309+
description: "Activates the ScalingMode-enum renderer paths (Stretch, Aspect Fill, Integer Scale, Native Resolution). Enabled by default as of Phase 2. The legacy nativeScaleEnabled/integerScaleEnabled boolean shims remain for backwards compatibility."
310310
)
311311
}
312312

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
//
2+
// AspectRatioOverride.swift
3+
// PVPrimitives
4+
//
5+
// Created by Provenance Emu on 2026-04-03.
6+
// Copyright © 2026 Provenance Emu. All rights reserved.
7+
//
8+
9+
import Foundation
10+
11+
/// A per-core aspect ratio override that can be applied on top of the global `ScalingMode`.
12+
///
13+
/// Each emulator core may support a subset of these overrides depending on the capabilities
14+
/// of the underlying emulator. Cores declare which overrides they support via the
15+
/// `CoreOptional.supportedAspectRatioOverrides` property.
16+
///
17+
/// When a core returns a non-`.auto` value from `CoreOptional.preferredAspectRatioOverride`,
18+
/// the renderer applies that override instead of (or in combination with) the global
19+
/// `ScalingMode` setting.
20+
///
21+
/// ## Override vs ScalingMode
22+
/// | `ScalingMode` | `AspectRatioOverride` | Result |
23+
/// |---|---|---|
24+
/// | `.aspectFit` | `.ratio_16_9` | Widescreen content letterboxed to fit |
25+
/// | `.aspectFill` | `.ratio_4_3` | 4:3 content zoomed to fill screen |
26+
/// | `.stretch` | `.auto` | Core's native ratio stretched to fill |
27+
/// | Any | `.ratio_1_1` | Square-pixel / pixel-perfect mapping |
28+
public enum AspectRatioOverride: String, Codable, Equatable, Hashable,
29+
CaseIterable, Sendable, CustomStringConvertible {
30+
31+
/// Let the core report its natural aspect ratio — no override applied.
32+
/// This is the default for all cores.
33+
case auto = "auto"
34+
35+
/// Force a 4:3 display ratio (classic TV / arcade).
36+
case ratio_4_3 = "4:3"
37+
38+
/// Force a 16:9 widescreen display ratio.
39+
case ratio_16_9 = "16:9"
40+
41+
/// Force a 1:1 square-pixel ratio (equal horizontal and vertical scaling).
42+
case ratio_1_1 = "1:1"
43+
44+
/// Force a 8:7 ratio used by the SNES / Super Famicom (non-square pixels).
45+
case ratio_8_7 = "8:7"
46+
47+
/// Stretch to fill the display, ignoring all aspect ratio constraints.
48+
/// Equivalent to `ScalingMode.stretch` but applied at the per-core level.
49+
case stretch = "stretch"
50+
51+
// MARK: - Display Metadata
52+
53+
public var displayName: String {
54+
switch self {
55+
case .auto: return "Auto"
56+
case .ratio_4_3: return "4:3"
57+
case .ratio_16_9: return "16:9"
58+
case .ratio_1_1: return "1:1"
59+
case .ratio_8_7: return "8:7"
60+
case .stretch: return "Stretch"
61+
}
62+
}
63+
64+
public var subtitle: String {
65+
switch self {
66+
case .auto:
67+
return "Use the aspect ratio reported by the core"
68+
case .ratio_4_3:
69+
return "Classic TV / arcade (4 wide, 3 tall)"
70+
case .ratio_16_9:
71+
return "Widescreen — enables widescreen hack if supported"
72+
case .ratio_1_1:
73+
return "Square pixels — 1:1 horizontal/vertical mapping"
74+
case .ratio_8_7:
75+
return "SNES native — non-square pixel correction"
76+
case .stretch:
77+
return "Stretch to fill — ignores aspect ratio"
78+
}
79+
}
80+
81+
public var symbolName: String {
82+
switch self {
83+
case .auto: return "aspectratio"
84+
case .ratio_4_3: return "tv"
85+
case .ratio_16_9: return "tv.fill"
86+
case .ratio_1_1: return "square"
87+
case .ratio_8_7: return "rectangle"
88+
case .stretch: return "arrow.left.and.right.righttriangle.left.righttriangle.right"
89+
}
90+
}
91+
92+
/// The floating-point aspect ratio value, or `nil` for `.auto` and `.stretch`.
93+
public var aspectRatioValue: Float? {
94+
switch self {
95+
case .auto: return nil
96+
case .ratio_4_3: return 4.0 / 3.0
97+
case .ratio_16_9: return 16.0 / 9.0
98+
case .ratio_1_1: return 1.0
99+
case .ratio_8_7: return 8.0 / 7.0
100+
case .stretch: return nil
101+
}
102+
}
103+
104+
/// Whether this override requests widescreen rendering (16:9 or wider).
105+
public var isWidescreen: Bool {
106+
switch self {
107+
case .ratio_16_9: return true
108+
default: return false
109+
}
110+
}
111+
112+
public var description: String { displayName }
113+
}
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
// AspectRatioOverrideTests.swift
2+
// PVPrimitivesTests
3+
//
4+
// Created by Provenance Emu on 2026-04-03.
5+
// Copyright © 2026 Provenance Emu. All rights reserved.
6+
//
7+
8+
import XCTest
9+
@testable import PVPrimitives
10+
11+
final class AspectRatioOverrideTests: XCTestCase {
12+
13+
// MARK: - Raw values
14+
15+
func testAutoRawValue() {
16+
XCTAssertEqual(AspectRatioOverride.auto.rawValue, "auto")
17+
}
18+
19+
func testRatio4_3RawValue() {
20+
XCTAssertEqual(AspectRatioOverride.ratio_4_3.rawValue, "4:3")
21+
}
22+
23+
func testRatio16_9RawValue() {
24+
XCTAssertEqual(AspectRatioOverride.ratio_16_9.rawValue, "16:9")
25+
}
26+
27+
func testRatio1_1RawValue() {
28+
XCTAssertEqual(AspectRatioOverride.ratio_1_1.rawValue, "1:1")
29+
}
30+
31+
func testRatio8_7RawValue() {
32+
XCTAssertEqual(AspectRatioOverride.ratio_8_7.rawValue, "8:7")
33+
}
34+
35+
func testStretchRawValue() {
36+
XCTAssertEqual(AspectRatioOverride.stretch.rawValue, "stretch")
37+
}
38+
39+
// MARK: - CaseIterable
40+
41+
func testAllCasesCount() {
42+
XCTAssertEqual(AspectRatioOverride.allCases.count, 6)
43+
}
44+
45+
func testAllCasesContainsAuto() {
46+
XCTAssertTrue(AspectRatioOverride.allCases.contains(.auto))
47+
}
48+
49+
// MARK: - aspectRatioValue
50+
51+
func testAutoAspectRatioValueIsNil() {
52+
XCTAssertNil(AspectRatioOverride.auto.aspectRatioValue)
53+
}
54+
55+
func testStretchAspectRatioValueIsNil() {
56+
XCTAssertNil(AspectRatioOverride.stretch.aspectRatioValue)
57+
}
58+
59+
func testRatio4_3Value() {
60+
XCTAssertEqual(AspectRatioOverride.ratio_4_3.aspectRatioValue, 4.0 / 3.0, accuracy: 0.0001)
61+
}
62+
63+
func testRatio16_9Value() {
64+
XCTAssertEqual(AspectRatioOverride.ratio_16_9.aspectRatioValue, 16.0 / 9.0, accuracy: 0.0001)
65+
}
66+
67+
func testRatio1_1Value() {
68+
XCTAssertEqual(AspectRatioOverride.ratio_1_1.aspectRatioValue, 1.0, accuracy: 0.0001)
69+
}
70+
71+
func testRatio8_7Value() {
72+
XCTAssertEqual(AspectRatioOverride.ratio_8_7.aspectRatioValue, 8.0 / 7.0, accuracy: 0.0001)
73+
}
74+
75+
// MARK: - isWidescreen
76+
77+
func testOnly16_9IsWidescreen() {
78+
for override in AspectRatioOverride.allCases {
79+
if override == .ratio_16_9 {
80+
XCTAssertTrue(override.isWidescreen, "\(override) should be widescreen")
81+
} else {
82+
XCTAssertFalse(override.isWidescreen, "\(override) should not be widescreen")
83+
}
84+
}
85+
}
86+
87+
// MARK: - Codable round-trip
88+
89+
func testCodableRoundTrip() throws {
90+
let encoder = JSONEncoder()
91+
let decoder = JSONDecoder()
92+
for override in AspectRatioOverride.allCases {
93+
let data = try encoder.encode(override)
94+
let decoded = try decoder.decode(AspectRatioOverride.self, from: data)
95+
XCTAssertEqual(decoded, override, "Codable round-trip failed for \(override)")
96+
}
97+
}
98+
99+
// MARK: - displayName
100+
101+
func testAutoDisplayName() {
102+
XCTAssertEqual(AspectRatioOverride.auto.displayName, "Auto")
103+
}
104+
105+
func testStretchDisplayName() {
106+
XCTAssertEqual(AspectRatioOverride.stretch.displayName, "Stretch")
107+
}
108+
109+
// MARK: - description
110+
111+
func testDescriptionMatchesDisplayName() {
112+
for override in AspectRatioOverride.allCases {
113+
XCTAssertEqual(override.description, override.displayName)
114+
}
115+
}
116+
}

0 commit comments

Comments
 (0)