[net11.0] Make iOS image scaling deadlock-safe

[kubaflo]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Summary

• Replace the unsupported UIGraphics.BeginImageContext image-scaling path with a private CGBitmapContext pushed onto UIKit's per-thread graphics stack.
• Preserve the legacy 1x backing scale, transparent output, standard color range, image orientation, rendering behavior, and the public CGSize overload's no-op behavior for nonpositive dimensions.
• Keep synchronous ScaleImage usable from background threads without changing process-global UIKit diagnostics or synchronously depending on main-queue progress.
• Carry only the isolated Graphics fix from [net11] Image metadata API in Graphics; stop Android MediaPicker mutating picked source files #36581, without the unrelated image-metadata API work.

Failure addressed

The .NET 11 iOS 26.5 bindings mark BeginImageContext, GetImageFromCurrentImageContext, and EndImageContext unsupported on iOS and MacCatalyst 17 or later. These CA1416 diagnostics are promoted to errors and stop affected builds before tests run, as demonstrated by build 1537895.

PR #37090 provides the intentionally minimal Preview 7 build unblock by suppressing those diagnostics. This PR is the durable net11.0/RC1 implementation that removes the dependency on the unsupported APIs.

Coverage

• ScaleImageUsesOneXBackingScale covers 1x, 2x, and 3x source images and verifies returned scale and backing pixel dimensions.
• ScaleImageReturnsOriginalForNonPositiveSize covers zero and negative width and height values, including disposeOriginal behavior.
• ScaleImageCanRunOnBackgroundThread verifies ordinary worker-thread use.
• ScaleImageDoesNotRequireMainThreadProgress blocks the main thread while scaling on a worker and verifies the synchronous API cannot deadlock waiting for main-queue progress.
• ScaleImageMatchesUIKitRendering compares exact pixels against UIGraphicsImageRenderer for all eight orientations using fractional dimensions, alpha, and transparency.
• The implementation previously passed all 50 Graphics device tests on both iOS and MacCatalyst; fresh net11.0 CI will validate the retargeted PR.

Issues Fixed

N/A

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 37057

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37057"

[kubaflo]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).

[Copilot]

Pull request overview

This PR updates the iOS/MacCatalyst image-scaling implementation in Microsoft.Maui.Graphics to avoid deprecated/unsupported UIKit APIs by switching from UIGraphics.BeginImageContext to UIGraphicsImageRenderer, while keeping the output non-opaque and preserving the source image scale.

Changes:

• Replace UIGraphics.BeginImageContext / GetImageFromCurrentImageContext / EndImageContext usage with UIGraphicsImageRenderer in UIImageExtensions.ScaleImage.
• Preserve transparency (Opaque = false) and source scale (Scale = target.CurrentScale) for scaled images.

[kubaflo]

@PureWeen this fix is now proven at the current head 0d1994b2: device build 1537994 contains that commit, and the Mono device-test build stage succeeded. Its build log has no CA1416 diagnostics, clearing the exact UIImageExtensions.ScaleImage blocker this PR targets. Please merge this preview7 fix.

[PureWeen]

Adversarial code review — 3 independent reviewers with adversarial consensus (+ repo domain rules)

One substantive finding, unanimous across all three reviewers. The deprecated-API removal itself is sound; the concern is a silent behavior change that rides along with it.

Findings

❌ Regression / Image Handling — ScaleImage(this UIImage, CGSize, bool) now rasterizes at the source image's scale instead of 1.0 (3/3 reviewers)

UIGraphics.BeginImageContext(size) is documented by Apple as equivalent to UIGraphicsBeginImageContextWithOptions(size, opaque: false, scale: 1.0) — the scale is always 1.0, independent of the source image. The new renderer sets Scale = target.CurrentScale.

Net effect for a source image with CurrentScale != 1:

	before	after
returned UIImage.Size (points)	size	size (unchanged)
returned UIImage.Scale	1.0	target.CurrentScale
backing CGImage pixels	size.W × size.H	size.W·scale × size.H·scale

Because the logical Size is unchanged, PlatformImage.Width/Height still report the expected values — so this is invisible to a smoke test, but the raster behind them grows 4× (@2x) or 9× (@3x).

Concrete scenario: a @3x asset-catalog image goes through IImage.Downsize(500, 500) (e.g. PlatformImage.Downsize → ScaleImage). Previously the result was backed by a ~500×500 pixel buffer. Now it is 500×500 points backed by 1500×1500 pixels. AsPNG()/AsJPEG() encode the pixel buffer, so downstream memory and encoded file size grow ~9×, which works against Downsize's stated purpose of bounding image size. This reaches real consumers — Essentials MediaPicker ImageProcessor.ApplyResizing calls Downsize(...) to honor caller-supplied maxWidth/maxHeight before saving.

Worth noting the PR description says the change preserves the source image scale — the previous code did not preserve it, it forced 1.0. So this is a deliberate semantic change, not a preservation.

Mitigating context (reviewers agreed on this): the dominant in-repo path PlatformImage.FromStream → UIImage.LoadFromData yields CurrentScale == 1.0, so that path is byte-identical. The exposure is via the public ScaleImage/Downsize surface receiving a scale > 1 image.

