Fix #5162: eliminate Metal rotation black flash on idle rotate#5165
Merged
Conversation
On the iOS Metal backend, rotating the device after the app had gone idle produced a momentary black flash during the rotation animation. Root cause: METALView.updateFrameBufferSize: rebuilds screenTexture at the new size and changes layer.drawableSize on every rotation. Changing a CAMetalLayer's drawableSize invalidates the layer's currently displayed drawable, so the layer falls back to its opaque (black) background until the next presentDrawable:. Because this port disables the CADisplayLink (startAnimation is commented out), the next present only arrives once the EDT wakes, re-lays-out and repaints. While the app is actively painting that gap is sub-frame and invisible; once the app is idle (~10s) the gap spans the whole ~0.3s rotation animation, so the black background shows. The GL/CAEAGLLayer path doesn't use a drawable pool and so never flashed. Fix: preserve the previous frame across the resize. updateFrameBufferSize now captures the old screen texture, scale-blits it into the freshly created (new-size) screenTexture instead of priming to black, and presents it once immediately. The CAMetalLayer therefore keeps showing the last frame (stretched, like UIKit's own rotation snapshot) for the duration of the animation, until the EDT repaint presents the correctly laid-out frame. The black clear is retained for the very first sizing, where no previous frame exists. Verified on Xcode 26.3 / iOS 26.3 simulator (Metal backend): clean compile, app runs, the full 122-screen UI screenshot suite renders correctly, and the post-rotation landscape frame is correct (the new preserve+present path runs with no rendering regression). The device- specific transient flash itself (iPhone 15 Pro Max / iOS 26.5) cannot be reproduced in the auto-cycling test app on the simulator and should be confirmed on-device by the reporter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
|
Compared 122 screenshots: 122 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 122 screenshots: 122 matched. Benchmark Results
Detailed Performance Metrics
|
shai-almog
added a commit
that referenced
this pull request
Jun 5, 2026
…nt blit Bisected the iOS Metal screenshot regression: it started on master with bf88863 ("Fix #5162: eliminate Metal rotation black flash on idle rotate") - the last green metal run was 3af3c17, the first red was bf88863. The job's app builds (`BUILD SUCCEEDED`) but then hangs/crashes on launch and delivers 0 screenshots, intermittently; master fails the same way. Root cause: the new present-on-resize block in METALView.updateFrameBufferSize does `[layer nextDrawable]` immediately after setting layer.drawableSize, then unconditionally `copyFromTexture: sourceSize:(pw,ph)` into the drawable. The first nextDrawable after a drawableSize change can still return a drawable at the previous size, so the copy overruns the destination. Under the Metal validation layer in *assert* mode - which the CI screenshot job enables (MTL_DEBUG_LAYER_ERROR_MODE=assert) but local dev does not - that "sourceSize exceeds destination" violation aborts the app on the first resize during launch, before any screenshot. That's why it "rendered correctly" when verified locally yet fails in CI, and why it's intermittent (depends on whether the layer committed the resize). Fix: clamp the copy region to fit both the source screenTexture and the drawable's actual texture, so the transient rotation-preserve blit is always in-bounds. Only the on-screen present is affected; the captured screenTexture content (and therefore every screenshot baseline) is unchanged. Also reverts the speculative build-ios-metal timeout bump from an earlier commit in this branch - the build was completing within the cap; the failure was this runtime crash, not a timeout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
shai-almog
added a commit
that referenced
this pull request
Jun 5, 2026
* Add modern pluggable advertising API and AdMob reference provider Replaces the deprecated ad stack (com.codename1.ads.AdsService/InnerActive, components.Ads, impl.FullScreenAdService/VServAds, and the legacy google.adUnitId/mopubId build-hint banners) with a single, pluggable, format-complete advertising subsystem plus a modern Google AdMob provider. Core API (com.codename1.ads): AdManager facade with zero-wiring provider discovery (via NativeLookup AdProviderInstaller), InterstitialAd, RewardedAd, RewardedInterstitialAd, AppOpenAd, BannerAd, NativeAdLoader, AdConsent (UMP + iOS ATT), AdConfig/AdRequest/AdListener. Event-driven, EDT-marshalled. Deep integration in core: bindInterstitialOnTransition (reuses setOnCurrentFormChange), enableAppOpenAds, peer-backed banners. AdMob reference cn1lib (maven/cn1-admob, modeled on cn1-ai-mlkit-translate): GMA v24+ on Android, modern GAD* + UMP + ATT on iOS, JavaSE placeholder so all flows are testable in the simulator. Mediation-ready. Native deps declared in codenameone_library_required.properties. Build plugin: admob.appId build hint maps to the Android APPLICATION_ID meta-data and the iOS GADApplicationIdentifier + SKAdNetworkItems; ATT plist default via an AiDependencyTable entry. Legacy banner-hint path kept but documented as deprecated. Legacy ad classes marked @deprecated (javadoc-redirected) but still compile. Tests: maven/core-unittests/.../ads/AdManagerTest (11, fake provider); existing AdsTest still green. Sample: Samples/samples/AdsSample. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix SpotBugs SIC_INNER_SHOULD_BE_STATIC_ANON in NativeAdLoader The anonymous Runnable/SuccessCallback instances in NativeAdLoader.load did not use the enclosing instance, so SpotBugs (Max effort, run only on the JDK 8 CI matrix) flagged them. Move the logic into a private static loadInternal helper so the anonymous classes carry no outer reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix non-ASCII em-dashes in ads package-info breaking the Ant Android build The Ant Android port build (Ports/Android/build.xml) compiles CodenameOne/src with the platform-default US-ASCII encoding (the Maven build uses UTF-8, which is why it passed). The em-dash characters in the ads package-info javadoc were unmappable under US-ASCII and failed javac. Replace them with ASCII hyphens. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Remove unnecessary AdRequest no-arg constructor (PMD gate) The quality-report PMD gate forbids UnnecessaryConstructor. AdRequest declared an explicit empty public no-arg constructor identical to the compiler-generated default; remove it. `new AdRequest()` still works via the implicit default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Rework advertising API per review: install(), AdCallback, generic build, mock lib, docs Addresses review feedback on the advertising API: - Registration: dropped the AdProviderInstaller/NativeLookup auto-discovery and its environment-justification comments. Providers now expose a static install() (AdMobProvider.install(), MockAdProvider.install()) that calls AdManager.registerProvider; AdManager just holds the registered provider. - Callbacks: replaced com.codename1.util.SuccessCallback with a functional com.codename1.ads.AdCallback<T> across the API and SPI. - Build genericity: reverted all builder + AiDependencyTable changes. No build hints or builder code are needed to implement the SPI. The AdMob app id is configured with the standard android.xapplication / ios.plistInject hints, documented in the guide; SDK deps stay in the library's required.properties. - Testing: added a deterministic, network-free mock provider (cn1-ads-mock) rendering fixed labelled ads for pixel-stable screenshots; added native-ad unit tests; wired an advertising screenshot test (banner + native feed) into scripts/hellocodenameone using the mock provider. - Native ads: clearer content-feed use case in the sample and the guide. - Docs: removed the old Google-play-ads section from Monetization and added a new Advertising chapter. - Removed the @author tags from the new files. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix developer guide docs quality gate (Vale, LanguageTool, unused image) - Vale Microsoft.Adverbs: drop "gracefully"/"strictly"/"naturally". - Vale Microsoft.Contractions: use "doesn't"/"it's"/"they're". - LanguageTool BEEN_PART_AGREEMENT: reword "the recommended flow is initialize" to "the recommended order is to initialize". - Remove the orphaned img/google-play-ads.png (its only reference was the deleted legacy ads section). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Wire ad screenshot test, build/release ad cn1libs, add AppLovin + LevelPlay providers - Wire AdsScreenshotTest into Cn1ssDeviceRunner.DEFAULT_TEST_CLASSES so the hellocodenameone screenshot suite actually runs it (it pulls cn1-ads-mock via Maven and produces the AdsScreen screenshot on the device platforms). - CI: the PR "Build AI cn1libs" step now also compiles+tests cn1-admob/common, cn1-applovin/common, cn1-unity-levelplay/common and cn1-ads-mock. The full reactor (setup-workspace) already builds and installs them, and the release-on-maven-central deploy publishes them with the framework at the shared version - same numbering as the AI cn1libs. - Add two more provider cn1libs besides AdMob for coverage: cn1-applovin (AppLovin MAX) and cn1-unity-levelplay (Unity LevelPlay / ironSource), each mirroring cn1-admob (common provider + bridge + callback + consent, Android + iOS native impls, JavaSE placeholder, lib aggregator). Registered in maven/pom.xml. - Developer guide: add an "Ad provider cn1libs" section (lib table + -lib Maven coordinates with type pom, released with the framework) mirroring the AI cn1lib section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add simulator screenshot of the advertising sample to the developer guide Rendered the advertising sample (news feed with an in-feed native ad + an anchored banner) via the JavaSE port against the deterministic mock provider, and embedded the screenshot in the Advertising chapter. Also made the feed labels transparent in AdsScreenshotTest so the dark feed shows through (the default Label UIID paints an opaque background), matching the screenshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix oversized mock banner and a docs contraction - MockAdProvider/AdMob placeholder banners used CN.convertToPixels(50) for the height, but convertToPixels takes a dip count (~mm), so the banner rendered ~half the screen and pushed the native-ad content off. A ~50dp banner is ~8mm; a 300x250 rectangle ~48x40mm. The banner is now a thin strip and the feed shows the full native ad. - Developer guide: "They are" -> "They're" (Vale Microsoft.Contractions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Use the readable phone-resolution ad screenshot; add Android golden Replace the unreadable desktop-skin screenshot in the developer guide with the phone-resolution render captured by the Android screenshot job (full native-ad feed row + a thin banner, at readable size), and commit it as the Android golden (scripts/android/screenshots/AdsScreen.png). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Give the iOS Metal build step headroom so it stops timing out build-ios-metal repeatedly died with exit 12 during the "Build sample iOS app (Metal)" step (zero screenshots captured) - it was hitting the 30-min build-step cap, which the workflow comments already note was borderline on master as the app grows. The non-metal build-ios compiled the same app and passed, so this is build-time/runner variance, not a code defect. Bump the Metal build step to 45m and the job to 90m (build <=45 + run <=45 <= 90). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add iOS GL and Mac screenshot goldens for the advertising test Captured the AdsScreen render from the passing iOS GL (build-ios) and Mac (build-mac-native) screenshot runs and committed them as the per-platform goldens, so the advertising screenshot is now strictly compared on those platforms too (joining the Android golden). The iOS Metal golden is intentionally not added yet: the build-ios-metal job's app hangs on the first Metal frame on the iOS 26.2 simulator (under MTL_DEBUG_LAYER assert) and delivers 0 screenshots - the same intermittent framework Metal issue that fails the job on master. The caches are byte-identical to the GL job and the metallib builds cleanly, so it is not cache pollution or the ad code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix iOS Metal launch crash (#5165 regression): clamp the resize present blit Bisected the iOS Metal screenshot regression: it started on master with bf88863 ("Fix #5162: eliminate Metal rotation black flash on idle rotate") - the last green metal run was 3af3c17, the first red was bf88863. The job's app builds (`BUILD SUCCEEDED`) but then hangs/crashes on launch and delivers 0 screenshots, intermittently; master fails the same way. Root cause: the new present-on-resize block in METALView.updateFrameBufferSize does `[layer nextDrawable]` immediately after setting layer.drawableSize, then unconditionally `copyFromTexture: sourceSize:(pw,ph)` into the drawable. The first nextDrawable after a drawableSize change can still return a drawable at the previous size, so the copy overruns the destination. Under the Metal validation layer in *assert* mode - which the CI screenshot job enables (MTL_DEBUG_LAYER_ERROR_MODE=assert) but local dev does not - that "sourceSize exceeds destination" violation aborts the app on the first resize during launch, before any screenshot. That's why it "rendered correctly" when verified locally yet fails in CI, and why it's intermittent (depends on whether the layer committed the resize). Fix: clamp the copy region to fit both the source screenTexture and the drawable's actual texture, so the transient rotation-preserve blit is always in-bounds. Only the on-screen present is affected; the captured screenTexture content (and therefore every screenshot baseline) is unchanged. Also reverts the speculative build-ios-metal timeout bump from an earlier commit in this branch - the build was completing within the cap; the failure was this runtime crash, not a timeout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Revert the #5162 Metal change that crashes the iOS Metal screenshot suite Bisected the iOS Metal screenshot regression to bf88863 ("Fix #5162: eliminate Metal rotation black flash on idle rotate"): the last green metal run was its parent 3af3c17, the first red was bf88863, and it is the only commit that touched the Metal renderer. The app builds (BUILD SUCCEEDED) but then crashes / hangs on the first launch-time resize and delivers 0 screenshots, intermittently; master fails the same way. The new updateFrameBufferSize code (scale-blit the old frame into the new screenTexture, then present it) trips the Metal validation layer, which the CI screenshot job runs in assert mode (MTL_DEBUG_LAYER_ERROR_MODE=assert) - so it aborts in CI but renders fine in local dev without validation, which is why it was not caught. A targeted clamp of the present blit did not resolve it, and the CI device logs are filtered to CN1SS markers so the exact validation message is not available to pinpoint which of the new operations violates. Revert METALView.updateFrameBufferSize to its pre-bf888636b form (prime the new screen texture to opaque black on resize). This restores the proven-green Metal path and matches the committed screenshots-metal baselines. The #5162 rotation flash is device-only and (per bf88863's own notes) not reproducible on the simulator; it should be re-implemented with Metal API Validation enabled so the violation is caught before merge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix #5162 properly: attach Stencil8 to the resize preserve-pass (Metal) Reinstates the #5162 rotation-black-flash fix (reverted in bba6b2c / clamped in dd4f25d as stop-gaps) with the actual root cause fixed, reproduced and verified locally in Xcode. Root cause (reproduced locally on Xcode 26.3 / iOS 26.3 simulator with MTL_DEBUG_LAYER=1 MTL_DEBUG_LAYER_ERROR_MODE=assert, exactly as the CI build-ios-metal job runs it): (Metal) -[MTLDebugRenderCommandEncoder setRenderPipelineState:], line 1639: error 'Set Render Pipeline State Validation For stencil attachment, the renderPipelineState pixelFormat must be MTLPixelFormatInvalid, as no texture is set.' ... We had a signal 6 updateFrameBufferSize:'s preserve step draws the previous frame into the new screenTexture via CN1MetalDrawImage, which binds a CN1MetalPipelineCache pipeline. Every pipeline in that cache declares stencilAttachmentPixelFormat=Stencil8 (polygon-clip #3921), so any render pass that binds one MUST attach a Stencil8 texture (same constraint already handled for the seed draw in CN1Metalcompat.m, #5103). The preserve pass (clearPass) attached only a colour target, so Metal validation aborted on the very first launch-time resize. Under the CI assert mode that abort is a SIGABRT, which is why the job built fine yet delivered 0 screenshots and timed out; without validation (local dev) it renders, which is why it slipped through. The earlier present-blit clamp addressed the wrong operation and did not help. Fix: attach a throwaway clear-on-load Stencil8 texture to the preserve pass (only when a previous frame exists; the plain black clear binds no pipeline), mirroring the seed-draw precedent. The preserve draw never engages the stencil test, so its contents are irrelevant. Verified locally: forced-Metal build of the device-runner app launched in the simulator under MTL_DEBUG_LAYER assert. Before the fix it aborted with the validation error above ~1s in and delivered 0 screenshots; after the fix it runs the full screenshot suite to completion (100+ screenshots delivered over the CN1SS WebSocket transport) with zero Metal validation errors and no signal 6. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add iOS Metal golden for the advertising screenshot test Captured from the now-green build-ios-metal CI job (run 26994110757) on the fixed Metal renderer (255e1f7). This was the last missing AdsScreen golden; Android, iOS-GL and Mac were already committed. The Metal suite renders the ad feed (native "Sponsored" row + anchored banner) identically to the other backends. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Give the Mac native screenshot step headroom so it stops flaky-timing-out The build-mac-native job intermittently failed with a 30-minute GitHub step timeout while still "Waiting for DeviceRunner completion marker" -- the Mac Catalyst app launched and was running the suite, no screenshot mismatch and no crash. run-mac-native-ui-tests.sh's own suite budget is already 1500s (25m); add app launch plus post-processing (compare 122 images, render the report, post the PR comment) and a slow macos-15 runner tips just past the old 30m step cap and gets SIGKILLed mid-compare. That is runner variance, not a code defect: the Mac Catalyst backend compiles EAGLView.m (GL) and excludes METALView.m entirely, so it is unaffected by the iOS Metal renderer changes on this branch. Mirror the iOS Metal headroom bump (a08e87b): screenshot-run step 30 -> 45m and the job cap 45 -> 90m (build <=45 + run <=45 <= 90), so the script's internal 25m timeout governs gracefully instead of a hard step SIGKILL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
#5162 — With Metal active, rotating an iPhone between portrait and landscape flashes black during the tilt, breaking the smooth-transform illusion. The reporter noted it happens consistently after waiting ~10s before tilting ("some kind of internal refresh cycle").
Root cause
METALView.updateFrameBufferSize:rebuildsscreenTextureat the new size and setslayer.drawableSizeon every rotation. Changing aCAMetalLayer'sdrawableSizeinvalidates the layer's currently displayed drawable, so the layer shows its opaque (black) background until the nextpresentDrawable:.This port disables the
CADisplayLink(startAnimationis commented out), so the next present only arrives once the EDT wakes, re-lays-out and repaints:The GL /
CAEAGLLayerpath doesn't use a drawable pool and never flashed, which is why this is Metal-specific. (The earlier #4954 fix corrected the new frame's size but said nothing about what's shown during the gap.)Fix
Preserve the previous frame across the resize.
updateFrameBufferSize:now:screenTexturebefore replacing it.screenTexture(viaCN1MetalDrawImage) instead of priming to black.The
CAMetalLayertherefore keeps showing the last frame — stretched to fill, exactly like UIKit's own rotation snapshot — for the duration of the animation, until the EDT repaint presents the correctly laid-out frame. The plain black clear is retained for the very first sizing, where there is no previous frame.The change is additive and only runs on an actual resize (when a prior frame exists); the first-paint / steady-state paths are byte-identical to before.
Verification (local, this machine)
Built and ran on Xcode 26.3 / iOS 26.3 simulator, Metal backend (
ios.metaldefault-on):** BUILD SUCCEEDED **—METALView.mcompiles cleanly on the real iOS Metal pipeline (full ParparVM Xcode build).scripts/run-ios-ui-tests.sh, metal refs).landscapeframe (2532×1170) is correct — proper layout, no black, no corruption — confirming the new preserve+present path executes on a real rotation/resize with no rendering regression.🤖 Generated with Claude Code