Skip to content

Conversation

@rbruels
Copy link

@rbruels rbruels commented Dec 13, 2025

This branch adds a new [somewhat experimental] fullscreen display mode that supports arbitrary screen resolutions, similar to Mac.

Instead of being limited to fixed Newton screen dimensions, the emulator can now scale to fill the entire iOS device screen while maintaining proper aspect ratio and coordinate mapping between the native display and the Newton's internal resolution. Both iPhone and iPad use an arbitrary (vibe check) scaling factor to make the emulator more human-useable on Retina screens.

This is only enabled if the user switches on "Use Full Screen" in the Settings app.

Enabling in Settings

Simulator Screenshot - iPhone 17 Pro Max - 2025-12-13 at 15 53 42

Classic Mode

Simulator Screenshot - iPhone 17 Pro Max - 2025-12-13 at 12 33 57 Simulator Screenshot - iPad Pro 13-inch (M5) - 2025-12-13 at 12 36 03

Fullscreen Mode

Simulator Screenshot - iPhone 17 Pro Max - 2025-12-13 at 13 03 18 Simulator Screenshot - iPad Pro 13-inch (M5) - 2025-12-13 at 12 35 34

Summary by CodeRabbit

  • New Features

    • Introduced "Use Full Screen" setting that automatically scales the Newton emulation to fill your device's screen with portrait orientation support
  • Chores

    • Updated application layout and view constraints
    • Refined display rendering system to support multiple display modes

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 13, 2025

Walkthrough

This pull request adds a full-screen rendering mode for the Newton emulator. The feature dynamically scales the emulator display to fill device bounds rather than using fixed resolutions. It includes settings management, view layout adjustments, resolution computation based on actual bounds, and conditional rendering logic with environment-specific background handling.

Changes

Cohort / File(s) Summary
Screen Manager
Emulator/Screen/TIOSScreenManager.h
Added public accessor GetImageBuffer() to retrieve image buffer pointer.
Settings & Defaults
app/iEinstein/Classes/iEinsteinAppDelegate.mm, app/iEinstein/Settings.bundle/Root.plist
Introduced use_full_screen preference with default value false and added UI toggle switch in Settings with descriptive footer text.
View Layer
app/iEinstein/Classes/iEinsteinView.h
Added useFullScreen member variable and updated setScreenManager: method signature to accept useFullScreen:(BOOL)fullScreen parameter.
View Rendering
app/iEinstein/Classes/iEinsteinView.mm
Implemented full-screen rendering mode with new drawBackgroundInContext:withBounds: helper method. Added conditional logic in drawRect: to compute padded screen rect and apply environment-specific background colors (green for classic, black for full-screen). Updated setScreenManager:useFullScreen: to store full-screen flag.
View Controller
app/iEinstein/Classes/iEinsteinViewController.h, app/iEinstein/Classes/iEinsteinViewController.mm
Moved emulator initialization from viewWillAppear to viewDidLayoutSubviews with one-time guard. Rewrote resolution logic to compute from view bounds when use_full_screen is enabled, apply portrait orientation and platform-specific scaling. Added settings-change detection to trigger emulator reset when full-screen mode or resolution changes.
UI Layout
app/iEinstein/Main.storyboard, Resources/iOSEinstein Storyboards-Info.plist
Updated storyboard with safe area guides, adjusted root view and subview frames (600×600 to 393×852 and 393×666 respectively), modified constraints to use safe area anchors, and updated color calibration. Reordered interface orientations in plist.

Sequence Diagram