The cited precedent, NormalizeOrientation, is a fair pattern to copy but not a fair semantics to copy: there, preserving CurrentScale is correct because the output is meant to match the source exactly. ScaleImage's entire job is to reduce size.

Suggested resolution: set Scale = 1 for exact parity with the code being replaced (still removes the CA1416-flagged APIs, which is the whole point of the backport) — or, if the scale-aware behavior is intended as a quality improvement, keep it but call it out explicitly as a behavior change rather than a preservation, and land it with a test. For a release/11.0.1xx-preview7 backport whose stated goal is unblocking a build, the minimal-delta option looks like the safer default.

⚠️ Testing — no coverage exists for ScaleImage or Downsize on any platform (3/3 reviewers; severity ranged 💡–❌)

A repo-wide search finds zero tests exercising either ScaleImage overload or IImage.Downsize. Since this change alters output raster dimensions in a scale-dependent way, nothing would catch a future flip in either direction. A small device test asserting Size, CurrentScale, and CGImage.Width/Height for both a 1× and a 2×/3× source would lock in whichever semantics are chosen above.

What looks right

Opaque = false faithfully matches the old context's implicit opaque: NO, and disposeOriginal remains correct — CreateImage runs its block synchronously, so target is fully drawn before target.Dispose(), with no use-after-dispose.

Considered and discarded

• Non-disposal of UIGraphicsImageRenderer/UIGraphicsImageRendererFormat — raised by 1/3, rejected by the other two: it is the identical pattern already used by NormalizeOrientation in the same file, the peers are finalizable rather than permanently leaked, and it is not introduced by this PR.
• Null-safety of target — target.CurrentScale would NRE on a null target, but so would the previous target.Draw(...). No new hazard.
• Thread affinity — UIGraphicsImageRenderer.CreateImage avoids the old mutable per-thread context stack. An improvement, not a regression.
• Unverified claim (noted, not scored): the "0 warnings, 0 errors on net11.0-ios26.5" validation could not be independently reproduced here. It is directionally credible and consistent with Apple's documented deprecation.

Prior review status

One prior automated review (copilot-pull-request-reviewer[bot]) posted a descriptive overview with no findings. No unresolved inline threads. Nothing here duplicates it.

---

Methodology: 3 independent reviewers with adversarial consensus, plus this repository's own expert-review dimensions (.github/agents/maui-expert-reviewer.md) routed to the changed file. Findings surviving cross-validation only. Code review only — CI status is out of scope for this review.

[kubaflo]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Graphics/src/Graphics/Platforms/iOS/UIImageExtensions.cs:61