sequenceDiagram
    participant App as App Lifecycle
    participant VC as ViewController
    participant Settings as Settings/Prefs
    participant View as View
    participant SM as ScreenManager
    participant Render as Rendering

    App->>VC: viewDidLayoutSubviews
    VC->>Settings: Read use_full_screen, screen_resolution
    alt use_full_screen == true
        VC->>VC: Compute resolution from bounds<br/>Apply portrait orientation<br/>Round width to even
    else use_full_screen == false
        VC->>VC: Use LUT-based classic resolution
    end
    VC->>VC: Create/initialize mScreenManager
    VC->>View: setScreenManager:useFullScreen:
    View->>View: Store useFullScreen flag
    
    Note over View,Render: Rendering Loop
    App->>View: drawRect:
    View->>View: drawBackgroundInContext:
    alt useFullScreen == true
        View->>Render: Compute padded screenImageRect<br/>Draw black background
    else useFullScreen == false
        View->>Render: Use classic letterbox logic<br/>Draw green background
    end
    View->>SM: Get image buffer
    Render->>Render: Draw Newton screen image
    
    Note over VC,Settings: Settings Change Detection
    App->>VC: startEmulator (on resume)
    VC->>Settings: Check current use_full_screen<br/>Check current screen_resolution
    VC->>VC: Compare with lastKnown*
    alt Settings changed
        VC->>VC: resetEmulator
        VC->>VC: Update lastKnown* cache
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • iEinsteinViewController.mm: Initialization timing change with guard logic, resolution computation with platform-specific scaling and constraints, settings-change detection with conditional reset—requires careful review of state management and timing dependencies.
  • iEinsteinView.mm: New branching logic in drawRect: for full-screen vs. classic rendering modes, background color selection based on mode, bounds calculations with flooring—verify correctness of coordinate transformations.
  • Main.storyboard: Frame size and constraint changes from fixed 600×600 to 393×852—validate layout for various device sizes and orientations.
  • Resolution computation logic: Width rounding to even value, portrait enforcement, platform-specific scaling factors—confirm correctness for target devices.
  • Integration points: setScreenManager:useFullScreen: call from ViewController to View and subsequent use in rendering—ensure proper state propagation.

Poem

🐰 Hops with glee

A Newton screen now fills the view,
No more tiny portrait too!
Full-screen bounds and proper math—
The emulator walks a new path.
Settings dance, rendering dreams,
Scaling beautifully, so it seems! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Support full-screen mode on iOS' accurately captures the primary change in the PR, which introduces full-screen display functionality for the iOS emulator across multiple files and systems.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (4)
app/iEinstein/Classes/iEinsteinAppDelegate.mm (1)

39-44: Defaults registration for use_full_screen looks correct; consider modern literals for consistency/readability.
Optional cleanup:

- NSDictionary* defaults = [NSDictionary dictionaryWithObjectsAndKeys:
-                           [NSNumber numberWithInt:0], @"screen_resolution",
-                           [NSNumber numberWithBool:NO], @"use_full_screen",
-                           [NSNumber numberWithBool:NO], @"clear_flash_ram",
-                           @"{8,9.88897}", @"printable_size",
-                           nil];
+ NSDictionary* defaults = [NSDictionary dictionaryWithObjectsAndKeys:
+                           @0, @"screen_resolution",
+                           @NO, @"use_full_screen",
+                           @NO, @"clear_flash_ram",
+                           @"{8,9.88897}", @"printable_size",
+                           nil];
app/iEinstein/Settings.bundle/Root.plist (1)

30-32: Label change to “Screen Resolution” is fine—consider localization parity if you localize Settings.bundle.

app/iEinstein/Classes/iEinsteinView.h (1)

40-46: Prefer consistent boolean type (BOOL vs bool) across the Obj-C API surface.
Since the selector takes a BOOL, consider making the ivar BOOL useFullScreen; too (or change the parameter to bool if everything is Obj-C++), to avoid subtle conversions and mixed conventions.

-    bool useFullScreen;
+    BOOL useFullScreen;
app/iEinstein/Classes/iEinsteinViewController.mm (1)

368-379: Make fullscreen “vibe check” scaling explicit (avoid float-to-int surprises).
Right now int /= 2.0 / int /= 1.25 silently truncates; if that’s intended, consider rounding explicitly to reduce device-to-device jitter.