• UIGraphicsImageRenderer wraps native resources (NSObject). To avoid delaying native resource release in hot paths, it should be disposed deterministically (consistent with other usages in the repo that use using var renderer = ...).

			var renderer = new UIGraphicsImageRenderer(size, new UIGraphicsImageRendererFormat

src/Graphics/tests/DeviceTests/Tests/ImageTests.cs:62

• Consider disposing sourceRenderer as well to avoid keeping a native renderer alive for the full test duration unnecessarily (it implements IDisposable via NSObject).

		var sourceRenderer = new UIGraphicsImageRenderer(sourceSize, new UIGraphicsImageRendererFormat

[PureWeen]

Round 2 — re-review of 38740560 "Preserve one-times image scale when downsizing"

Re-ran the full adversarial review against the new commit. Both round-1 findings are resolved. No blocking issues remain; two non-blocking notes below.

Round-1 findings — status

✅ RESOLVED — ❌ Regression (backing pixel dimensions). Scale = 1 is verifiably the exact semantic of the API being replaced: Apple documents UIGraphicsBeginImageContext(size) as equivalent to UIGraphicsBeginImageContextWithOptions(size, opaque: NO, scale: 1.0). Combined with Opaque = false, the returned UIImage.Size, .Scale, and CGImage pixel dimensions now all match the pre-change behavior. The Downsize → ScaleImage chain that feeds Essentials MediaPicker no longer inflates saved output for retina sources. Reviewers specifically checked that Scale on a new UIGraphicsImageRendererFormat() actually takes effect rather than silently no-opping — it does; it's a plain settable property, and this exact object-initializer pattern already ships in NormalizeOrientation in the same file.

✅ RESOLVED — ⚠️ Testing (no coverage). ScaleImageUsesOneXBackingScale is a genuine regression test, not a placebo. Reviewers worked the arithmetic independently: at sourceScale = 3 the source is 30×20 pt / 90×60 px; scaled to CGSize(10, 5) the old Scale = CurrentScale code produced CurrentScale == 3 and a 30×15 px CGImage, so Assert.Equal(1, …CurrentScale) and Assert.Equal(10, (int)…CGImage.Width) both fail against the regressed implementation and pass now. Mechanics also check out: context.FillRect(CGRect) is a real UIGraphicsRendererContext member; Assert.Equal(1, (double)…) resolves fine (the same int-literal-vs-double pattern already compiles elsewhere in this repo); the values asserted are exactly representable so strict float equality is safe; and #if IOS || MACCATALYST correctly excludes the guarded UIKit code from the android/windows TFMs this project also targets. The absence of main-thread dispatch is correct — UIGraphicsImageRenderer is explicitly designed for off-main-thread offscreen compositing, which is one of the reasons it supersedes the legacy context-stack API. And Graphics.DeviceTests is wired into eng/pipelines/device-tests.yml and eng/helix_xharness.proj for both the ios and catalyst legs, so this test actually runs.

Remaining non-blocking notes

💡 Regression — UIGraphicsImageRendererFormat.PreferredRange is left at its default (2/3 reviewers)

Scale and opacity now match exactly, but pixel format may not. PreferredRange defaults to Automatic, which selects extended/wide-gamut range on devices that support it, whereas BeginImageContext always produced a fixed 8-bit-per-component device-RGB bitmap. One reviewer reported probing this and observing 16-bpc extended-sRGB output where the old path gave 8-bpc — treat that specific measurement as reported-but-not-independently-confirmed here, though the documented Automatic default supports the direction. A third reviewer argued the deprecated parameterless UIGraphicsImageRendererFormat() initializer yields plain sRGB instead; that position lost 2-to-1.

Concrete consequence if it holds: a downsized wide-gamut image carries roughly double the backing memory per pixel and may encode differently through AsPNG()/AsJPEG() — a color-fidelity and memory difference, not a dimension regression.

Why this is not blocking: the identical gap already ships in NormalizeOrientation in this same file, so it is not a new divergence for the file — though it is new for ScaleImage specifically, since that method previously went through BeginImageContext. Given this is a preview7 backport whose purpose is clearing CA1416, PreferredRange = UIGraphicsImageRendererFormatRange.Standard would be the tightest-parity option if you want it, but chasing it here (and in NormalizeOrientation) is reasonable follow-up work rather than a merge gate.

💡 Testing — the [InlineData(1f)] row cannot catch the regression (2/3 reviewers)

At sourceScale = 1 the old and new implementations are identical (CurrentScale == 1 == 1), so every assertion in that row passes against the buggy code too. It is a fine baseline sanity check, but only the 3f row has regression-catching power. Consequence: if someone reintroduces Scale = target.CurrentScale, a green 1f row offers false reassurance. Adding [InlineData(2f)] would broaden the discriminating coverage cheaply.

Considered and discarded

Undisposed sourceRenderer / UIGraphicsImageRendererFormat instances in the test — raised by 1/3, rejected by the others: lightweight NSObject config wrappers, matching the pattern already shipping in NormalizeOrientation, with no held resource in a short-lived test.

Verdict

The fix is correct on its own technical merits, not merely responsive to prior feedback — the reviewers verified Scale = 1 against the replaced API's documented contract rather than accepting it as "different from what was flagged." Nothing blocking from this reviewer.

---

Methodology: 3 independent reviewers with adversarial consensus (round 2), plus this repository's own expert-review dimensions routed to the changed files. Code review only — CI status out of scope.

[kubaflo]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Graphics/tests/DeviceTests/Tests/ImageTests.cs:73

• The UIGraphicsImageRenderer callback parameter is a UIGraphicsImageRendererContext, which doesn’t expose FillRect; this should draw via ctx.CGContext. Also, the renderer itself should be disposed (it’s IDisposable) to avoid native resource pressure in test runs.

		var sourceRenderer = new UIGraphicsImageRenderer(sourceSize, new UIGraphicsImageRendererFormat
		{
			Opaque = false,
			Scale = sourceScale,
		});

src/Graphics/src/Graphics/Platforms/iOS/UIImageExtensions.cs:66

• UIGraphicsImageRenderer is IDisposable; consider disposing it (using var) to avoid holding native resources longer than necessary, especially since this helper may be called frequently.

			var renderer = new UIGraphicsImageRenderer(size, new UIGraphicsImageRendererFormat
			{
				Opaque = false,
				PreferredRange = UIGraphicsImageRendererFormatRange.Standard,
				Scale = 1,
			});

[MauiBot]

Expert Review — 5 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Async and Threading Safety — Failure path leaks a live background task over a disposed UIImage. If scaleTask.Wait(TimeSpan.FromSeconds(5)) on line 135 times out (i.e. the deadlock this test exists to detect has actually regressed), completedWhileMainThreadWasBlocked is false, this Assert.True throws, and the method unwinds — which runs the using var source disposal on line 127 while the Task.Run on line 134 is still inside source.ScaleImage(...) calling target.Draw(...) on that same native UIImage. The result is a native crash on a thread-pool thread instead of a clean TrueException, so the very regression this test guards against would be reported as an APP_CRASH with no failing test rather than as "ScaleImage must not synchronously depend on main-thread progress".

Suggested fix — source must not be in a using, because ownership is conditional on the worker actually settling (GC.KeepAlive is not sufficient: it prevents collection, not the deterministic Dispose() the using emits on the exception path):

// requires: using System.Runtime.ExceptionServices;
var source = CreatePatternImage(UIImageOrientation.Up);   // deliberately not `using`
Task<UIImage> scaleTask = null;
Exception waitFailure = null;
var completedWhileMainThreadWasBlocked = false;

void ScaleAndWait()
{
    try
    {
        scaleTask = Task.Run(() => source.ScaleImage(new CGSize(10, 5)));
        // the ONLY main-queue block, and it is the measurement itself: bounded, never retried
        completedWhileMainThreadWasBlocked = scaleTask.Wait(TimeSpan.FromSeconds(5));
    }
    catch (Exception ex)
    {
        waitFailure = ex;   // never unwind a managed exception through the native GCD frame
    }
}

if (NSThread.IsMain)
    ScaleAndWait();
else
    DispatchQueue.MainQueue.DispatchSync(ScaleAndWait);

if (waitFailure is not null)
{
    source.Dispose();                       // Wait threw => task settled => worker done with source
    ExceptionDispatchInfo.Capture(waitFailure).Throw();
}

if (!completedWhileMainThreadWasBlocked)
{
    // Regression case: hand ownership of `source` to a thread-pool continuation that runs only
    // once the worker really settles. Nothing here waits on the main queue.
    _ = scaleTask.ContinueWith(static (t, s) =>
    {
        if (t.Status == TaskStatus.RanToCompletion)
            t.Result?.Dispose();
        else
            _ = t.Exception;                // observe, don't escalate
        ((UIImage)s).Dispose();
    }, source, TaskScheduler.Default);

    Assert.Fail("ScaleImage must not synchronously depend on main-thread progress.");
}

try
{
    using var scaled = await scaleTask;     // already RanToCompletion: no further waiting
    Assert.Equal(10, (int)scaled.CGImage.Width);
    Assert.Equal(5, (int)scaled.CGImage.Height);
}
finally
{
    source.Dispose();
}

source is disposed only where the task has provably settled; if the worker truly never returns it is intentionally leaked for the remaining process lifetime, which is the right trade in a test host. Task.Wait(TimeSpan) throws on a faulted task rather than returning true, so completedWhileMainThreadWasBlocked == true implies RanToCompletion — that is what makes the await and the final Dispose() non-blocking, including on the NSThread.IsMain branch where any second wait would self-deadlock. The try/catch in ScaleAndWait also covers the secondary hazard: without it, an AggregateException from Wait has to unwind through the native DispatchSync frame.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[minor] Public API Surface Design — This early return hands back the same instance and silently ignores disposeOriginal: true. That is the right runtime behaviour (you cannot dispose what you are returning), and it matches the pre-existing shape at lines 11-14/29, but it is now frozen as intended behaviour by a new test, so it is worth making explicit.

The ownership hazard is concrete one level up: PlatformImage.Downsize wraps the result in a new PlatformImage (Platforms/iOS/PlatformImage.cs:25,31), so a caller passing disposeOriginal: true ends up with two IImage instances aliasing one undisposed UIImage while believing the original was released — the exact expectation Issue21886 asserts against (original.Width must throw ObjectDisposedException). Unreachable from Downsize today because the float overload guards first, hence minor. An XML <remarks> on the public method stating that disposeOriginal is ignored when the original is returned unchanged would close the contract gap without any behaviour change.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Graphics/tests/DeviceTests/Tests/ImageTests.cs:99

• This test intends to validate disposeOriginal behavior for non-positive sizes, but it currently only asserts reference equality. If ScaleImage(..., disposeOriginal: true) accidentally disposed the source before returning it, Assert.Same would still pass and the regression wouldn’t be detected. Add an assertion that the source is still undisposed after the call (e.g., source.Handle != IntPtr.Zero).

		var scaled = source.ScaleImage(new CGSize(width, height), disposeOriginal: true);

		Assert.Same(source, scaled);
	}

[MauiBot]

Expert Review — 4 findings

See inline comments for details.

[MauiBot]

AI Review Summary

@kubaflo — new AI review results are available based on this last commit: d11f043.

---

🗂️ Review Sessions — click to expand

🚦 Gate — Test Before & After Fix

Gate Result: ❌ FAILED

Platform: IOS · Base: net11.0 · Merge base: f9639d94

🩺 Test does not reproduce the bug — ran the same in both states (PASS without fix, PASS with fix). The repro test is not exercising the issue. Strengthen the test before reviewing the fix.

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
📱 ImageTests (ScaleImageReturnsOriginalForNonPositiveSize) ImageTests	❌ PASS — 144s	✅ PASS — 29s

🔴 Without fix — 📱 ImageTests (ScaleImageReturnsOriginalForNonPositiveSize): PASS ❌ · 144s

(no coded error found; showing last 1200 chars)

com.microsoft.maui.graphics.devicetests' from 'iPhone 11 Pro'
info: Application 'com.microsoft.maui.graphics.devicetests' was uninstalled successfully
info: <<XHARNESS_RESULT_START>>
      {
        "version": 1,
        "machineName": "XXN70R7KNX-1",
        "exitCode": 80,
        "exitCodeName": "APP_CRASH",
        "platform": "apple",
        "device": "iPhone 11 Pro",
        "deviceOsVersion": "26.5",
        "files": [
          {
            "name": "test-ios-simulator-64_26.5-0444836A-53AF-4720-9B1D-ABBEF6DEF6F6.log",
            "type": "executionlog"
          },
          {
            "name": "list-ios-simulator-64_26.5-20260805_140351.log",
            "type": "devicelist"
          },
          {
            "name": "iPhone 11 Pro.log",
            "type": "systemlog"
          },
          {
            "name": "Microsoft.Maui.Graphics.DeviceTests.log",
            "type": "systemlog"
          },
          {
            "name": "com.microsoft.maui.graphics.devicetests.log",
            "type": "applicationlog"
          }
        ]
      }
      <<XHARNESS_RESULT_END>>
XHarness exit code: 80 (APP_CRASH)
  Passed: 78
  Failed: 0
  Tests completed with exit code: 80

🟢 With fix — 📱 ImageTests (ScaleImageReturnsOriginalForNonPositiveSize): PASS ✅ · 29s

(no coded error found; showing last 1200 chars)

: "XXN70R7KNX-1",
        "exitCode": 0,
        "exitCodeName": "SUCCESS",
        "platform": "apple",
        "device": "iPhone 11 Pro",
        "deviceOsVersion": "26.5",
        "files": [
          {
            "name": "test-ios-simulator-64_26.5-0444836A-53AF-4720-9B1D-ABBEF6DEF6F6.log",
            "type": "executionlog"
          },
          {
            "name": "list-ios-simulator-64_26.5-20260805_140608.log",
            "type": "devicelist"
          },
          {
            "name": "test-ios-simulator-64_26.5-20260805_140610.log",
            "type": "testlog"
          },
          {
            "name": "iPhone 11 Pro.log",
            "type": "systemlog"
          },
          {
            "name": "Microsoft.Maui.Graphics.DeviceTests.log",
            "type": "systemlog"
          },
          {
            "name": "com.microsoft.maui.graphics.devicetests.log",
            "type": "applicationlog"
          },
          {
            "name": "xunit-test-ios-simulator-64_26.5-20260805_140610.xml",
            "type": "xmllog"
          }
        ]
      }
      <<XHARNESS_RESULT_END>>
XHarness exit code: 0
  Passed: 136
  Failed: 0
  Tests completed successfully

⚠️ Failure Details

• ❌ ImageTests (ScaleImageReturnsOriginalForNonPositiveSize) PASSED without fix (should fail) — tests don't catch the bug

📁 Fix files reverted (1 files)

• src/Graphics/src/Graphics/Platforms/iOS/UIImageExtensions.cs

---

📋 Pre-Flight — Context & Validation

Issue: N/A - .NET 11 iOS/MacCatalyst CA1416 image-scaling build failure
PR: #37057 - [net11.0] Make iOS image scaling deadlock-safe
Platforms Affected: iOS, MacCatalyst
Files Changed: 1 implementation, 1 test

Key Findings

• The PR replaces unsupported UIGraphics.BeginImageContext APIs with a per-call CGBitmapContext pushed onto UIKit's thread-local graphics stack, preserving 1x output and avoiding synchronous main-queue progress.
• The gate failed because the without-fix run ended in APP_CRASH (XHarness exit 80) after reporting zero failed xUnit tests, which the gate classifier treated as PASS; the with-fix run completed successfully.
• UIImageExtensions.cs:75 omits CGBitmapFlags.ByteOrder32Little, changing the returned CGImage metadata from the legacy BGRA layout to the default byte order.
• ImageTests.cs:98 verifies reference identity for invalid sizes but not that the returned original remains alive when disposeOriginal: true.
• Non-finite dimensions pass the new guard and throw OverflowException; this is safer than the old native crash but leaves the new invalid-size contract incomplete.
• Blast radius is limited to iOS/MacCatalyst ScaleImage and Downsize callers; there is no startup or shared-state impact. Adjacent risks include raw-pixel consumers, orientation, fractional sizes, disposal, and background execution.