-    if (UI_USER_INTERFACE_IDIOM() == UIUserInterfaceIdiomPad) {
-      newtonScreenWidth /= 2.0;
-      newtonScreenHeight /= 2.0;
-    } else {
-      newtonScreenWidth /= 1.25;
-      newtonScreenHeight /= 1.25;
-    }
+    const CGFloat scale = (UI_USER_INTERFACE_IDIOM() == UIUserInterfaceIdiomPad) ? 0.5 : (1.0 / 1.25);
+    newtonScreenWidth  = (int)lround((CGFloat)newtonScreenWidth  * scale);
+    newtonScreenHeight = (int)lround((CGFloat)newtonScreenHeight * scale);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a29e0a0 and b4c5f20.

📒 Files selected for processing (9)
  • Emulator/Screen/TIOSScreenManager.h (1 hunks)
  • Resources/iOSEinstein Storyboards-Info.plist (1 hunks)
  • app/iEinstein/Classes/iEinsteinAppDelegate.mm (1 hunks)
  • app/iEinstein/Classes/iEinsteinView.h (1 hunks)
  • app/iEinstein/Classes/iEinsteinView.mm (3 hunks)
  • app/iEinstein/Classes/iEinsteinViewController.h (1 hunks)
  • app/iEinstein/Classes/iEinsteinViewController.mm (4 hunks)
  • app/iEinstein/Main.storyboard (1 hunks)
  • app/iEinstein/Settings.bundle/Root.plist (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (6)
Resources/iOSEinstein Storyboards-Info.plist (1)

44-50: Orientation order change can affect initial/preferred orientation—please sanity-check launch behavior.
Even though the supported set is unchanged, putting landscape first may influence which orientation the app prefers on launch/presentation on some devices/flows.

app/iEinstein/Classes/iEinsteinViewController.h (1)

40-52: Ensure new state ivars are explicitly initialized and kept in sync with preference changes.
mEmulatorInitialized / lastKnownFullScreenMode should be set deterministically (e.g., in init/viewDidLoad) to avoid relying on zeroed memory or differing init paths under storyboards.

app/iEinstein/Settings.bundle/Root.plist (1)

11-26: Settings specifier wiring looks consistent (use_full_screen + default false).
Main thing to confirm is whether fullscreen changes are picked up live or require an app relaunch (Settings.bundle commonly requires relaunch unless you observe defaults changes).

app/iEinstein/Main.storyboard (1)

1-31: Please verify “fullscreen” intent vs safe-area pinning + background bleed-through in letterbox paths.
Pinning iEinsteinView to the safe area (and making it transparent) is great for avoiding the notch/home-indicator, but it can also prevent truly edge-to-edge rendering and may reveal the root background color in any path that doesn’t paint the full view (classic/letterboxed).

app/iEinstein/Classes/iEinsteinView.mm (2)

83-88: Setter update looks fine; consider nulling/clearing cached rect on mode change.
Storing useFullScreen + setNeedsDisplay is good; if you ever allow toggling without full emulator reset, you may also want to clear mScreenImage / recompute screenImageRect.


206-231: Background helper is a good extraction.
Centralizing classic vs fullscreen background + superview background alignment is clean and makes safe-area behavior easier to reason about.

Comment on lines 122 to 128
if (mScreenManager == NULL)
{
// Just fill black
// Fill with black when no screen manager
CGFloat black[] = { 0.0, 0.0, 0.0, 1.0 };
CGRect frame = [self frame];
CGContextSetFillColor(theContext, black);
CGContextFillRect(theContext, frame);
CGContextFillRect(theContext, bounds);
} else
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Also set superview background when mScreenManager == NULL.
Right now you fill self black, but safe areas outside this view can keep the previous background until a later draw.

   if (mScreenManager == NULL)
   {
+    if (self.superview) self.superview.backgroundColor = [UIColor blackColor];
     // Fill with black when no screen manager
     CGFloat black[] = { 0.0, 0.0, 0.0, 1.0 };
     CGContextSetFillColor(theContext, black);
     CGContextFillRect(theContext, bounds);
   }
🤖 Prompt for AI Agents
In app/iEinstein/Classes/iEinsteinView.mm around lines 122 to 128, when
mScreenManager == NULL you only fill this view black but leave the superview’s
safe-area/background unchanged; update the code to also set the superview’s
background to black (check superview != nil) — e.g. set
superview.backgroundColor to black (or its equivalent NSColor/UIColor), then
fill this view as before and optionally call setNeedsDisplay on the superview so
the change renders immediately.

Comment on lines +148 to 195
if (useFullScreen) {
// Full screen mode: leave room for border on all sides
// On iPad with 2x scaling, double the padding to maintain proportions
CGFloat borderPadding = (UI_USER_INTERFACE_IDIOM() == UIUserInterfaceIdiomPad) ? 30.0 : 15.0;
screenImageRect = CGRectInset(bounds, borderPadding, borderPadding);
} else {
// Classic mode: letterbox/center the Newton screen with integer scaling
CGRect screenBounds = bounds;
CGRect r = bounds;

if (screenBounds.size.width > newtonScreenWidth && screenBounds.size.height > newtonScreenHeight)
{
// Newton screen resolution is square (like 320x320)

int mod = (int) screenBounds.size.width % newtonScreenWidth;
r.size.width -= mod;
r.size.height = r.size.width;
} else
{
// Newton screen resolution is rectangular (like 320x480)

int wmod = (int) r.size.width % newtonScreenWidth;
int hmod = (int) r.size.height % newtonScreenHeight;

if (wmod > hmod)
if (newtonScreenWidth == newtonScreenHeight)
{
r.size.width -= wmod;

int scale = (int) r.size.width / newtonScreenWidth;
r.size.height = newtonScreenHeight * scale;
// Newton screen resolution is square (like 320x320)
int mod = (int) screenBounds.size.width % newtonScreenWidth;
r.size.width -= mod;
r.size.height = r.size.width;
} else
{
r.size.height -= hmod;

int scale = (int) r.size.height / newtonScreenHeight;
r.size.width = newtonScreenWidth * scale;
// Newton screen resolution is rectangular (like 320x480)
int wmod = (int) r.size.width % newtonScreenWidth;
int hmod = (int) r.size.height % newtonScreenHeight;

if (wmod > hmod)
{
r.size.width -= wmod;
int scale = (int) r.size.width / newtonScreenWidth;
r.size.height = newtonScreenHeight * scale;
} else
{
r.size.height -= hmod;
int scale = (int) r.size.height / newtonScreenHeight;
r.size.width = newtonScreenWidth * scale;
}
}
}

// Center image on screen
// Center image on screen
r.origin.x += (screenBounds.size.width - r.size.width) / 2;
r.origin.y += (screenBounds.size.height - r.size.height) / 2;
}

r.origin.x += (screenBounds.size.width - r.size.width) / 2;
r.origin.y += (screenBounds.size.height - r.size.height) / 2;
screenImageRect = r;
screenImageRect.origin.x = floor(screenImageRect.origin.x);
screenImageRect.origin.y = floor(screenImageRect.origin.y);
screenImageRect.size.width = floor(screenImageRect.size.width);
screenImageRect.size.height = floor(screenImageRect.size.height);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fullscreen rect should preserve Newton aspect ratio (avoid subtle stretching).
screenImageRect = CGRectInset(bounds, …) can distort if screenImageRect ratio doesn’t match newtonScreenWidth/newtonScreenHeight (padding changes ratio; and any future orientation/buffer change will too).

-      CGFloat borderPadding = (UI_USER_INTERFACE_IDIOM() == UIUserInterfaceIdiomPad) ? 30.0 : 15.0;
-      screenImageRect = CGRectInset(bounds, borderPadding, borderPadding);
+      CGFloat borderPadding = (UI_USER_INTERFACE_IDIOM() == UIUserInterfaceIdiomPad) ? 30.0 : 15.0;
+      CGRect avail = CGRectInset(bounds, borderPadding, borderPadding);
+      const CGFloat sx = avail.size.width  / (CGFloat)newtonScreenWidth;
+      const CGFloat sy = avail.size.height / (CGFloat)newtonScreenHeight;
+      const CGFloat s = MIN(sx, sy); // aspect-fit
+      CGSize sz = CGSizeMake(floor(newtonScreenWidth * s), floor(newtonScreenHeight * s));
+      screenImageRect = CGRectMake(
+        floor(avail.origin.x + (avail.size.width  - sz.width)  / 2.0),
+        floor(avail.origin.y + (avail.size.height - sz.height) / 2.0),
+        sz.width, sz.height
+      );
🤖 Prompt for AI Agents
In app/iEinstein/Classes/iEinsteinView.mm around lines 148 to 195, the
fullscreen branch currently uses CGRectInset(bounds, ...) which can change the
aspect ratio and subtly stretch the Newton image; instead compute the available
area as the inset bounds, then fit a rect into that area that preserves the
Newton aspect ratio (newtonScreenWidth/newtonScreenHeight): calculate target
width/height by comparing availableAspect to newtonAspect, choose width-limited
or height-limited scaling, floor the resulting origin/size to integers, center
the rect in the available area, and assign that to screenImageRect so fullscreen
never distorts the Newton aspect ratio.

Comment on lines 44 to +65
- (void)viewWillAppear:(BOOL)animated
{
[super viewWillAppear:animated];
#ifdef USE_STORYBOARDS
[self initEmulator];
#endif
// Initialization moved to viewDidLayoutSubviews to ensure correct bounds
}

- (void)viewDidAppear:(BOOL)animated
{
[super viewDidAppear:animated];
}

- (void)viewDidLayoutSubviews
{
[super viewDidLayoutSubviews];
#ifdef USE_STORYBOARDS
// Only initialize once, after layout is complete so we have correct bounds
if (!mEmulatorInitialized) {
mEmulatorInitialized = YES;
[self initEmulator];
}
#endif
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix init regression for non-storyboard builds (emulator may never initialize).
-viewDidLayoutSubviews only calls initEmulator under #ifdef USE_STORYBOARDS, while -viewWillAppear: no longer initializes. If USE_STORYBOARDS is off, startup can skip initialization entirely.

- (void)viewDidLayoutSubviews
+ (void)viewDidLayoutSubviews
 {
   [super viewDidLayoutSubviews];
-#ifdef USE_STORYBOARDS
   // Only initialize once, after layout is complete so we have correct bounds
   if (!mEmulatorInitialized) {
     mEmulatorInitialized = YES;
     [self initEmulator];
   }
-#endif
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- (void)viewWillAppear:(BOOL)animated
{
[super viewWillAppear:animated];
#ifdef USE_STORYBOARDS
[self initEmulator];
#endif
// Initialization moved to viewDidLayoutSubviews to ensure correct bounds
}
- (void)viewDidAppear:(BOOL)animated
{
[super viewDidAppear:animated];
}
- (void)viewDidLayoutSubviews
{
[super viewDidLayoutSubviews];
#ifdef USE_STORYBOARDS
// Only initialize once, after layout is complete so we have correct bounds
if (!mEmulatorInitialized) {
mEmulatorInitialized = YES;
[self initEmulator];
}
#endif
}
- (void)viewDidLayoutSubviews
{
[super viewDidLayoutSubviews];
// Only initialize once, after layout is complete so we have correct bounds
if (!mEmulatorInitialized) {
mEmulatorInitialized = YES;
[self initEmulator];
}
}
🤖 Prompt for AI Agents
In app/iEinstein/Classes/iEinsteinViewController.mm around lines 44 to 65, the
emulator initialization was moved to viewDidLayoutSubviews but is wrapped in
#ifdef USE_STORYBOARDS so non-storyboard builds never call initEmulator; restore
a one-time initialization path for non-storyboard builds by calling initEmulator
(guarded by the same mEmulatorInitialized flag) from viewWillAppear when
USE_STORYBOARDS is not defined, or remove the #ifdef so the existing
viewDidLayoutSubviews initialization runs for all builds—ensure the init is only
performed once.

Comment on lines +345 to +392
// Determine screen resolution based on use_full_screen setting
NSUserDefaults* prefs = [NSUserDefaults standardUserDefaults];
BOOL useFullScreen = [(NSNumber*) [prefs objectForKey:@"use_full_screen"] boolValue];
int newtonScreenWidth, newtonScreenHeight;

if (useFullScreen) {
// Full screen mode: use native view bounds for resolution
CGRect viewBounds = [einsteinView bounds];

// Use point dimensions (not pixels) for the Newton resolution.
// The Newton is always in portrait orientation internally (width < height).
int viewWidth = (int)viewBounds.size.width;
int viewHeight = (int)viewBounds.size.height;

// Ensure width is the smaller dimension (portrait mode for Newton)
if (viewWidth < viewHeight) {
newtonScreenWidth = viewWidth;
newtonScreenHeight = viewHeight;
} else {
newtonScreenWidth = viewHeight;
newtonScreenHeight = viewWidth;
}

// On iPad, use 2x scaling (halve resolution, scale up when drawing)
if (UI_USER_INTERFACE_IDIOM() == UIUserInterfaceIdiomPad) {
newtonScreenWidth /= 2.0;
newtonScreenHeight /= 2.0;
} else {
newtonScreenWidth /= 1.25;
newtonScreenHeight /= 1.25;
}

// Round width to an even number (required for pixel operations)
newtonScreenWidth = (newtonScreenWidth + 1) & ~1;
} else {
// Classic mode: use LUT-based resolution from Settings
static int widthLUT[] = { 320, 640, 384, 786, 640, 320, 750, 375, 1080, 540 };
static int heightLUT[] = { 480, 960, 512, 1024, 1136, 568, 1134, 567, 1920, 960 };

int index = [(NSNumber*) [prefs objectForKey:@"screen_resolution"] intValue];
newtonScreenWidth = widthLUT[index];
newtonScreenHeight = heightLUT[index];
}

// Store current settings for change detection
lastKnownFullScreenMode = useFullScreen;
lastKnownScreenResolution = [(NSNumber*) [prefs objectForKey:@"screen_resolution"] intValue];

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Clamp screen_resolution index before LUT access to avoid OOB.
If Settings ever stores an unexpected value, widthLUT[index] / heightLUT[index] can read out of bounds.

-    int index = [(NSNumber*) [prefs objectForKey:@"screen_resolution"] intValue];
-    newtonScreenWidth = widthLUT[index];
-    newtonScreenHeight = heightLUT[index];
+    int index = [(NSNumber*) [prefs objectForKey:@"screen_resolution"] intValue];
+    const int lutSize = (int)(sizeof(widthLUT) / sizeof(widthLUT[0]));
+    if (index < 0) index = 0;
+    if (index >= lutSize) index = lutSize - 1;
+    newtonScreenWidth = widthLUT[index];
+    newtonScreenHeight = heightLUT[index];
🤖 Prompt for AI Agents
In app/iEinstein/Classes/iEinsteinViewController.mm around lines 345 to 392, the
code reads screen_resolution into an index and directly indexes
widthLUT/heightLUT which can go out-of-bounds; clamp the index to the valid
range before LUT access by computing maxIndex =
(sizeof(widthLUT)/sizeof(widthLUT[0])) - 1, read the stored value into an
NSInteger, then set index = MAX(0, MIN(index, maxIndex)); use that clamped index
for widthLUT[index] and heightLUT[index], and store the clamped value in
lastKnownScreenResolution.

Comment on lines +434 to +437
// Check if screen settings have changed since last time
NSUserDefaults* prefs = [NSUserDefaults standardUserDefaults];
[prefs synchronize];

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

NSUserDefaults synchronize deprecated iOS modern alternative

💡 Result:

Short answer: don't call synchronize — it's unnecessary and deprecated. Use UserDefaults (UserDefaults.standard or UserDefaults(suiteName:)) normally; changes are written asynchronously by the system. For cross-device sync use NSUbiquitousKeyValueStore; for shared-app-group storage use UserDefaults(suiteName:). There is no supported API to forcibly flush writes on iOS — design so you don't depend on immediate disk writes. [1][2][3]

Sources:

  • Apple UserDefaults documentation (synchronize: “unnecessary and shouldn't be used”). [1]
  • iOS 12 release notes: “Removed synchronization requirements… these methods will be deprecated”. [2]
  • Guidance / practical details and migration notes. [3]

Remove deprecated [NSUserDefaults synchronize] call.

The synchronize method is deprecated and unnecessary on modern iOS. The system automatically writes changes to NSUserDefaults asynchronously. If you need cross-device synchronization, use NSUbiquitousKeyValueStore; for app-group shared storage, use UserDefaults(suiteName:). Simply remove the synchronize call and rely on normal NSUserDefaults reads.

🤖 Prompt for AI Agents
In app/iEinstein/Classes/iEinsteinViewController.mm around lines 434 to 437,
remove the deprecated call to [NSUserDefaults synchronize]; it is
unnecessary—just read from [NSUserDefaults standardUserDefaults] (or migrate to
NSUbiquitousKeyValueStore or UserDefaults(suiteName:) if cross-device or
app-group sync is required) and delete the synchronize line so the code relies
on the system-managed asynchronous persistence.

Comment on lines +165 to +174
///
/// Accessor to the image buffer.
///
/// \return a pointer to the image buffer.
///
KUInt32*
GetImageBuffer(void)
{
return mImageBuffer;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make GetImageBuffer() contract explicit (lifetime/thread-safety) and prefer read-only access by default.

Returning a raw mutable KUInt32* leaks internal state: callers can accidentally write into the buffer, hold onto it across resizes/power transitions, or race with emulator rendering. At minimum, document ownership/nullability and whether the pointer can change; ideally expose a const getter and add an explicit mutable accessor only if you truly need external writes.

-    KUInt32*
-    GetImageBuffer(void)
-    {
-        return mImageBuffer;
-    }
+    /// Read-only access; pointer is owned by TIOSScreenManager and may become
+    /// invalid after resolution/layout changes or PowerOff.
+    const KUInt32*
+    GetImageBuffer(void) const
+    {
+        return mImageBuffer;
+    }
+
+    /// Mutable access (only if required by the rendering pipeline).
+    KUInt32*
+    GetMutableImageBuffer(void)
+    {
+        return mImageBuffer;
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
///
/// Accessor to the image buffer.
///
/// \return a pointer to the image buffer.
///
KUInt32*
GetImageBuffer(void)
{
return mImageBuffer;
}
/// Read-only access; pointer is owned by TIOSScreenManager and may become
/// invalid after resolution/layout changes or PowerOff.
const KUInt32*
GetImageBuffer(void) const
{
return mImageBuffer;
}
/// Mutable access (only if required by the rendering pipeline).
KUInt32*
GetMutableImageBuffer(void)
{
return mImageBuffer;
}
🤖 Prompt for AI Agents
In Emulator/Screen/TIOSScreenManager.h around lines 165 to 174, the
GetImageBuffer() accessor returns a raw mutable KUInt32* exposing internal
state; change it to return a const KUInt32* by default, add a separate
clearly-named mutable accessor only if external mutation is required, and
document ownership, nullability, lifetime (e.g., “pointer valid until buffer
resize/power state change”) and thread-safety (e.g., caller must hold render
lock or perform copies) in the method comments; if you need to support safe
slices, consider returning a span-like view or a pair {pointer, size} instead of
a bare pointer.

@MatthiasWM
Copy link
Collaborator

Anyone with iOS experience seeing this? @panicsteve @zydeco @pguyot @jake-b ?

@rbruels
Copy link
Author

rbruels commented Dec 13, 2025

Appreciate it @MatthiasWM -- also submitted #200 and #202 which could use the same iOS-savvy eyes 👀

@rbruels
Copy link
Author

rbruels commented Jan 2, 2026

@MatthiasWM @pguyot Anything I can do to support these PRs getting reviewed/integrated?

This one is probably the "riskiest" so could use some other eyes, but #200 and #202 are pretty straightforward bug fixes and quality-of-life improvements

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