Code Review Summary

Verdict: NEEDS_CHANGES
Confidence: low
Errors: 0 | Warnings: 2 | Suggestions: 1

Key code review findings:

• ⚠ UIImageExtensions.cs:75 changes the backing pixel byte-order contract.
• ⚠ ImageTests.cs:98 does not prove the returned original image remains usable.
• ℹ UIImageExtensions.cs:59 should reject non-finite dimensions explicitly.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #37057	Draw into a private CGBitmapContext through UIKit's per-thread graphics stack	❌ FAILED (Gate)	UIImageExtensions.cs	With-fix tests pass, but the without-fix native crash was misclassified as PASS; code review also found byte-order and liveness-test gaps.

---

🔬 Code Review — Deep Analysis

Code Review — PR #37057

Independent Assessment

What this changes: Replaces deprecated iOS/MacCatalyst image-context APIs with a thread-local 1× CGBitmapContext, adding invalid-size handling and device coverage.
Inferred motivation: Remove CA1416 failures without introducing main-thread dependence or changing rendered output.

Reconciliation with PR Narrative

Author claims: Legacy rendering, scale, transparency, orientation, and thread behavior are preserved.
Agreement/disagreement: Pixel and threading behavior match. However, the returned CGImage byte-order metadata changes, and invalid-size coverage is incomplete.

Prior Review Reconciliation

Prior ❌ Error Finding	Source	Status	Evidence
Retina sources produced oversized backing buffers	PureWeen	✅ Fixed	Output is explicitly created at scale 1; scale/device tests cover 1×–3×.
No regression coverage	PureWeen	✅ Fixed	Device tests now cover scale, threading, orientation, and invalid dimensions.
Renderer construction required the UI thread	Earlier expert review	🔄 Obsolete	Current implementation uses CGBitmapContext, not UIGraphicsImageRenderer.

No unresolved prior ❌ Error findings found.

Blast Radius Assessment

• Runs for all instances: Only iOS/MacCatalyst callers invoking ScaleImage/Downsize.
• Startup impact: No.
• Static/shared state: No.
• Adjacent Image/Graphics risks: Raw pixel consumers, orientation, fractional dimensions, disposal, and background execution were examined.

CI Status

• Required-check result: Undetermined because gh pr checks --required was unavailable; public data showed 3 failures and 19 pending checks.
• Classification: The completed failures are unrelated infrastructure failures—truncated Microsoft OpenJDK downloads (curl: (18)).
• Action taken: Invoked azdo-build-investigator; confidence capped low while CI remains pending.

External Output Contract

Consumer token/pattern	Producer location	Producer emission condition	Consumer assumption	Ordinary negative case	Downstream effect
None	—	—	No external-output classifier changed	—	—

Findings

⚠️ Warning — Backing pixel format no longer matches legacy output

UIImageExtensions.cs:75 creates the bitmap without CGBitmapFlags.ByteOrder32Little. The previous API and UIGraphicsImageRenderer return BGRA/ByteOrder32Little; this code returns ByteOrderDefault. Raw-data consumers can consequently reject the image or interpret channels incorrectly. The repository’s RawBitmap.cs:94-111 explicitly requires ByteOrder32Little.

Use:

CGBitmapFlags.ByteOrder32Little | CGBitmapFlags.PremultipliedFirst

⚠️ Warning — Invalid-size test does not verify liveness

ImageTests.cs:98 proves reference identity but still passes if the implementation disposes and returns the original image. Add Assert.NotNull(scaled.CGImage).

💡 Suggestion — Reject non-finite dimensions

UIImageExtensions.cs:59 permits NaN and infinity, which later produce OverflowException. Consider explicitly rejecting non-finite values.

Failure-Mode Probing

• Background execution: Uses a per-thread graphics stack; PopContext is protected by finally.
• Fractional sizes/orientations: Empirical comparison matched UIKit across all eight orientations.
• Invalid dimensions: Zero/negative values safely return the live original; non-finite values throw.
• Raw-byte consumers: Observe the byte-order regression described above.

Verdict: NEEDS_CHANGES

Confidence: low
Summary: The rendering and deadlock fix is sound, but the observable backing format should retain legacy ByteOrder32Little, and the disposal test should prove the returned image remains usable. CI failures observed so far are infrastructure-related, but checks remain pending.

---

🛠️ Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix / claude-opus-5	Pure Core Graphics resampling with explicit orientation transforms	❌ FAIL — 43 passed, 7 failed	1 file	Correct geometry and threading, but non-Up exact-pixel rows differ from UIKit's private sub-pixel sampling.
2	try-fix / gpt-5.6-sol	UIGraphicsImageRenderer + native Objective-C format construction	✅ PASS — Debug 50/50, Release 50/50	1 file	Exact fidelity and blocked-main-thread safety, but adds ABI-sensitive native interop and is not clearly better than the PR.
PR	PR #37057	Private CGBitmapContext + UIKit PushContext/PopContext + UIImage.Draw	❌ FAILED (Gate)	2 files	With-fix run passes; without-fix APP_CRASH was misclassified as PASS. Code review found byte-order and test-liveness gaps.

Cross-Pollination

Model	Round	New Ideas?	Details
claude-opus-5	1	Yes	Preserve UIImage.Draw semantics through a supported renderer rather than reimplementing UIKit orientation sampling.
gpt-5.6-sol	1	Yes	Bypass only the managed format constructor's Debug thread guard with narrow native interop; keep the renderer on public APIs.
claude-opus-5	2	No	Exact UIKit fidelity forces UIImage.Draw; the deprecated context API, the PR's pushed context, and candidate 2's renderer exhaust the viable context providers under blocked-main-thread constraints.

Exhausted: Yes
Selected Fix: PR's context-stack architecture, after addressing the pre-flight byte-order, invalid-size, and liveness-test findings — candidate 1 failed exact rendering, while candidate 2 passed but adds ABI-sensitive Objective-C interop and is not demonstrably better overall.

---

📝 Recommended PR Title & Description

Assessment: ✏️ Recommend updating — the current title omits MacCatalyst and the metadata predates the winning fix's byte-order, invalid-size, and expanded coverage changes.

Recommended title

[iOS/MacCatalyst] Graphics: Make .NET 11 image scaling deadlock-safe

Recommended description

## Summary

- Replace the unsupported `UIGraphics.BeginImageContext` image-scaling path with a private `CGBitmapContext` pushed onto UIKit's per-thread graphics stack.
- Preserve the legacy 1x backing scale, transparent output, standard color range, UIKit-native BGRA byte order, image orientation, rendering behavior, and ceil-to-integral output size.
- Return the live original image for nonpositive, non-finite, oversized, or otherwise unrepresentable dimensions, regardless of `disposeOriginal`.
- Keep synchronous `ScaleImage` usable from background threads without changing process-global UIKit diagnostics or synchronously depending on main-queue progress.
- Carry only the isolated Graphics fix from #36581, without the unrelated image-metadata API work.

## Failure addressed

The .NET 11 iOS 26.5 bindings mark `BeginImageContext`, `GetImageFromCurrentImageContext`, and `EndImageContext` unsupported on iOS and MacCatalyst 17 or later. These `CA1416` diagnostics are promoted to errors and stop affected builds before tests run, as demonstrated by [build 1537895](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1537895).

PR #37090 provides the intentionally minimal Preview 7 build unblock by suppressing those diagnostics. This PR is the durable `net11.0`/RC1 implementation that removes the dependency on the unsupported APIs.

## Files changed

- `src/Graphics/src/Graphics/Platforms/iOS/UIImageExtensions.cs` replaces the unsupported context APIs, preserves UIKit drawing semantics and BGRA metadata, and guards invalid bitmap dimensions.
- `src/Graphics/tests/DeviceTests/Tests/ImageTests.cs` covers backing scale, invalid-size liveness, threading, orientation, fractional sizing, byte order, alpha, and transparency.

## Coverage

- `ScaleImageUsesOneXBackingScale` covers 1x, 2x, and 3x source images and verifies returned scale and backing pixel dimensions.
- `ScaleImageReturnsOriginalForInvalidSize` covers zero, negative, NaN, positive-infinity, and oversized width and height values, including `disposeOriginal` behavior and the returned image's liveness.
- `ScaleImageCanRunOnBackgroundThread` verifies ordinary worker-thread use.
- `ScaleImageDoesNotRequireMainThreadProgress` blocks the main thread while scaling on a worker and verifies the synchronous API cannot deadlock waiting for main-queue progress.
- `ScaleImageMatchesUIKitRendering` compares exact pixels against `UIGraphicsImageRenderer` for all eight orientations using fractional dimensions, alpha, and transparency, and pins the ceil-rounded output size and UIKit-native byte order.
- The reviewer-enhanced implementation passed all 56 focused Graphics image tests on iOS 26.5 in both Debug and Release. The underlying implementation previously passed all 50 Graphics device tests on both iOS and MacCatalyst; fresh `net11.0` CI will validate the expanded coverage on both platforms.

## Alternatives evaluated

- Pure Core Graphics orientation normalization and resampling was rejected after 7 of 50 exact-pixel cases diverged from UIKit's private fractional/orientation sampling.
- `UIGraphicsImageRenderer` passed all 50 Debug and Release cases, but required ABI-sensitive Objective-C message declarations and manual native ownership to construct `UIGraphicsImageRendererFormat` off-main in Debug. That complexity was not justified over the private bitmap-context approach.

### Issues Fixed

N/A

---

🏁 Report — Final Recommendation

Comparative Report - PR #37057

Decision

Winner: pr-plus-reviewer

It is the only candidate that combines exact UIKit rendering behavior, blocked-main-thread safety, the legacy BGRA byte-order contract, robust invalid-size handling, focused Debug and Release passes, and no ABI-sensitive Objective-C interop.

Ranking

Rank	Candidate	Regression evidence	Assessment
1	pr-plus-reviewer	PASS - iOS Debug 56/56; Release 56/56	Keeps the PR's minimal context-stack design and fixes all four expert findings with discriminating coverage.
2	try-fix-2	PASS - iOS Debug 50/50; Release 50/50	Behaviorally correct and deadlock-safe, but requires four ABI-sensitive objc_msgSend declarations and manual native ownership solely to bypass a binding thread check.
3	pr	Gate FAILED; with-fix run 50/50	The gate failure is a baseline-classification artifact rather than a product regression, but the raw fix still changes byte-order metadata and leaves invalid-size, liveness, and fractional-size contracts under-covered. It therefore ranks below candidates with clean regression passes.
4	try-fix-1	FAIL - 43/50	Pure Core Graphics avoids UIKit drawing state, but all seven non-Up exact-pixel rows diverge from UIKit's private fractional/orientation sampling. It also loses UIImage.Draw behaviors for resizable, template, and CIImage-backed images.

Comparative analysis

• Correctness and fidelity: pr-plus-reviewer and try-fix-2 preserve UIImage.Draw behavior for all eight orientations. try-fix-1 is empirically incompatible with the pixel oracle. The raw PR renders correctly but changes CGImage.ByteOrderInfo.
• Threading: The winner and try-fix-2 both complete while the main thread is blocked. The winner uses public CGBitmapContext plus UIKit's per-thread context stack; try-fix-2 relies on native construction of UIGraphicsImageRendererFormat to evade a managed Debug thread guard.
• Maintainability: The winner is a surgical extension of the submitted fix. try-fix-2 adds platform ABI and ownership code with a future dependency on binding behavior. try-fix-1 adds a large orientation-normalization implementation without matching UIKit.
• Coverage: The winner expands the focused suite from 50 to 56 cases and directly pins liveness, invalid numeric inputs, ceil rounding, byte order, background execution, blocked-main-thread execution, and exact pixels.
• Gate handling: The supplied gate remains failed and was not rerun as instructed. Its without-fix native abort is not representable as a normal managed xUnit failure, so the candidate ranking uses the recorded candidate regression runs while still ranking the formally failed raw PR below passing candidates.

pr-plus-reviewer is the strongest fix and should be used as the PR outcome.

---

🧭 Next Steps — review latest findings

No alternative fix was selected for this run. Review the session findings and CI results before merging.

[sheiksyedm]

/azp run maui-pr-uitests , maui-pr-devicetests

[azure-pipelines]

Azure Pipelines:
Successfully started running 2 pipeline(s).

[github-actions]

Tests Failure Analysis

@kubaflo — test-failure review results are available based on commit d11f043.

Maintainers can request a fresh review after new comments, commits, or CI runs by commenting /review tests.

Test Failure Review: Not ready - click to expand

Overall verdict: Not ready. Deterministically, 60 legs/tests are red on this PR but green across the 5 recent net11.0 base builds sampled per definition, so they score as regressions vs base; only 1 of 65 distinct failures also appears on base (flaky). The failures cluster into a macOS build-provisioning break and a large block of WinUI/Android UITest timeouts that look like a host-launch/infra cascade rather than the iOS graphics change, but none can be dismissed as pre-existing, so a human must confirm before merge.

• ✗ PR-related — macOS provisioning/publish chain (~7 legs): brew install --cask microsoft-openjdk@17 fails, cascading into empty test-result/log publish steps; representative Provision JDK - MSB3073. Green on all 5 base builds — flagged as regressed-vs-base though it reads as an environment break.
• ✗ PR-related — WinUI/Android UITest mass element timeouts (~50 tests): every Entry and TimePicker VisualState test times out waiting for elements, consistent with the host app failing to load on the leg rather than a per-test defect; representative VerifyTextWhenTextColorSetCorrectly. Green on base.
• i Uncertain — unexplained/aborted build legs (~19 legs + 2 checks): build-break legs with no extractable failure (crossgen/publish/Controls suites) plus cancelled MacCatalyst UITests Controls Shell and WinUI CV1 Controls (CV1) checks and the unmapped Build Analysis check — must be opened by hand.
• i Uncertain — unattributed / flaky-on-base (~5): DragEvents (flaky on base), Android platform-tools;35.0.2 provisioning, ShellBackgroundSupportsGradientBrush (baseline snapshot not yet created), and an Android result-publish error — none provably PR-caused nor dismissible.

Coverage: 141 checks · 129 passing · 12 failing · 0 pending · 0 inaccessible · 1 unmapped · 19 unexplained build legs · 0 unaccounted failing checks · 2 aborted failing checks · 0 canceled-build checks · 0 device-test unverified · 5 unattributed · 60 regressed-vs-base. Deterministic ceiling: Not ready — 60 legs regressed vs base, 19 unexplained build legs, 2 aborted checks, 1 unmapped check, and 5 unattributed failures.

Builds (this PR): maui-pr 1541641, maui-pr-uitests 1542212, maui-pr-devicetests 1542214 (device tests confirmed clean). Base sampling (net11.0, 5 recent builds per definition): maui-pr 1541716, 1541599; maui-pr-uitests 1541749, 1541455.

Recommended action

Have a human open the macOS Provision JDK leg and a representative WinUI UITest leg — the mass timeouts and brew failure look like an infra/host-launch cascade, but since base builds were green they cannot be auto-dismissed; re-run the affected legs and confirm before merging.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Graphics/tests/DeviceTests/Tests/ImageTests.cs:96

• This test covers NaN and Infinity in addition to nonpositive values, so the method name is misleading. Consider renaming to reflect that it’s asserting the no-op behavior for invalid/non-finite sizes too.

	public void ScaleImageReturnsOriginalForNonPositiveSize(double width, double height)

src/Graphics/src/Graphics/Platforms/iOS/UIImageExtensions.cs:63

• The input guard relies on !(size.Width > 0) to treat NaN as invalid, but that intent isn’t obvious. Making NaN/Infinity checks explicit (and using <= 0 for the nonpositive case) improves readability and reduces the chance of future edits accidentally changing the NaN behavior.

			if (!(size.Width > 0) || !(size.Height > 0) ||
				double.IsInfinity(size.Width) || double.IsInfinity(size.Height))
			{
				return target;
			}